On Fri, Aug 29, 2025 at 5:46 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > =?utf-8?Q?=C3=81lvaro?= Herrera <alvhe...@kurilemu.de> writes: > > I propose to use this report: > > > ERROR: cannot create statistics on specified relation > > DETAIL: CREATE STATISTICS only supports tables, materialized views, > > foreign tables, and partitioned tables. > > WFM, although I think you could shorten it to "tables, materialized > views, and foreign tables". We generally expect that partitioned > tables are included when saying "tables", no? I'm not dead set on > that either way, though. >
https://www.postgresql.org/docs/current/sql-copy.html use "COPY TO can be used only with plain tables, not views, and does not copy rows from child tables or child partitions" > > Alternatively, maybe the right fix is to move the parse-analysis > work into CreateStatistics altogether. I think there is not a > very good argument for ProcessUtilitySlow doing all that stuff. > It's supposed to mainly just be a dispatching switch(), IMO. > seems doable. transformStatsStmt, CreateStatistics both used only twice, refactoring arguments should be fine. please check the attached POC, regress tests also added. main idea is first check CreateStatsStmt->relations, then call transformStatsStmt, transformStatsStmt only to transform CreateStatsStmt->exprs. also the above complaint about the relation lock issue will be resolved.
From 04975c4b93e60e2554b3d247d9bc38b5385530a3 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Fri, 29 Aug 2025 12:03:49 +0800 Subject: [PATCH v2 1/1] refactor CreateStatsStmt discussion: https://postgr.es/m/ --- src/backend/commands/statscmds.c | 14 ++++++++++++-- src/backend/commands/tablecmds.c | 2 +- src/backend/tcop/utility.c | 25 +------------------------ src/include/commands/defrem.h | 2 +- src/test/regress/expected/stats_ext.out | 20 ++++++++++++++++++++ src/test/regress/sql/stats_ext.sql | 12 ++++++++++++ 6 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index e24d540cd45..094d1a4fc1b 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, espepcially transformStatsStmt. */ ObjectAddress -CreateStatistics(CreateStatsStmt *stmt) +CreateStatistics(CreateStatsStmt *stmt, const char *queryString) { int16 attnums[STATS_MAX_DIMENSIONS]; int nattnums = 0; @@ -112,7 +115,8 @@ 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("cannot create statistics on specified relation"), + errdetail("CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables"))); /* * CREATE STATISTICS will influence future execution plans but does @@ -150,6 +154,12 @@ CreateStatistics(CreateStatsStmt *stmt) Assert(rel); relid = RelationGetRelid(rel); + if (queryString != NULL && !stmt->transformed) + { + /* Run parse analysis ... */ + (void) transformStatsStmt(relid, stmt, queryString); + } + /* * 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 4f4191b0ea6..8d35ae971d5 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1867,32 +1867,9 @@ ProcessUtilitySlow(ParseState *pstate, 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("only a single relation is allowed in CREATE STATISTICS"))); - - /* - * 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(stmt, queryString); } break; 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); diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index c66e09f8b16..fb920312b4d 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -88,6 +88,26 @@ ERROR: statistics creation on system columns is not supported -- statistics without a less-than operator not supported CREATE STATISTICS tst (ndistinct) ON w from ext_stats_test1; ERROR: column "w" cannot be used in statistics because its type xid has no default btree operator class +CREATE STATISTICS tst ON (x is not null) FROM ext_stats_test s, lateral(values(s.x)); +ERROR: only a single relation is allowed in CREATE STATISTICS +CREATE STATISTICS tst ON (x is not null) FROM (SELECT * FROM ext_stats_test); +ERROR: cannot create statistics on specified relation +DETAIL: CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables +CREATE STATISTICS tst ON (x is not null) FROM ext_stats_test s TABLESAMPLE system (x) REPEATABLE (200); +ERROR: cannot create statistics on specified relation +DETAIL: CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables +CREATE STATISTICS tst ON (x is not null) FROM JSON_TABLE(jsonb '123', '$' COLUMNS (item int PATH '$')); +ERROR: cannot create statistics on specified relation +DETAIL: CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables +CREATE FUNCTION tftest(int) returns table(a int, b int) as $$ +BEGIN + RETURN QUERY SELECT $1, $1+i FROM generate_series(1,5) g(i); +END; +$$ LANGUAGE plpgsql IMMUTABLE STRICT; +CREATE STATISTICS alt_stat2 ON a, b FROM tftest(1); +ERROR: cannot create statistics on specified relation +DETAIL: CREATE STATISTICS only supports tables, materialized views, foreign tables, and partitioned tables +DROP FUNCTION tftest; DROP TABLE ext_stats_test1; -- Ensure stats are dropped sanely, and test IF NOT EXISTS while at it CREATE TABLE ab1 (a INTEGER, b INTEGER, c INTEGER); diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index 9ce4c670ecb..f63048f4c96 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -58,6 +58,18 @@ CREATE STATISTICS tst on (tableoid::int+1) from ext_stats_test1; CREATE STATISTICS tst (ndistinct) ON xmin from ext_stats_test1; -- statistics without a less-than operator not supported CREATE STATISTICS tst (ndistinct) ON w from ext_stats_test1; + +CREATE STATISTICS tst ON (x is not null) FROM ext_stats_test s, lateral(values(s.x)); +CREATE STATISTICS tst ON (x is not null) FROM (SELECT * FROM ext_stats_test); +CREATE STATISTICS tst ON (x is not null) FROM ext_stats_test s TABLESAMPLE system (x) REPEATABLE (200); +CREATE STATISTICS tst ON (x is not null) FROM JSON_TABLE(jsonb '123', '$' COLUMNS (item int PATH '$')); +CREATE FUNCTION tftest(int) returns table(a int, b int) as $$ +BEGIN + RETURN QUERY SELECT $1, $1+i FROM generate_series(1,5) g(i); +END; +$$ LANGUAGE plpgsql IMMUTABLE STRICT; +CREATE STATISTICS alt_stat2 ON a, b FROM tftest(1); +DROP FUNCTION tftest; DROP TABLE ext_stats_test1; -- Ensure stats are dropped sanely, and test IF NOT EXISTS while at it -- 2.34.1
