From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tender Wang <tndrwang@gmail.com>
Cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org, Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: Re: BUG #18468: CREATE TABLE ... LIKE leaves orphaned column reference in extended statistics
Date: Fri, 17 May 2024 16:39:05 -0400
Message-ID: <2548234.1715978345@sss.pgh.pa.us> (raw)
In-Reply-To: <2135832.1715885118@sss.pgh.pa.us>
References: <18468-f5add190e3fa5902@postgresql.org>
<CAHewXNnbUZ1=ezBPtuc6iv8kwzJBsPXkv=XPwrEYg-CXerPEFQ@mail.gmail.com>
<CAHewXNkng5jG472hh+icVT_g8BT5Gv4tNFjBP_X5cK1aqiH=ag@mail.gmail.com>
<2135832.1715885118@sss.pgh.pa.us>
I wrote:
> We don't know the column mapping there, either. What we need to have
> our hands on is the "attmap" computed in expandTableLikeClause, and
> then we can pass the stats expressions through map_variable_attnos().
> I think this might not need to be a really large patch, but it
> definitely has to change where generateClonedExtStatsStmt is
> called from. I can have a go at it if Tomas doesn't want to.
Yeah, this doesn't require a whole lot more than moving the
processing in transformTableLikeClause to expandTableLikeClause.
I chose to get rid of transformExtendedStatistics, because not
only is there nothing for it to do, there will never be any
statements for it to do nothing to.
I plan to sit on this till after the beta1 freeze.
regards, tom lane
Attachments:fix-bug-18468.patch (text/x-diff, 10.2K) downloaddiff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.cindex 639cfa443e..d5c2b2ff0b 100644--- a/src/backend/parser/parse_utilcmd.c+++ b/src/backend/parser/parse_utilcmd.c@@ -87,7 +87,6 @@ typedef struct
List *fkconstraints; /* FOREIGN KEY constraints */
List *ixconstraints; /* index-creating constraints */
List *likeclauses; /* LIKE clauses that need post-processing */
- List *extstats; /* cloned extended statistics */
List *blist; /* "before list" of things to do before
* creating the table */
List *alist; /* "after list" of things to do after creating
@@ -120,13 +119,14 @@ static void transformTableLikeClause(CreateStmtContext *cxt,
static void transformOfType(CreateStmtContext *cxt,
TypeName *ofTypename);
static CreateStatsStmt *generateClonedExtStatsStmt(RangeVar *heapRel,
- Oid heapRelid, Oid source_statsid);+ Oid heapRelid,+ Oid source_statsid,+ const AttrMap *attmap);
static List *get_collation(Oid collation, Oid actual_datatype);
static List *get_opclass(Oid opclass, Oid actual_datatype);
static void transformIndexConstraints(CreateStmtContext *cxt);
static IndexStmt *transformIndexConstraint(Constraint *constraint,
CreateStmtContext *cxt);
-static void transformExtendedStatistics(CreateStmtContext *cxt);
static void transformFKConstraints(CreateStmtContext *cxt,
bool skipValidation,
bool isAddConstraint);
@@ -246,7 +246,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL;
cxt.likeclauses = NIL;
- cxt.extstats = NIL;
cxt.blist = NIL;
cxt.alist = NIL;
cxt.pkey = NULL;
@@ -339,11 +338,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
*/
transformCheckConstraints(&cxt, !cxt.isforeign);
- /*- * Postprocess extended statistics.- */- transformExtendedStatistics(&cxt);-
/*
* Output results.
*/
@@ -1111,61 +1105,25 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
}
/*
- * We cannot yet deal with defaults, CHECK constraints, or indexes, since- * we don't yet know what column numbers the copied columns will have in- * the finished table. If any of those options are specified, add the- * LIKE clause to cxt->likeclauses so that expandTableLikeClause will be- * called after we do know that. Also, remember the relation OID so that- * expandTableLikeClause is certain to open the same table.+ * We cannot yet deal with defaults, CHECK constraints, indexes, or+ * statistics, since we don't yet know what column numbers the copied+ * columns will have in the finished table. If any of those options are+ * specified, add the LIKE clause to cxt->likeclauses so that+ * expandTableLikeClause will be called after we do know that. Also,+ * remember the relation OID so that expandTableLikeClause is certain to+ * open the same table.
*/
if (table_like_clause->options &
(CREATE_TABLE_LIKE_DEFAULTS |
CREATE_TABLE_LIKE_GENERATED |
CREATE_TABLE_LIKE_CONSTRAINTS |
- CREATE_TABLE_LIKE_INDEXES))+ CREATE_TABLE_LIKE_INDEXES |+ CREATE_TABLE_LIKE_STATISTICS))
{
table_like_clause->relationOid = RelationGetRelid(relation);
cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
}
- /*- * We may copy extended statistics if requested, since the representation- * of CreateStatsStmt doesn't depend on column numbers.- */- if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS)- {- List *parent_extstats;- ListCell *l;-- parent_extstats = RelationGetStatExtList(relation);-- foreach(l, parent_extstats)- {- Oid parent_stat_oid = lfirst_oid(l);- CreateStatsStmt *stats_stmt;-- stats_stmt = generateClonedExtStatsStmt(cxt->relation,- RelationGetRelid(relation),- parent_stat_oid);-- /* Copy comment on statistics object, if requested */- if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)- {- comment = GetComment(parent_stat_oid, StatisticExtRelationId, 0);-- /*- * We make use of CreateStatsStmt's stxcomment option, so as- * not to need to know now what name the statistics will have.- */- stats_stmt->stxcomment = comment;- }-- cxt->extstats = lappend(cxt->extstats, stats_stmt);- }-- list_free(parent_extstats);- }-
/*
* Close the parent rel, but keep our AccessShareLock on it until xact
* commit. That will prevent someone else from deleting or ALTERing the
@@ -1423,6 +1381,44 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
}
}
+ /*+ * Process extended statistics if required.+ */+ if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS)+ {+ List *parent_extstats;+ ListCell *l;++ parent_extstats = RelationGetStatExtList(relation);++ foreach(l, parent_extstats)+ {+ Oid parent_stat_oid = lfirst_oid(l);+ CreateStatsStmt *stats_stmt;++ stats_stmt = generateClonedExtStatsStmt(heapRel,+ RelationGetRelid(childrel),+ parent_stat_oid,+ attmap);++ /* Copy comment on statistics object, if requested */+ if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)+ {+ comment = GetComment(parent_stat_oid, StatisticExtRelationId, 0);++ /*+ * We make use of CreateStatsStmt's stxcomment option, so as+ * not to need to know now what name the statistics will have.+ */+ stats_stmt->stxcomment = comment;+ }++ result = lappend(result, stats_stmt);+ }++ list_free(parent_extstats);+ }+
/* Done with child rel */
table_close(childrel, NoLock);
@@ -1837,10 +1833,12 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
* Generate a CreateStatsStmt node using information from an already existing
* extended statistic "source_statsid", for the rel identified by heapRel and
* heapRelid.
+ *+ * Attribute numbers in expression Vars are adjusted according to attmap.
*/
static CreateStatsStmt *
generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
- Oid source_statsid)+ Oid source_statsid, const AttrMap *attmap)
{
HeapTuple ht_stats;
Form_pg_statistic_ext statsrec;
@@ -1923,10 +1921,19 @@ generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
foreach(lc, exprs)
{
+ Node *expr = (Node *) lfirst(lc);
StatsElem *selem = makeNode(StatsElem);
+ bool found_whole_row;++ /* Adjust Vars to match new table's column numbering */+ expr = map_variable_attnos(expr,+ 1, 0,+ attmap,+ InvalidOid,+ &found_whole_row);
selem->name = NULL;
- selem->expr = (Node *) lfirst(lc);+ selem->expr = expr;
def_names = lappend(def_names, selem);
}
@@ -2652,19 +2659,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
return index;
}
-/*- * transformExtendedStatistics- * Handle extended statistic objects- *- * Right now, there's nothing to do here, so we just append the list to- * the existing "after" list.- */-static void-transformExtendedStatistics(CreateStmtContext *cxt)-{- cxt->alist = list_concat(cxt->alist, cxt->extstats);-}-
/*
* transformCheckConstraints
* handle CHECK constraints
@@ -3457,7 +3451,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL;
cxt.likeclauses = NIL;
- cxt.extstats = NIL;
cxt.blist = NIL;
cxt.alist = NIL;
cxt.pkey = NULL;
@@ -3763,9 +3756,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
newcmds = lappend(newcmds, newcmd);
}
- /* Append extended statistics objects */- transformExtendedStatistics(&cxt);-
/* Close rel */
relation_close(rel, NoLock);
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.outindex 0ed94f1d2f..6bfc6d040f 100644--- a/src/test/regress/expected/create_table_like.out+++ b/src/test/regress/expected/create_table_like.out@@ -261,8 +261,23 @@ Check constraints:
Inherits: test_like_5,
test_like_5x
+-- Test updating of column numbers in statistics expressions (bug #18468)+CREATE TABLE test_like_6 (a int, c text, b text);+CREATE STATISTICS ext_stat ON (a || b) FROM test_like_6;+ALTER TABLE test_like_6 DROP COLUMN c;+CREATE TABLE test_like_6c (LIKE test_like_6 INCLUDING ALL);+\d+ test_like_6c+ Table "public.test_like_6c"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+----------+--------------+-------------+ a | integer | | | | plain | | + b | text | | | | extended | | +Statistics objects:+ "public.test_like_6c_expr_stat" ON (a || b) FROM test_like_6c+
DROP TABLE test_like_4, test_like_4a, test_like_4b, test_like_4c, test_like_4d;
DROP TABLE test_like_5, test_like_5x, test_like_5c;
+DROP TABLE test_like_6, test_like_6c;
CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
INSERT INTO inhg VALUES (5, 10);
INSERT INTO inhg VALUES (20, 10); -- should fail
diff --git a/src/test/regress/sql/create_table_like.sql b/src/test/regress/sql/create_table_like.sqlindex 4929d373a2..04008a027b 100644--- a/src/test/regress/sql/create_table_like.sql+++ b/src/test/regress/sql/create_table_like.sql@@ -95,8 +95,16 @@ CREATE TABLE test_like_5c (LIKE test_like_4 INCLUDING ALL)
INHERITS (test_like_5, test_like_5x);
\d test_like_5c
+-- Test updating of column numbers in statistics expressions (bug #18468)+CREATE TABLE test_like_6 (a int, c text, b text);+CREATE STATISTICS ext_stat ON (a || b) FROM test_like_6;+ALTER TABLE test_like_6 DROP COLUMN c;+CREATE TABLE test_like_6c (LIKE test_like_6 INCLUDING ALL);+\d+ test_like_6c+
DROP TABLE test_like_4, test_like_4a, test_like_4b, test_like_4c, test_like_4d;
DROP TABLE test_like_5, test_like_5x, test_like_5c;
+DROP TABLE test_like_6, test_like_6c;
CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
INSERT INTO inhg VALUES (5, 10);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, tndrwang@gmail.com, tomas.vondra@enterprisedb.com
Subject: Re: BUG #18468: CREATE TABLE ... LIKE leaves orphaned column reference in extended statistics
In-Reply-To: <2548234.1715978345@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox