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"
