Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1s84NF-009RTN-Pl for pgsql-bugs@arkaria.postgresql.org; Fri, 17 May 2024 20:40:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1s84MH-000CPY-GB for pgsql-bugs@arkaria.postgresql.org; Fri, 17 May 2024 20:39:17 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1s84MH-000CPQ-6n for pgsql-bugs@lists.postgresql.org; Fri, 17 May 2024 20:39:17 +0000 Received: from sss.pgh.pa.us ([68.162.161.243]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1s84M9-000lsh-2P for pgsql-bugs@lists.postgresql.org; Fri, 17 May 2024 20:39:16 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 44HKd57b2548235; Fri, 17 May 2024 16:39:05 -0400 From: Tom Lane To: Tender Wang cc: exclusion@gmail.com, pgsql-bugs@lists.postgresql.org, Tomas Vondra Subject: Re: BUG #18468: CREATE TABLE ... LIKE leaves orphaned column reference in extended statistics In-reply-to: <2135832.1715885118@sss.pgh.pa.us> References: <18468-f5add190e3fa5902@postgresql.org> <2135832.1715885118@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Thu, 16 May 2024 14:45:18 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2548204.1715978322.0@sss.pgh.pa.us> Date: Fri, 17 May 2024 16:39:05 -0400 Message-ID: <2548234.1715978345@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2548204.1715978322.1@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 ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="fix-bug-18468.patch"; charset="us-ascii" Content-ID: <2548204.1715978322.2@sss.pgh.pa.us> Content-Description: fix-bug-18468.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse= _utilcmd.c index 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(CreateStmtConte= xt *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 *quer= yString) cxt.fkconstraints =3D NIL; cxt.ixconstraints =3D NIL; cxt.likeclauses =3D NIL; - cxt.extstats =3D NIL; cxt.blist =3D NIL; cxt.alist =3D NIL; cxt.pkey =3D NULL; @@ -339,11 +338,6 @@ transformCreateStmt(CreateStmt *stmt, const char *que= ryString) */ transformCheckConstraints(&cxt, !cxt.isforeign); = - /* - * Postprocess extended statistics. - */ - transformExtendedStatistics(&cxt); - /* * Output results. */ @@ -1111,61 +1105,25 @@ transformTableLikeClause(CreateStmtContext *cxt, T= ableLikeClause *table_like_cla } = /* - * We cannot yet deal with defaults, CHECK constraints, or indexes, sinc= e - * 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 tha= t - * 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 =3D RelationGetRelid(relation); cxt->likeclauses =3D lappend(cxt->likeclauses, table_like_clause); } = - /* - * We may copy extended statistics if requested, since the representatio= n - * of CreateStatsStmt doesn't depend on column numbers. - */ - if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS) - { - List *parent_extstats; - ListCell *l; - - parent_extstats =3D RelationGetStatExtList(relation); - - foreach(l, parent_extstats) - { - Oid parent_stat_oid =3D lfirst_oid(l); - CreateStatsStmt *stats_stmt; - - stats_stmt =3D 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 =3D 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 =3D comment; - } - - cxt->extstats =3D 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, TableLikeC= lause *table_like_clause) } } = + /* + * Process extended statistics if required. + */ + if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS) + { + List *parent_extstats; + ListCell *l; + + parent_extstats =3D RelationGetStatExtList(relation); + + foreach(l, parent_extstats) + { + Oid parent_stat_oid =3D lfirst_oid(l); + CreateStatsStmt *stats_stmt; + + stats_stmt =3D 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 =3D 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 =3D comment; + } + + result =3D lappend(result, stats_stmt); + } + + list_free(parent_extstats); + } + /* Done with child rel */ table_close(childrel, NoLock); = @@ -1837,10 +1833,12 @@ generateClonedIndexStmt(RangeVar *heapRel, Relatio= n source_idx, * Generate a CreateStatsStmt node using information from an already exis= ting * 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 =3D (Node *) lfirst(lc); StatsElem *selem =3D makeNode(StatsElem); + bool found_whole_row; + + /* Adjust Vars to match new table's column numbering */ + expr =3D map_variable_attnos(expr, + 1, 0, + attmap, + InvalidOid, + &found_whole_row); = selem->name =3D NULL; - selem->expr =3D (Node *) lfirst(lc); + selem->expr =3D expr; = def_names =3D lappend(def_names, selem); } @@ -2652,19 +2659,6 @@ transformIndexConstraint(Constraint *constraint, Cr= eateStmtContext *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 =3D list_concat(cxt->alist, cxt->extstats); -} - /* * transformCheckConstraints * handle CHECK constraints @@ -3457,7 +3451,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, cxt.fkconstraints =3D NIL; cxt.ixconstraints =3D NIL; cxt.likeclauses =3D NIL; - cxt.extstats =3D NIL; cxt.blist =3D NIL; cxt.alist =3D NIL; cxt.pkey =3D NULL; @@ -3763,9 +3756,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, newcmds =3D 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/re= gress/expected/create_table_like.out index 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 tar= get | 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_li= ke_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); /* copie= s 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.sql index 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 A= LL) 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_li= ke_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); /* copie= s indexes */ INSERT INTO inhg VALUES (5, 10); ------- =_aaaaaaaaaa0--