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