Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt

From: Álvaro Herrera <alvherre(at)kurilemu(dot)de>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Kirill Reshke <reshkekirill(at)gmail(dot)com>
Subject: Re: misleading error message in ProcessUtilitySlow T_CreateStatsStmt
Date: 2025-10-17 12:13:08
Message-ID: 202510171014.mwjtro7sl5i7@alvherre.pgsql
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2025-Oct-02, jian he wrote:

> hi.
>
> moving all code in ProcessUtilitySlow (case T_CreateStatsStmt:)
> to CreateStatistic seems more intuitive to me.
>
> so I rebased v3.

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?

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;

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"

Attachment Content-Type Size
v4-0001-Restructure-CreateStatsStmt-parse-analysis-proces.patch text/x-diff 8.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2025-10-17 12:25:19 Re: ci: Skip minfree file in the cores_backtrace.sh
Previous Message shveta malik 2025-10-17 12:07:18 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart