v3-0001-refactor-CreateStatsStmt.patch

From f11f1c97ef4a18a31d5442e4e745a530caa02d07 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 4 Sep 2025 12:52:13 +0800
Subject: [PATCH v3 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 reduces redundant error
checks.
The logic is now more intuitive: 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 | 17 ++++++++++++++---
 src/backend/commands/tablecmds.c |  2 +-
 src/backend/tcop/utility.c       | 30 +-----------------------------
 src/include/commands/defrem.h    |  2 +-
 4 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index e24d540cd45..100eb82db64 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,11 @@ compare_int16(const void *a, const void *b)
 
 /*
  *		CREATE STATISTICS
+ * Ensure queryString is not NULL when we need do parse analysis for stmt!
+ * stmt changed on the go, see transformStatsStmt.
  */
 ObjectAddress
-CreateStatistics(CreateStatsStmt *stmt)
+CreateStatistics(CreateStatsStmt *stmt, const char *queryString)
 {
 	int16		attnums[STATS_MAX_DIMENSIONS];
 	int			nattnums = 0;
@@ -111,8 +114,9 @@ CreateStatistics(CreateStatsStmt *stmt)
 
 		if (!IsA(rln, RangeVar))
 			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("only a single relation is allowed in CREATE STATISTICS")));
+					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("cannot create statistics on the specified relation"),
+					errdetail("CREATE STATISTICS only supports tables, foreign tables and materialized views."));
 
 		/*
 		 * CREATE STATISTICS will influence future execution plans but does
@@ -150,6 +154,13 @@ CreateStatistics(CreateStatsStmt *stmt)
 	Assert(rel);
 	relid = RelationGetRelid(rel);
 
+	if (queryString != NULL)
+	{
+		/* Run parse analysis ... */
+		(void) 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 082a3575d62..b8dd9306b93 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9654,7 +9654,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 5f442bc3bd4..027356f6e77 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1866,35 +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("cannot create statistics on the specified relation"),
-								 errdetail("CREATE STATISTICS only supports tables, foreign tables and materialized views.")));
-
-					/*
-					 * 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