Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5278668

Browse files
committed
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
1 parent c372671 commit 5278668

File tree

3 files changed

+86
-73
lines changed

3 files changed

+86
-73
lines changed

src/backend/parser/parse_utilcmd.c

Lines changed: 63 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ typedef struct
8787
List *fkconstraints; /* FOREIGN KEY constraints */
8888
List *ixconstraints; /* index-creating constraints */
8989
List *likeclauses; /* LIKE clauses that need post-processing */
90-
List *extstats; /* cloned extended statistics */
9190
List *blist; /* "before list" of things to do before
9291
* creating the table */
9392
List *alist; /* "after list" of things to do after creating
@@ -120,13 +119,14 @@ static void transformTableLikeClause(CreateStmtContext *cxt,
120119
static void transformOfType(CreateStmtContext *cxt,
121120
TypeName *ofTypename);
122121
static CreateStatsStmt *generateClonedExtStatsStmt(RangeVar *heapRel,
123-
Oid heapRelid, Oid source_statsid);
122+
Oid heapRelid,
123+
Oid source_statsid,
124+
const AttrMap *attmap);
124125
static List *get_collation(Oid collation, Oid actual_datatype);
125126
static List *get_opclass(Oid opclass, Oid actual_datatype);
126127
static void transformIndexConstraints(CreateStmtContext *cxt);
127128
static IndexStmt *transformIndexConstraint(Constraint *constraint,
128129
CreateStmtContext *cxt);
129-
static void transformExtendedStatistics(CreateStmtContext *cxt);
130130
static void transformFKConstraints(CreateStmtContext *cxt,
131131
bool skipValidation,
132132
bool isAddConstraint);
@@ -246,7 +246,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
246246
cxt.fkconstraints = NIL;
247247
cxt.ixconstraints = NIL;
248248
cxt.likeclauses = NIL;
249-
cxt.extstats = NIL;
250249
cxt.blist = NIL;
251250
cxt.alist = NIL;
252251
cxt.pkey = NULL;
@@ -339,11 +338,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
339338
*/
340339
transformCheckConstraints(&cxt, !cxt.isforeign);
341340

342-
/*
343-
* Postprocess extended statistics.
344-
*/
345-
transformExtendedStatistics(&cxt);
346-
347341
/*
348342
* Output results.
349343
*/
@@ -1111,61 +1105,25 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
11111105
}
11121106

11131107
/*
1114-
* We cannot yet deal with defaults, CHECK constraints, or indexes, since
1115-
* we don't yet know what column numbers the copied columns will have in
1116-
* the finished table. If any of those options are specified, add the
1117-
* LIKE clause to cxt->likeclauses so that expandTableLikeClause will be
1118-
* called after we do know that. Also, remember the relation OID so that
1119-
* expandTableLikeClause is certain to open the same table.
1108+
* We cannot yet deal with defaults, CHECK constraints, indexes, or
1109+
* statistics, since we don't yet know what column numbers the copied
1110+
* columns will have in the finished table. If any of those options are
1111+
* specified, add the LIKE clause to cxt->likeclauses so that
1112+
* expandTableLikeClause will be called after we do know that. Also,
1113+
* remember the relation OID so that expandTableLikeClause is certain to
1114+
* open the same table.
11201115
*/
11211116
if (table_like_clause->options &
11221117
(CREATE_TABLE_LIKE_DEFAULTS |
11231118
CREATE_TABLE_LIKE_GENERATED |
11241119
CREATE_TABLE_LIKE_CONSTRAINTS |
1125-
CREATE_TABLE_LIKE_INDEXES))
1120+
CREATE_TABLE_LIKE_INDEXES |
1121+
CREATE_TABLE_LIKE_STATISTICS))
11261122
{
11271123
table_like_clause->relationOid = RelationGetRelid(relation);
11281124
cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
11291125
}
11301126

1131-
/*
1132-
* We may copy extended statistics if requested, since the representation
1133-
* of CreateStatsStmt doesn't depend on column numbers.
1134-
*/
1135-
if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS)
1136-
{
1137-
List *parent_extstats;
1138-
ListCell *l;
1139-
1140-
parent_extstats = RelationGetStatExtList(relation);
1141-
1142-
foreach(l, parent_extstats)
1143-
{
1144-
Oid parent_stat_oid = lfirst_oid(l);
1145-
CreateStatsStmt *stats_stmt;
1146-
1147-
stats_stmt = generateClonedExtStatsStmt(cxt->relation,
1148-
RelationGetRelid(relation),
1149-
parent_stat_oid);
1150-
1151-
/* Copy comment on statistics object, if requested */
1152-
if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
1153-
{
1154-
comment = GetComment(parent_stat_oid, StatisticExtRelationId, 0);
1155-
1156-
/*
1157-
* We make use of CreateStatsStmt's stxcomment option, so as
1158-
* not to need to know now what name the statistics will have.
1159-
*/
1160-
stats_stmt->stxcomment = comment;
1161-
}
1162-
1163-
cxt->extstats = lappend(cxt->extstats, stats_stmt);
1164-
}
1165-
1166-
list_free(parent_extstats);
1167-
}
1168-
11691127
/*
11701128
* Close the parent rel, but keep our AccessShareLock on it until xact
11711129
* commit. That will prevent someone else from deleting or ALTERing the
@@ -1423,6 +1381,44 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
14231381
}
14241382
}
14251383

1384+
/*
1385+
* Process extended statistics if required.
1386+
*/
1387+
if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS)
1388+
{
1389+
List *parent_extstats;
1390+
ListCell *l;
1391+
1392+
parent_extstats = RelationGetStatExtList(relation);
1393+
1394+
foreach(l, parent_extstats)
1395+
{
1396+
Oid parent_stat_oid = lfirst_oid(l);
1397+
CreateStatsStmt *stats_stmt;
1398+
1399+
stats_stmt = generateClonedExtStatsStmt(heapRel,
1400+
RelationGetRelid(childrel),
1401+
parent_stat_oid,
1402+
attmap);
1403+
1404+
/* Copy comment on statistics object, if requested */
1405+
if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
1406+
{
1407+
comment = GetComment(parent_stat_oid, StatisticExtRelationId, 0);
1408+
1409+
/*
1410+
* We make use of CreateStatsStmt's stxcomment option, so as
1411+
* not to need to know now what name the statistics will have.
1412+
*/
1413+
stats_stmt->stxcomment = comment;
1414+
}
1415+
1416+
result = lappend(result, stats_stmt);
1417+
}
1418+
1419+
list_free(parent_extstats);
1420+
}
1421+
14261422
/* Done with child rel */
14271423
table_close(childrel, NoLock);
14281424

@@ -1837,10 +1833,12 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
18371833
* Generate a CreateStatsStmt node using information from an already existing
18381834
* extended statistic "source_statsid", for the rel identified by heapRel and
18391835
* heapRelid.
1836+
*
1837+
* Attribute numbers in expression Vars are adjusted according to attmap.
18401838
*/
18411839
static CreateStatsStmt *
18421840
generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
1843-
Oid source_statsid)
1841+
Oid source_statsid, const AttrMap *attmap)
18441842
{
18451843
HeapTuple ht_stats;
18461844
Form_pg_statistic_ext statsrec;
@@ -1923,10 +1921,19 @@ generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
19231921

19241922
foreach(lc, exprs)
19251923
{
1924+
Node *expr = (Node *) lfirst(lc);
19261925
StatsElem *selem = makeNode(StatsElem);
1926+
bool found_whole_row;
1927+
1928+
/* Adjust Vars to match new table's column numbering */
1929+
expr = map_variable_attnos(expr,
1930+
1, 0,
1931+
attmap,
1932+
InvalidOid,
1933+
&found_whole_row);
19271934

19281935
selem->name = NULL;
1929-
selem->expr = (Node *) lfirst(lc);
1936+
selem->expr = expr;
19301937

19311938
def_names = lappend(def_names, selem);
19321939
}
@@ -2652,19 +2659,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
26522659
return index;
26532660
}
26542661

2655-
/*
2656-
* transformExtendedStatistics
2657-
* Handle extended statistic objects
2658-
*
2659-
* Right now, there's nothing to do here, so we just append the list to
2660-
* the existing "after" list.
2661-
*/
2662-
static void
2663-
transformExtendedStatistics(CreateStmtContext *cxt)
2664-
{
2665-
cxt->alist = list_concat(cxt->alist, cxt->extstats);
2666-
}
2667-
26682662
/*
26692663
* transformCheckConstraints
26702664
* handle CHECK constraints
@@ -3457,7 +3451,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
34573451
cxt.fkconstraints = NIL;
34583452
cxt.ixconstraints = NIL;
34593453
cxt.likeclauses = NIL;
3460-
cxt.extstats = NIL;
34613454
cxt.blist = NIL;
34623455
cxt.alist = NIL;
34633456
cxt.pkey = NULL;
@@ -3763,9 +3756,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
37633756
newcmds = lappend(newcmds, newcmd);
37643757
}
37653758

3766-
/* Append extended statistics objects */
3767-
transformExtendedStatistics(&cxt);
3768-
37693759
/* Close rel */
37703760
relation_close(rel, NoLock);
37713761

src/test/regress/expected/create_table_like.out

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,23 @@ Check constraints:
261261
Inherits: test_like_5,
262262
test_like_5x
263263

264+
-- Test updating of column numbers in statistics expressions (bug #18468)
265+
CREATE TABLE test_like_6 (a int, c text, b text);
266+
CREATE STATISTICS ext_stat ON (a || b) FROM test_like_6;
267+
ALTER TABLE test_like_6 DROP COLUMN c;
268+
CREATE TABLE test_like_6c (LIKE test_like_6 INCLUDING ALL);
269+
\d+ test_like_6c
270+
Table "public.test_like_6c"
271+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
272+
--------+---------+-----------+----------+---------+----------+--------------+-------------
273+
a | integer | | | | plain | |
274+
b | text | | | | extended | |
275+
Statistics objects:
276+
"public.test_like_6c_expr_stat" ON (a || b) FROM test_like_6c
277+
264278
DROP TABLE test_like_4, test_like_4a, test_like_4b, test_like_4c, test_like_4d;
265279
DROP TABLE test_like_5, test_like_5x, test_like_5c;
280+
DROP TABLE test_like_6, test_like_6c;
266281
CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
267282
INSERT INTO inhg VALUES (5, 10);
268283
INSERT INTO inhg VALUES (20, 10); -- should fail

src/test/regress/sql/create_table_like.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,16 @@ CREATE TABLE test_like_5c (LIKE test_like_4 INCLUDING ALL)
9595
INHERITS (test_like_5, test_like_5x);
9696
\d test_like_5c
9797

98+
-- Test updating of column numbers in statistics expressions (bug #18468)
99+
CREATE TABLE test_like_6 (a int, c text, b text);
100+
CREATE STATISTICS ext_stat ON (a || b) FROM test_like_6;
101+
ALTER TABLE test_like_6 DROP COLUMN c;
102+
CREATE TABLE test_like_6c (LIKE test_like_6 INCLUDING ALL);
103+
\d+ test_like_6c
104+
98105
DROP TABLE test_like_4, test_like_4a, test_like_4b, test_like_4c, test_like_4d;
99106
DROP TABLE test_like_5, test_like_5x, test_like_5c;
107+
DROP TABLE test_like_6, test_like_6c;
100108

101109
CREATE TABLE inhg (x text, LIKE inhx INCLUDING INDEXES, y text); /* copies indexes */
102110
INSERT INTO inhg VALUES (5, 10);

0 commit comments

Comments
 (0)