Re: Jumble the CALL command in pg_stat_statements

From: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)fr>
Subject: Re: Jumble the CALL command in pg_stat_statements
Date: 2023-09-13 23:09:19
Message-ID: C27A2406-992E-4E3B-8FC7-A4C8242AF260@amazon.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Still this grouping is much better than having thousands of entries
> with different values. I am not sure if we should bother improving
> that more than what you suggest that, especially as FuncExpr->args can
> itself include Const nodes as far as I recall.

I agree.

> As far as the SET command mentioned in [1] is concerned, it is a bit more complex
> as it requires us to deal with A_Constants which is not very straightforward. We can surely
> deal with SET currently by applying custom query jumbling logic to VariableSetStmt,
> but this can be dealt with in a separate discussion.

> As VariableSetStmt is the top-most node structure for SET/RESET
> commands, using a custom implementation may be wise in this case,

I do have a patch for this with test cases, 0001-v1-Jumble-the-SET-command.patch
If you feel this needs a separate discussion I can start one.

In the patch, the custom _jumbleVariableSetStmt jumbles
the kind, name, is_local and number of arguments ( in case of a list )
and tracks the locations for normalization.

> This choice is a bit surprising. How does it influence the jumbling?
> For example, if I add a query_jumble_ignore to it, the regression
> tests of pg_stat_statements still pass. This is going to require more
> test coverage to prove that this addition is useful.

CALL with OUT or INOUT args is a bit strange, because
as the doc [1] mentions "Arguments must be supplied for all procedure parameters
that lack defaults, including OUT parameters. However, arguments
matching OUT parameters are not evaluated, so it's customary
to just write NULL for them."

so for pgss, passing a NULL or some other value into OUT/INOUT args should
be normalized like IN args.

0001-v2-Jumble-the-CALL-command-in-pg_stat_statements.patch adds
these test cases.

Regards,

Sami Imseih
Amazon Web Services (AWS)

[1] https://www.postgresql.org/docs/current/sql-call.html

Attachment Content-Type Size
0001-v1-Jumble-the-SET-command.patch application/octet-stream 9.7 KB
0001-v2-Jumble-the-CALL-command-in-pg_stat_statements.patch application/octet-stream 4.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2023-09-13 23:39:05 Re: Redundant Unique plan node for table with a unique index
Previous Message Nathan Bossart 2023-09-13 22:47:53 Re: Inefficiency in parallel pg_restore with many tables