Hi,

On Thu, Dec 18, 2025 at 06:44:17PM -0600, Sami Imseih wrote:
> For the second point, since JumbleState can be shared by multiple
> extensions, hooks should receive it as a const pointer. This
> signals read-only intent and prevents extensions from
> accidentally modifying it through the hook. This change is in
> 0002. This does mean that extensions will need to update their
> hooks, but I do not see that as an issue for a major version.

I can see that 0001 is doing:

-static void fill_in_constant_lengths(JumbleState *jstate, const char *query,

and then:

+static void
+fill_in_constant_lengths(const JumbleState *jstate, const char *query,

Should the const addition be in 0002? But...

While looking at fill_in_constant_lengths(), I can see that it is doing:

locs = jstate->clocations;

and then things like:

locs[i].length = -1

or

locs[i].length = strlen(yyextra.scanbuf + loc)

While this is technically correct so the compiler does not complain (because
clocations is a non const pointer in JumbleState and the added const does not
apply to what clocations points to), I think that adding const here is 
misleading.

Indeed, the function clearly modifies data accessible through the parameter.

Thoughts?

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Reply via email to