Fix handling of extended expression statistics in CREATE TABLE LIKE.

transformTableLikeClause believed that it could process extended
statistics immediately because "the representation of CreateStatsStmt
doesn't depend on column numbers".  That was true when extended stats
were first introduced, but it was falsified by the addition of
extended stats on expressions: the parsed expression tree is fed
forward by the LIKE option, and that will contain Vars.  So if the
new table doesn't have attnums identical to the old one's (typically
because there are some dropped columns in the old one), that doesn't
work.  The CREATE goes through, but it emits invalid statistics
objects that will cause problems later.

Fortunately, we already have logic that can adapt expression trees
to the possibly-new column numbering.  To use it, we have to delay
processing of CREATE_TABLE_LIKE_STATISTICS into expandTableLikeClause,
just as for other LIKE options that involve expressions.

Per bug #18468 from Alexander Lakhin.  Back-patch to v14 where
extended statistics on expressions were added.

Discussion: https://postgr.es/m/18468-f5add190e3fa5902@postgresql.org
This commit is contained in:
Tom Lane 2024-05-22 17:54:17 -04:00
parent 4ac385adc5
commit 2f3cfcf767
3 changed files with 86 additions and 73 deletions

View File

@ -84,7 +84,6 @@ typedef struct
List *fkconstraints; /* FOREIGN KEY constraints */ List *fkconstraints; /* FOREIGN KEY constraints */
List *ixconstraints; /* index-creating constraints */ List *ixconstraints; /* index-creating constraints */
List *likeclauses; /* LIKE clauses that need post-processing */ List *likeclauses; /* LIKE clauses that need post-processing */
List *extstats; /* cloned extended statistics */
List *blist; /* "before list" of things to do before List *blist; /* "before list" of things to do before
* creating the table */ * creating the table */
List *alist; /* "after list" of things to do after creating List *alist; /* "after list" of things to do after creating
@ -117,13 +116,14 @@ static void transformTableLikeClause(CreateStmtContext *cxt,
static void transformOfType(CreateStmtContext *cxt, static void transformOfType(CreateStmtContext *cxt,
TypeName *ofTypename); TypeName *ofTypename);
static CreateStatsStmt *generateClonedExtStatsStmt(RangeVar *heapRel, 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_collation(Oid collation, Oid actual_datatype);
static List *get_opclass(Oid opclass, Oid actual_datatype); static List *get_opclass(Oid opclass, Oid actual_datatype);
static void transformIndexConstraints(CreateStmtContext *cxt); static void transformIndexConstraints(CreateStmtContext *cxt);
static IndexStmt *transformIndexConstraint(Constraint *constraint, static IndexStmt *transformIndexConstraint(Constraint *constraint,
CreateStmtContext *cxt); CreateStmtContext *cxt);
static void transformExtendedStatistics(CreateStmtContext *cxt);
static void transformFKConstraints(CreateStmtContext *cxt, static void transformFKConstraints(CreateStmtContext *cxt,
bool skipValidation, bool skipValidation,
bool isAddConstraint); bool isAddConstraint);
@ -243,7 +243,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
cxt.fkconstraints = NIL; cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL; cxt.ixconstraints = NIL;
cxt.likeclauses = NIL; cxt.likeclauses = NIL;
cxt.extstats = NIL;
cxt.blist = NIL; cxt.blist = NIL;
cxt.alist = NIL; cxt.alist = NIL;
cxt.pkey = NULL; cxt.pkey = NULL;
@ -336,11 +335,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
*/ */
transformCheckConstraints(&cxt, !cxt.isforeign); transformCheckConstraints(&cxt, !cxt.isforeign);
/*
* Postprocess extended statistics.
*/
transformExtendedStatistics(&cxt);
/* /*
* Output results. * Output results.
*/ */
@ -1132,61 +1126,25 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
} }
/* /*
* We cannot yet deal with defaults, CHECK constraints, or indexes, since * We cannot yet deal with defaults, CHECK constraints, indexes, or
* we don't yet know what column numbers the copied columns will have in * statistics, since we don't yet know what column numbers the copied
* the finished table. If any of those options are specified, add the * columns will have in the finished table. If any of those options are
* LIKE clause to cxt->likeclauses so that expandTableLikeClause will be * specified, add the LIKE clause to cxt->likeclauses so that
* called after we do know that. Also, remember the relation OID so that * expandTableLikeClause will be called after we do know that. Also,
* expandTableLikeClause is certain to open the same table. * remember the relation OID so that expandTableLikeClause is certain to
* open the same table.
*/ */
if (table_like_clause->options & if (table_like_clause->options &
(CREATE_TABLE_LIKE_DEFAULTS | (CREATE_TABLE_LIKE_DEFAULTS |
CREATE_TABLE_LIKE_GENERATED | CREATE_TABLE_LIKE_GENERATED |
CREATE_TABLE_LIKE_CONSTRAINTS | CREATE_TABLE_LIKE_CONSTRAINTS |
CREATE_TABLE_LIKE_INDEXES)) CREATE_TABLE_LIKE_INDEXES |
CREATE_TABLE_LIKE_STATISTICS))
{ {
table_like_clause->relationOid = RelationGetRelid(relation); table_like_clause->relationOid = RelationGetRelid(relation);
cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause); 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 * Close the parent rel, but keep our AccessShareLock on it until xact
* commit. That will prevent someone else from deleting or ALTERing the * commit. That will prevent someone else from deleting or ALTERing the
@ -1452,6 +1410,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 */ /* Done with child rel */
table_close(childrel, NoLock); table_close(childrel, NoLock);
@ -1886,10 +1882,12 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
* Generate a CreateStatsStmt node using information from an already existing * Generate a CreateStatsStmt node using information from an already existing
* extended statistic "source_statsid", for the rel identified by heapRel and * extended statistic "source_statsid", for the rel identified by heapRel and
* heapRelid. * heapRelid.
*
* Attribute numbers in expression Vars are adjusted according to attmap.
*/ */
static CreateStatsStmt * static CreateStatsStmt *
generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid, generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
Oid source_statsid) Oid source_statsid, const AttrMap *attmap)
{ {
HeapTuple ht_stats; HeapTuple ht_stats;
Form_pg_statistic_ext statsrec; Form_pg_statistic_ext statsrec;
@ -1973,10 +1971,19 @@ generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
foreach(lc, exprs) foreach(lc, exprs)
{ {
Node *expr = (Node *) lfirst(lc);
StatsElem *selem = makeNode(StatsElem); 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->name = NULL;
selem->expr = (Node *) lfirst(lc); selem->expr = expr;
def_names = lappend(def_names, selem); def_names = lappend(def_names, selem);
} }
@ -2703,19 +2710,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
return index; 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 * transformCheckConstraints
* handle CHECK constraints * handle CHECK constraints
@ -3358,7 +3352,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
cxt.fkconstraints = NIL; cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL; cxt.ixconstraints = NIL;
cxt.likeclauses = NIL; cxt.likeclauses = NIL;
cxt.extstats = NIL;
cxt.blist = NIL; cxt.blist = NIL;
cxt.alist = NIL; cxt.alist = NIL;
cxt.pkey = NULL; cxt.pkey = NULL;
@ -3640,9 +3633,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
newcmds = lappend(newcmds, newcmd); newcmds = lappend(newcmds, newcmd);
} }
/* Append extended statistics objects */
transformExtendedStatistics(&cxt);
/* Close rel */ /* Close rel */
relation_close(rel, NoLock); relation_close(rel, NoLock);

View File

@ -261,8 +261,23 @@ Check constraints:
Inherits: test_like_5, Inherits: test_like_5,
test_like_5x 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_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_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 */ CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
INSERT INTO inhg VALUES (5, 10); INSERT INTO inhg VALUES (5, 10);
INSERT INTO inhg VALUES (20, 10); -- should fail INSERT INTO inhg VALUES (20, 10); -- should fail

View File

@ -95,8 +95,16 @@ CREATE TABLE test_like_5c (LIKE test_like_4 INCLUDING ALL)
INHERITS (test_like_5, test_like_5x); INHERITS (test_like_5, test_like_5x);
\d test_like_5c \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_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_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 */ CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
INSERT INTO inhg VALUES (5, 10); INSERT INTO inhg VALUES (5, 10);