Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt

On Fri, Oct 17, 2025 at 8:13 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> Yeah, this looks good. But I think we can go a little further. Because
> CreateStatistics() now calls transformStatsStmt(), we don't need to do
> the same in ATPostAlterTypeParse(), which allows us to simplify the code
> there. In turn this means we can pass the whole Relation rather than
> merely the OID, so transformStatsStmt doesn't need to open it. I attach
> v4 with those changes. Comments?
>

/*
* transformStatsStmt - parse analysis for CREATE STATISTICS
*
* To avoid race conditions, it's important that this function relies only on
* the passed-in rel (and not on stmt->relation) as the target relation.
*/
CreateStatsStmt *
transformStatsStmt(Relation rel, CreateStatsStmt *stmt, const char *queryString)
{
......
}

the (Relation rel) effectively comes from "stmt->relations", which
conflict with the above
comments.

> For a moment it looked weird to me to pass a Relation to the parse
> analysis routine, but I see that other functions declared in
> parse_utilcmd.h are already doing that.
>
>
> One more thing I noticed is that we're scribbling on the parser node,
> which we can easily avoid by creating a copy of the node in
> transformStatsStmt() before doing any changes. I'm not too clear
> on whether this is really necessary or not. We do it in other places,
> but we haven't done it here for a long while, and nothing seems to break
> because of that.
>
> diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
> index 89c8315117b..7797c707f73 100644
> --- a/src/backend/parser/parse_utilcmd.c
> +++ b/src/backend/parser/parse_utilcmd.c
> @@ -3150,6 +3150,9 @@ transformStatsStmt(Relation rel, CreateStatsStmt *stmt, const char *queryString)
> if (stmt->transformed)
> return stmt;
>
> + /* create a copy to scribble on */
> + stmt = copyObject(stmt);
> +
> /* Set up pstate */
> pstate = make_parsestate(NULL);
> pstate->p_sourcetext = queryString;
>
in src/backend/parser/parse_utilcmd.c, we have only two occurences of
copyObject.