Re: Jumble the CALL command in pg_stat_statements

On Wed, Sep 13, 2023 at 11:09:19PM +0000, Imseih (AWS), Sami wrote:
> 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.

Agreed tha tthis should have its own thread with a proper subject.

> 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.

There is much more going on here, like FunctionSetResetClause, or
AlterSystemStmt with its generic_reset.

+        foreach    (l, expr->args)
+        {
+            A_Const *ac = (A_Const *) lfirst(l);
+
+            if(ac->type != T_String)
+                RecordConstLocation(jstate, ac->location);
+        }

Even this part, I am not sure if it is always correct.  Couldn't we
have cases where String's A_Const had better be recorded as const?

> 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.

I've been studying this one, and I can see why you're right here.
This feels much more natural to include.  The INOUT parameters get
registered twice at the same position, and the duplicates are
discarded by pg_stat_statements, which is OK.  The patch is straight
for the CALL part, so I have applied it.
--
Michael