v5-0001-refactor-CreateStatsStmt.patch

From 929e3d9a1bcf36937b1f362963f78595bfd47f88 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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+5QdftwUNQMu5h7OgjmqDwQ@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