On Sun, May 19, 2024 at 10:43 PM Erik Wienhold <e...@ewie.name> wrote: > > On 2024-05-19 07:00 +0200, Alexander Lakhin wrote: > > I encountered anomalies that you address with this patch too. > > And I can confirm that it fixes most cases, but there is another one: > > SELECT $300000000 \bind 'foo' \g > > ERROR: invalid memory alloc request size 1200000000 > > > > Maybe you would find this worth fixing as well. > > Yes, that error message is not great. In variable_paramref_hook we > check paramno > INT_MAX/sizeof(Oid) when in fact MaxAllocSize/sizeof(Oid) > is the more appropriate limit to avoid that unspecific alloc size error. > > Fixed in v4 with a separate patch because it's unrelated to the param > number parsing. But it fits nicely into the broader issue on the upper > limit for param numbers. Note that $268435455 is still the largest > possible param number ((2^30-1)/4) and that we just return a more > user-friendly error message for params beyond that limit. >
hi, one minor issue: /* Check parameter number is in range */ if (paramno <= 0 || paramno > MaxAllocSize / sizeof(Oid)) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_PARAMETER), errmsg("there is no parameter $%d", paramno), parser_errposition(pstate, pref->location))); if paramno <= 0 then "there is no parameter $%d" makes sense to me. but, if paramno > 0 why not just say, we can only allow MaxAllocSize / sizeof(Oid) number of parameters.