Hello Sami and Sirisha,
Thank You very much for the patches.
I tested the v2 series on with a clean meson build (cassert enabled). Both
patches apply cleanly to master
(92819e57945) and the full test suite passes: 362 OK, 0 failures.
Your example normalizes as described:
WAIT FOR LSN 'FFFFFFFF/FFFFFFFF' WITH (MODE 'primary_flush', TIMEOUT
'1ms', NO_THROW);
WAIT FOR LSN 'FFFFFFFE/FFFFFFFF' WITH (MODE 'primary_flush', TIMEOUT
'2ms', NO_THROW);
calls | query
-------+------------------------------------------------------
2 | WAIT FOR LSN $1 WITH (MODE $2, TIMEOUT $3, NO_THROW)
A few other cases I checked, all behaving sensibly:
- TIMEOUT 1000 and TIMEOUT '1ms' normalize to the same entry despite
the different literal types.
- Different option order or a different number of options produce
separate entries, which seems right since the query text differs.
- Argument-less options (NO_THROW) are fine: arg_location stays -1 and
RecordConstLocation() already ignores negative locations.
- VACUUM/EXPLAIN/COPY still record their option values literally, as
expected since 0001 only tracks arg_location without changing
jumbling for statements that haven't opted in.
Agreed the helper looks reusable for VACUUM and ALTER ROLE later.
Regards,
Rithvika Devisetti
On Sat, Aug 29, 2026 at 6:03 PM Sami Imseih <[email protected]> wrote:
> Hi,
>
> > The query jumbling facilities should be used to handle this.
>
> +1 for this. I think we need to do a bit more than what is suggested
> in v1-0001, which only normalizes the target LSN. I think we should
> also look at the rest of the rest of the WAIT FOR syntax and normalize
> option values, for example
>
> ```
> WAIT FOR LSN 'FFFFFFFF/FFFFFFFF'
> WITH (MODE 'primary_flush', TIMEOUT '1ms', NO_THROW);
>
> WAIT FOR LSN 'FFFFFFFE/FFFFFFFF'
> WITH (MODE 'primary_flush', TIMEOUT '2ms', NO_THROW);
> ```
>
> These should normalize to one pg_stat_statements entry
>
> ```
> WAIT FOR LSN $1 WITH (MODE $2, TIMEOUT $3, NO_THROW)
> ```
>
> Because these options are carried as DefElem, I think we should also
> track DefElem arg_location, and then statement parse nodes with such
> DefElem option lists can use pg_node_attr(custom_query_jumble) to
> traverse those lists and normalize the option values.
>
> WAIT FOR is one case, but I think the same approach could also be
> useful for other utility statements such as VACUUM and ALTER ROLE. For
> example, ALTER ROLE could normalize PASSWORD and VALID UNTIL. I have
> kept this series focused on WAIT FOR for now, though.
>
> So, attached in v2, v2-0001 adds the DefElem arg_location tracking,
> and v2-0002 adds the WAIT FOR jumbling changes.
> JumbleDefElemOptions() is a small helper in queryjumblefuncs.c that
> other statements can use to implement the same kind of option
> jumbling.
>
> CC'ing Michael also to get his thoughts on the approach.
>
> --
> Sami Imseih
> Amazon Web Services (AWS)
>
