Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 37681d7

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix handling of CREATE TABLE LIKE with inheritance.
If a CREATE TABLE command uses both LIKE and traditional inheritance, Vars in CHECK constraints and expression indexes that are absorbed from a LIKE parent table tended to get mis-numbered, resulting in wrong answers and/or bizarre error messages (though probably not any actual crashes, thanks to validation occurring in the executor). In v12 and up, the same could happen to Vars in GENERATED expressions, even in cases with no LIKE clause but multiple traditional-inheritance parents. The cause of the problem for LIKE is that parse_utilcmd.c supposed it could renumber such Vars correctly during transformCreateStmt(), which it cannot since we have not yet accounted for columns added via inheritance. Fix that by postponing processing of LIKE INCLUDING CONSTRAINTS, DEFAULTS, GENERATED, INDEXES till after we've performed DefineRelation(). The error with GENERATED and multiple inheritance is a simple oversight in MergeAttributes(); it knows it has to renumber Vars in inherited CHECK constraints, but forgot to apply the same processing to inherited GENERATED expressions (a/k/a defaults). Per bug #16272 from Tom Gottfried. The non-GENERATED variants of the issue are ancient, presumably dating right back to the addition of CREATE TABLE LIKE; hence back-patch to all supported branches. Discussion: https://postgr.es/m/16272-6e32da020e9a9381@postgresql.org
1 parent d4d2149 commit 37681d7
Copy full SHA for 37681d7

File tree

Expand file treeCollapse file tree

5 files changed

+213
-55
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+213
-55
lines changed

‎src/backend/parser/parse_utilcmd.c

Copy file name to clipboardExpand all lines: src/backend/parser/parse_utilcmd.c
+124-50Lines changed: 124 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "access/amapi.h"
3030
#include "access/htup_details.h"
3131
#include "access/reloptions.h"
32+
#include "access/tupconvert.h"
3233
#include "catalog/dependency.h"
3334
#include "catalog/heap.h"
3435
#include "catalog/index.h"
@@ -80,7 +81,6 @@ typedef struct
8081
List *ckconstraints; /* CHECK constraints */
8182
List *fkconstraints; /* FOREIGN KEY constraints */
8283
List *ixconstraints; /* index-creating constraints */
83-
List *inh_indexes; /* cloned indexes from INCLUDING INDEXES */
8484
List *blist; /* "before list" of things to do before
8585
* creating the table */
8686
List *alist; /* "after list" of things to do after creating
@@ -111,7 +111,7 @@ static void transformTableLikeClause(CreateStmtContext *cxt,
111111
TableLikeClause *table_like_clause);
112112
static void transformOfType(CreateStmtContext *cxt,
113113
TypeName *ofTypename);
114-
static IndexStmt *generateClonedIndexStmt(CreateStmtContext *cxt,
114+
static IndexStmt *generateClonedIndexStmt(RangeVar *heapRel,
115115
Relation source_idx,
116116
const AttrNumber *attmap, int attmap_length);
117117
static List *get_collation(Oid collation, Oid actual_datatype);
@@ -137,6 +137,9 @@ static void setSchemaName(char *context_schema, char **stmt_schema_name);
137137
* Returns a List of utility commands to be done in sequence. One of these
138138
* will be the transformed CreateStmt, but there may be additional actions
139139
* to be done before and after the actual DefineRelation() call.
140+
* In addition to normal utility commands such as AlterTableStmt and
141+
* IndexStmt, the result list may contain TableLikeClause(s), representing
142+
* the need to perform additional parse analysis after DefineRelation().
140143
*
141144
* SQL allows constraints to be scattered all over, so thumb through
142145
* the columns and collect all constraints into one place.
@@ -225,7 +228,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
225228
cxt.ckconstraints = NIL;
226229
cxt.fkconstraints = NIL;
227230
cxt.ixconstraints = NIL;
228-
cxt.inh_indexes = NIL;
229231
cxt.blist = NIL;
230232
cxt.alist = NIL;
231233
cxt.pkey = NULL;
@@ -718,8 +720,11 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
718720
* transformTableLikeClause
719721
*
720722
* Change the LIKE <srctable> portion of a CREATE TABLE statement into
721-
* column definitions which recreate the user defined column portions of
722-
* <srctable>.
723+
* column definitions that recreate the user defined column portions of
724+
* <srctable>. Also, if there are any LIKE options that we can't fully
725+
* process at this point, add the TableLikeClause to cxt->alist, which
726+
* will cause utility.c to call expandTableLikeClause() after the new
727+
* table has been created.
723728
*/
724729
static void
725730
transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_clause)
@@ -728,7 +733,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
728733
Relation relation;
729734
TupleDesc tupleDesc;
730735
TupleConstr *constr;
731-
AttrNumber *attmap;
732736
AclResult aclresult;
733737
char *comment;
734738
ParseCallbackState pcbstate;
@@ -742,6 +746,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
742746
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
743747
errmsg("LIKE is not supported for creating foreign tables")));
744748

749+
/* Open the relation referenced by the LIKE clause */
745750
relation = relation_openrv(table_like_clause->relation, AccessShareLock);
746751

747752
if (relation->rd_rel->relkind != RELKIND_RELATION &&
@@ -779,15 +784,10 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
779784
tupleDesc = RelationGetDescr(relation);
780785
constr = tupleDesc->constr;
781786

782-
/*
783-
* Initialize column number map for map_variable_attnos(). We need this
784-
* since dropped columns in the source table aren't copied, so the new
785-
* table can have different column numbers.
786-
*/
787-
attmap = (AttrNumber *) palloc0(sizeof(AttrNumber) * tupleDesc->natts);
788-
789787
/*
790788
* Insert the copied attributes into the cxt for the new table definition.
789+
* We must do this now so that they appear in the table in the relative
790+
* position where the LIKE clause is, as required by SQL99.
791791
*/
792792
for (parent_attno = 1; parent_attno <= tupleDesc->natts;
793793
parent_attno++)
@@ -797,7 +797,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
797797
ColumnDef *def;
798798

799799
/*
800-
* Ignore dropped columns in the parent. attmap entry is left zero.
800+
* Ignore dropped columns in the parent.
801801
*/
802802
if (attribute->attisdropped)
803803
continue;
@@ -829,8 +829,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
829829
*/
830830
cxt->columns = lappend(cxt->columns, def);
831831

832-
attmap[parent_attno - 1] = list_length(cxt->columns);
833-
834832
/*
835833
* Copy default, if present and the default has been requested
836834
*/
@@ -890,22 +888,88 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
890888
/* We use oids if at least one LIKE'ed table has oids. */
891889
cxt->hasoids |= relation->rd_rel->relhasoids;
892890

891+
/*
892+
* We cannot yet deal with CHECK constraints or indexes, since we don't
893+
* yet know what column numbers the copied columns will have in the
894+
* finished table. If any of those options are specified, add the LIKE
895+
* clause to cxt->alist so that expandTableLikeClause will be called after
896+
* we do know that.
897+
*/
898+
if (table_like_clause->options &
899+
(CREATE_TABLE_LIKE_CONSTRAINTS |
900+
CREATE_TABLE_LIKE_INDEXES))
901+
cxt->alist = lappend(cxt->alist, table_like_clause);
902+
903+
/*
904+
* Close the parent rel, but keep our AccessShareLock on it until xact
905+
* commit. That will prevent someone else from deleting or ALTERing the
906+
* parent before we can run expandTableLikeClause.
907+
*/
908+
heap_close(relation, NoLock);
909+
}
910+
911+
/*
912+
* expandTableLikeClause
913+
*
914+
* Process LIKE options that require knowing the final column numbers
915+
* assigned to the new table's columns. This executes after we have
916+
* run DefineRelation for the new table. It returns a list of utility
917+
* commands that should be run to generate indexes etc.
918+
*/
919+
List *
920+
expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
921+
{
922+
List *result = NIL;
923+
List *atsubcmds = NIL;
924+
Relation relation;
925+
Relation childrel;
926+
TupleDesc tupleDesc;
927+
TupleConstr *constr;
928+
AttrNumber *attmap;
929+
char *comment;
930+
931+
/*
932+
* Open the relation referenced by the LIKE clause. We should still have
933+
* the table lock obtained by transformTableLikeClause (and this'll throw
934+
* an assertion failure if not). Hence, no need to recheck privileges
935+
* etc.
936+
*/
937+
relation = relation_openrv(table_like_clause->relation, NoLock);
938+
939+
tupleDesc = RelationGetDescr(relation);
940+
constr = tupleDesc->constr;
941+
942+
/*
943+
* Open the newly-created child relation; we have lock on that too.
944+
*/
945+
childrel = relation_openrv(heapRel, NoLock);
946+
947+
/*
948+
* Construct a map from the LIKE relation's attnos to the child rel's.
949+
* This re-checks type match etc, although it shouldn't be possible to
950+
* have a failure since both tables are locked.
951+
*/
952+
attmap = convert_tuples_by_name_map(RelationGetDescr(childrel),
953+
tupleDesc,
954+
gettext_noop("could not convert row type"));
955+
893956
/*
894957
* Copy CHECK constraints if requested, being careful to adjust attribute
895958
* numbers so they match the child.
896959
*/
897960
if ((table_like_clause->options & CREATE_TABLE_LIKE_CONSTRAINTS) &&
898-
tupleDesc->constr)
961+
constr != NULL)
899962
{
900963
int ccnum;
901964

902-
for (ccnum = 0; ccnum < tupleDesc->constr->num_check; ccnum++)
965+
for (ccnum = 0; ccnum < constr->num_check; ccnum++)
903966
{
904-
char *ccname = tupleDesc->constr->check[ccnum].ccname;
905-
char *ccbin = tupleDesc->constr->check[ccnum].ccbin;
906-
Constraint *n = makeNode(Constraint);
967+
char *ccname = constr->check[ccnum].ccname;
968+
char *ccbin = constr->check[ccnum].ccbin;
907969
Node *ccbin_node;
908970
bool found_whole_row;
971+
Constraint *n;
972+
AlterTableCmd *atsubcmd;
909973

910974
ccbin_node = map_variable_attnos(stringToNode(ccbin),
911975
1, 0,
@@ -926,12 +990,21 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
926990
ccname,
927991
RelationGetRelationName(relation))));
928992

993+
n = makeNode(Constraint);
929994
n->contype = CONSTR_CHECK;
930995
n->location = -1;
931996
n->conname = pstrdup(ccname);
932997
n->raw_expr = NULL;
933998
n->cooked_expr = nodeToString(ccbin_node);
934-
cxt->ckconstraints = lappend(cxt->ckconstraints, n);
999+
1000+
/* We can skip validation, since the new table should be empty. */
1001+
n->skip_validation = true;
1002+
n->initially_valid = true;
1003+
1004+
atsubcmd = makeNode(AlterTableCmd);
1005+
atsubcmd->subtype = AT_AddConstraint;
1006+
atsubcmd->def = (Node *) n;
1007+
atsubcmds = lappend(atsubcmds, atsubcmd);
9351008

9361009
/* Copy comment on constraint */
9371010
if ((table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS) &&
@@ -943,19 +1016,35 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
9431016
CommentStmt *stmt = makeNode(CommentStmt);
9441017

9451018
stmt->objtype = OBJECT_TABCONSTRAINT;
946-
stmt->objname = list_make3(makeString(cxt->relation->schemaname),
947-
makeString(cxt->relation->relname),
1019+
stmt->objname = list_make3(makeString(heapRel->schemaname),
1020+
makeString(heapRel->relname),
9481021
makeString(n->conname));
9491022
stmt->objargs = NIL;
9501023
stmt->comment = comment;
9511024

952-
cxt->alist = lappend(cxt->alist, stmt);
1025+
result = lappend(result, stmt);
9531026
}
9541027
}
9551028
}
9561029

9571030
/*
958-
* Likewise, copy indexes if requested
1031+
* If we generated any ALTER TABLE actions above, wrap them into a single
1032+
* ALTER TABLE command. Stick it at the front of the result, so it runs
1033+
* before any CommentStmts we made above.
1034+
*/
1035+
if (atsubcmds)
1036+
{
1037+
AlterTableStmt *atcmd = makeNode(AlterTableStmt);
1038+
1039+
atcmd->relation = copyObject(heapRel);
1040+
atcmd->cmds = atsubcmds;
1041+
atcmd->relkind = OBJECT_TABLE;
1042+
atcmd->missing_ok = false;
1043+
result = lcons(atcmd, result);
1044+
}
1045+
1046+
/*
1047+
* Process indexes if required.
9591048
*/
9601049
if ((table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) &&
9611050
relation->rd_rel->relhasindex)
@@ -974,7 +1063,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
9741063
parent_index = index_open(parent_index_oid, AccessShareLock);
9751064

9761065
/* Build CREATE INDEX statement to recreate the parent_index */
977-
index_stmt = generateClonedIndexStmt(cxt, parent_index,
1066+
index_stmt = generateClonedIndexStmt(heapRel, parent_index,
9781067
attmap, tupleDesc->natts);
9791068

9801069
/* Copy comment on index, if requested */
@@ -989,19 +1078,23 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
9891078
index_stmt->idxcomment = comment;
9901079
}
9911080

992-
/* Save it in the inh_indexes list for the time being */
993-
cxt->inh_indexes = lappend(cxt->inh_indexes, index_stmt);
1081+
result = lappend(result, index_stmt);
9941082

9951083
index_close(parent_index, AccessShareLock);
9961084
}
9971085
}
9981086

1087+
/* Done with child rel */
1088+
heap_close(childrel, NoLock);
1089+
9991090
/*
10001091
* Close the parent rel, but keep our AccessShareLock on it until xact
10011092
* commit. That will prevent someone else from deleting or ALTERing the
10021093
* parent before the child is committed.
10031094
*/
10041095
heap_close(relation, NoLock);
1096+
1097+
return result;
10051098
}
10061099

10071100
static void
@@ -1054,7 +1147,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
10541147
* "source_idx". Attribute numbers should be adjusted according to attmap.
10551148
*/
10561149
static IndexStmt *
1057-
generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
1150+
generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
10581151
const AttrNumber *attmap, int attmap_length)
10591152
{
10601153
Oid source_relid = RelationGetRelid(source_idx);
@@ -1111,7 +1204,7 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
11111204

11121205
/* Begin building the IndexStmt */
11131206
index = makeNode(IndexStmt);
1114-
index->relation = cxt->relation;
1207+
index->relation = heapRel;
11151208
index->accessMethod = pstrdup(NameStr(amrec->amname));
11161209
if (OidIsValid(idxrelrec->reltablespace))
11171210
index->tableSpace = get_tablespace_name(idxrelrec->reltablespace);
@@ -1461,24 +1554,6 @@ transformIndexConstraints(CreateStmtContext *cxt)
14611554
indexlist = lappend(indexlist, index);
14621555
}
14631556

1464-
/* Add in any indexes defined by LIKE ... INCLUDING INDEXES */
1465-
foreach(lc, cxt->inh_indexes)
1466-
{
1467-
index = (IndexStmt *) lfirst(lc);
1468-
1469-
if (index->primary)
1470-
{
1471-
if (cxt->pkey != NULL)
1472-
ereport(ERROR,
1473-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
1474-
errmsg("multiple primary keys for table \"%s\" are not allowed",
1475-
cxt->relation->relname)));
1476-
cxt->pkey = index;
1477-
}
1478-
1479-
indexlist = lappend(indexlist, index);
1480-
}
1481-
14821557
/*
14831558
* Scan the index list and remove any redundant index specifications. This
14841559
* can happen if, for instance, the user writes UNIQUE PRIMARY KEY. A
@@ -2499,7 +2574,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
24992574
cxt.ckconstraints = NIL;
25002575
cxt.fkconstraints = NIL;
25012576
cxt.ixconstraints = NIL;
2502-
cxt.inh_indexes = NIL;
25032577
cxt.blist = NIL;
25042578
cxt.alist = NIL;
25052579
cxt.pkey = NULL;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.