Re: Set query_id for query contained in utility statement

Hi,

On Fri, Oct 11, 2024 at 2:39 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> in transformSelectStmt we do
> makeNode(Query), assign Query's stmt_len, stmt_location value.
> if in transformSelectStmt we did it wrong, then
> transformTopLevelStmt
>
> result->stmt_location = parseTree->stmt_location;
> result->stmt_len = parseTree->stmt_len;
>
> will override values we've previously assigned at transformSelectStmt.
>
> this feel not safe?

Good point. There are multiple spots in the call tree where the
location/length was set which is definitely confusing. I've updated
the patch to always set the location/length within the transformStmt
calls, near the creation of the query nodes.
This means that transformSelectStmt was only doing a call
transformOptionalSelectInto and was mostly unnecessary. I've replaced
transformSelectStmt calls by direct calls to
transformOptionalSelectInto.

> +qry->stmt_len = pstate->p_stmt_len - (stmt->location -
> pstate->p_stmt_location);
> i feel like, the comments didn't explain very well the difference between
> stmt->location and pstate->p_stmt_location.
> i know it's related to multi-statement, possibly through \;

I've added more details to the comments.

> SELECT pg_stat_statements_reset() IS NOT NULL AS t;
> explain (values (1));
> explain ((values (1)));
> explain table tenk1;
> explain ((table tenk1));
> SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query
> COLLATE "C", toplevel;
> final output:
>
> toplevel | calls | query
> ----------+-------+--------------------------------------------------------------------------------------------
> t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
> t | 0 | SELECT toplevel, calls, query FROM
> pg_stat_statements ORDER BY query COLLATE "C", toplevel
> t | 2 | explain (values ($1))
> f | 2 | explain (values ($1));
> f | 2 | explain table tenk1
> t | 2 | explain table tenk1
>
> I already mentioned this at the end of [1].
> Can you try to also normalize these cases, since we've normalized the
> nested select query in explain statement.

I've missed that, thanks for the reminder. Which made me realise that
the TABLE command was also not handled. I've added both TABLE and
VALUES in the tests and they should now report correctly when nested.