Re: WAIT FOR command should do some query jumbling

From: Rithvika Devisetti <devisettirithvika(at)gmail(dot)com>
To: Sami Imseih <samimseih(dot)pg(at)gmail(dot)com>
Cc: sirisha chamarthi <sirichamarthi22(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alexander Korotkov <akorotkov(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: WAIT FOR command should do some query jumbling
Date: 2026-08-30 20:35:08
Message-ID: CA+HR5vinS4cFSgM0pABedU2iDWsHiBTYJMAUux2-6cLP-PpkLQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 <samimseih(dot)pg(at)gmail(dot)com> 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)
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shihao zhong 2026-08-30 20:45:50 Re: Commitfest manager for September 2026
Previous Message Daniel Gustafsson 2026-08-30 20:15:27 Re: Changing the state of data checksums in a running cluster