hi. moving all code in ProcessUtilitySlow (case T_CreateStatsStmt:) to CreateStatistic seems more intuitive to me.
so I rebased v3.
From 929e3d9a1bcf36937b1f362963f78595bfd47f88 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Thu, 2 Oct 2025 11:24:40 +0800 Subject: [PATCH v5 1/1] refactor CreateStatsStmt move all CreateStatsStmt logic into CreateStatistics. Previously, CreateStatistics and ProcessUtilitySlow opened the same relation twice with a ShareUpdateExclusiveLock. This refactor ensures the relation is opened with ShareUpdateExclusiveLock only once and also remove redundant error checks. in function: CreateStatsStmt we first error checking CreateStatsStmt->relations and then call transformStatsStmt to parse analysis CreateStatsStmt->exprs. discussion: https://postgr.es/m/CACJufxFcvo=fM6J9RC88n1U=es9+5qdftwunqmu5h7ogjmq...@mail.gmail.com --- src/backend/commands/statscmds.c | 14 ++++++++++++-- src/backend/commands/tablecmds.c | 2 +- src/backend/tcop/utility.c | 29 +---------------------------- src/include/commands/defrem.h | 2 +- 4 files changed, 15 insertions(+), 32 deletions(-) diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index e24d540cd45..400bb293fa4 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -29,6 +29,7 @@ #include "miscadmin.h" #include "nodes/nodeFuncs.h" #include "optimizer/optimizer.h" +#include "parser/parse_utilcmd.h" #include "statistics/statistics.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -57,9 +58,12 @@ compare_int16(const void *a, const void *b) /* * CREATE STATISTICS + * queryString is the source text of the CREATE STATISTICS command; + * It must be supplied if the CreateStatsStmt not being transformed + * (run transformStatsStmt), else it can be NULL. */ ObjectAddress -CreateStatistics(CreateStatsStmt *stmt) +CreateStatistics(CreateStatsStmt *stmt, const char *queryString) { int16 attnums[STATS_MAX_DIMENSIONS]; int nattnums = 0; @@ -112,7 +116,7 @@ CreateStatistics(CreateStatsStmt *stmt) if (!IsA(rln, RangeVar)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("only a single relation is allowed in CREATE STATISTICS"))); + errmsg("CREATE STATISTICS only supports relation names in the FROM clause"))); /* * CREATE STATISTICS will influence future execution plans but does @@ -150,6 +154,12 @@ CreateStatistics(CreateStatsStmt *stmt) Assert(rel); relid = RelationGetRelid(rel); + /* Run parse analysis ... */ + if (queryString != NULL) + stmt = transformStatsStmt(relid, stmt, queryString); + + Assert(stmt->transformed); + /* * If the node has a name, split it up and determine creation namespace. * If not, put the object in the same namespace as the relation, and cons diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc89352b661..19c11970787 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9658,7 +9658,7 @@ ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, /* The CreateStatsStmt has already been through transformStatsStmt */ Assert(stmt->transformed); - address = CreateStatistics(stmt); + address = CreateStatistics(stmt, NULL); return address; } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 918db53dd5e..027356f6e77 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1866,34 +1866,7 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreateStatsStmt: - { - Oid relid; - CreateStatsStmt *stmt = (CreateStatsStmt *) parsetree; - RangeVar *rel = (RangeVar *) linitial(stmt->relations); - - if (!IsA(rel, RangeVar)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("CREATE STATISTICS only supports relation names in the FROM clause"))); - - /* - * CREATE STATISTICS will influence future execution plans - * but does not interfere with currently executing plans. - * So it should be enough to take ShareUpdateExclusiveLock - * on relation, conflicting with ANALYZE and other DDL - * that sets statistical information, but not with normal - * queries. - * - * XXX RangeVarCallbackOwnsRelation not needed here, to - * keep the same behavior as before. - */ - relid = RangeVarGetRelid(rel, ShareUpdateExclusiveLock, false); - - /* Run parse analysis ... */ - stmt = transformStatsStmt(relid, stmt, queryString); - - address = CreateStatistics(stmt); - } + address = CreateStatistics((CreateStatsStmt *) parsetree, queryString); break; case T_AlterStatsStmt: diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index dd22b5efdfd..684f62109fd 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -85,7 +85,7 @@ extern void RemoveOperatorById(Oid operOid); extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt); /* commands/statscmds.c */ -extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt); +extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt, const char *queryString); extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt); extern void RemoveStatisticsById(Oid statsOid); extern void RemoveStatisticsDataById(Oid statsOid, bool inh); -- 2.34.1
