Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt - Mailing list pgsql-hackers

On Fri, Oct 17, 2025 at 8:13 PM Álvaro Herrera <alvherre@kurilemu.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.