From 8f2f180ff10034494d947162d080363aab554cfa Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 11 Oct 2006 16:42:59 +0000
Subject: [PATCH] Code review for LIKE INCLUDING CONSTRAINTS patch --- improve
comments, don't cheat on the raw-vs-cooked status of a constraint.
---
src/backend/commands/tablecmds.c | 197 ++++++++++++++++++-------------
src/backend/parser/analyze.c | 27 +++--
src/include/nodes/parsenodes.h | 24 ++--
3 files changed, 142 insertions(+), 106 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9ad11b6f329..631b02e0fcb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.204 2006/10/06 17:13:59 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.205 2006/10/11 16:42:58 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -163,6 +163,8 @@ static List *MergeAttributes(List *schema, List *supers, bool istemp,
List **supOids, List **supconstr, int *supOidCount);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
+static void add_nonduplicate_constraint(Constraint *cdef,
+ ConstrCheck *check, int *ncheck);
static bool change_varattnos_walker(Node *node, const AttrNumber *newattno);
static void StoreCatalogInheritance(Oid relationId, List *supers);
static void StoreCatalogInheritance1(Oid relationId, Oid parentOid,
@@ -285,7 +287,6 @@ DefineRelation(CreateStmt *stmt, char relkind)
List *rawDefaults;
Datum reloptions;
ListCell *listptr;
- int i;
AttrNumber attnum;
/*
@@ -378,49 +379,35 @@ DefineRelation(CreateStmt *stmt, char relkind)
localHasOids = interpretOidsOption(stmt->options);
descriptor->tdhasoid = (localHasOids || parentOidCount > 0);
- if (old_constraints != NIL)
+ if (old_constraints || stmt->constraints)
{
- ConstrCheck *check = (ConstrCheck *)
- palloc0(list_length(old_constraints) * sizeof(ConstrCheck));
+ ConstrCheck *check;
int ncheck = 0;
+ /* make array that's certainly big enough */
+ check = (ConstrCheck *)
+ palloc((list_length(old_constraints) +
+ list_length(stmt->constraints)) * sizeof(ConstrCheck));
+ /* deal with constraints from MergeAttributes */
foreach(listptr, old_constraints)
{
Constraint *cdef = (Constraint *) lfirst(listptr);
- bool dup = false;
- if (cdef->contype != CONSTR_CHECK)
- continue;
- Assert(cdef->name != NULL);
- Assert(cdef->raw_expr == NULL && cdef->cooked_expr != NULL);
+ if (cdef->contype == CONSTR_CHECK)
+ add_nonduplicate_constraint(cdef, check, &ncheck);
+ }
+ /*
+ * analyze.c might have passed some precooked constraints too,
+ * due to LIKE tab INCLUDING CONSTRAINTS
+ */
+ foreach(listptr, stmt->constraints)
+ {
+ Constraint *cdef = (Constraint *) lfirst(listptr);
- /*
- * In multiple-inheritance situations, it's possible to inherit
- * the same grandparent constraint through multiple parents.
- * Hence, discard inherited constraints that match as to both name
- * and expression. Otherwise, gripe if the names conflict.
- */
- for (i = 0; i < ncheck; i++)
- {
- if (strcmp(check[i].ccname, cdef->name) != 0)
- continue;
- if (strcmp(check[i].ccbin, cdef->cooked_expr) == 0)
- {
- dup = true;
- break;
- }
- ereport(ERROR,
- (errcode(ERRCODE_DUPLICATE_OBJECT),
- errmsg("duplicate check constraint name \"%s\"",
- cdef->name)));
- }
- if (!dup)
- {
- check[ncheck].ccname = cdef->name;
- check[ncheck].ccbin = pstrdup(cdef->cooked_expr);
- ncheck++;
- }
+ if (cdef->contype == CONSTR_CHECK && cdef->cooked_expr != NULL)
+ add_nonduplicate_constraint(cdef, check, &ncheck);
}
+ /* if we found any, insert 'em into the descriptor */
if (ncheck > 0)
{
if (descriptor->constr == NULL)
@@ -1118,66 +1105,57 @@ MergeAttributes(List *schema, List *supers, bool istemp,
return schema;
}
+
/*
- * Varattnos of pg_constraint.conbin must be rewritten when subclasses inherit
- * constraints from parent classes, since the inherited attributes could
- * be given different column numbers in multiple-inheritance cases.
- *
- * Note that the passed node tree is modified in place!
- *
- * This function is used elsewhere such as in analyze.c
- *
+ * In multiple-inheritance situations, it's possible to inherit
+ * the same grandparent constraint through multiple parents.
+ * Hence, we want to discard inherited constraints that match as to
+ * both name and expression. Otherwise, gripe if there are conflicting
+ * names. Nonconflicting constraints are added to the array check[]
+ * of length *ncheck ... caller must ensure there is room!
*/
-
-void
-change_varattnos_of_a_node(Node *node, const AttrNumber *newattno)
+static void
+add_nonduplicate_constraint(Constraint *cdef, ConstrCheck *check, int *ncheck)
{
- change_varattnos_walker(node, newattno);
-}
-
-/* Generate a map for change_varattnos_of_a_node from two tupledesc's. */
+ int i;
-AttrNumber *
-varattnos_map(TupleDesc old, TupleDesc new)
-{
- int i,
- j;
- AttrNumber *attmap = palloc0(sizeof(AttrNumber) * old->natts);
+ /* Should only see precooked constraints here */
+ Assert(cdef->contype == CONSTR_CHECK);
+ Assert(cdef->name != NULL);
+ Assert(cdef->raw_expr == NULL && cdef->cooked_expr != NULL);
- for (i = 1; i <= old->natts; i++)
+ for (i = 0; i < *ncheck; i++)
{
- if (old->attrs[i - 1]->attisdropped)
- {
- attmap[i - 1] = 0;
+ if (strcmp(check[i].ccname, cdef->name) != 0)
continue;
- }
- for (j = 1; j <= new->natts; j++)
- if (!strcmp(NameStr(old->attrs[i - 1]->attname), NameStr(new->attrs[j - 1]->attname)))
- attmap[i - 1] = j;
+ if (strcmp(check[i].ccbin, cdef->cooked_expr) == 0)
+ return; /* duplicate constraint, so ignore it */
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("duplicate check constraint name \"%s\"",
+ cdef->name)));
}
- return attmap;
+ /* No match on name, so add it to array */
+ check[*ncheck].ccname = cdef->name;
+ check[*ncheck].ccbin = pstrdup(cdef->cooked_expr);
+ (*ncheck)++;
}
+
/*
- * Generate a map for change_varattnos_of_a_node from a tupledesc and a list of
- * ColumnDefs
+ * Replace varattno values in an expression tree according to the given
+ * map array, that is, varattno N is replaced by newattno[N-1]. It is
+ * caller's responsibility to ensure that the array is long enough to
+ * define values for all user varattnos present in the tree. System column
+ * attnos remain unchanged.
+ *
+ * Note that the passed node tree is modified in-place!
*/
-AttrNumber *
-varattnos_map_schema(TupleDesc old, List *schema)
+void
+change_varattnos_of_a_node(Node *node, const AttrNumber *newattno)
{
- int i;
- AttrNumber *attmap = palloc0(sizeof(AttrNumber) * old->natts);
-
- for (i = 1; i <= old->natts; i++)
- {
- if (old->attrs[i - 1]->attisdropped)
- {
- attmap[i - 1] = 0;
- continue;
- }
- attmap[i - 1] = findAttrByName(NameStr(old->attrs[i - 1]->attname), schema);
- }
- return attmap;
+ /* no setup needed, so away we go */
+ (void) change_varattnos_walker(node, newattno);
}
static bool
@@ -1206,6 +1184,59 @@ change_varattnos_walker(Node *node, const AttrNumber *newattno)
(void *) newattno);
}
+/*
+ * Generate a map for change_varattnos_of_a_node from old and new TupleDesc's,
+ * matching according to column name.
+ */
+AttrNumber *
+varattnos_map(TupleDesc old, TupleDesc new)
+{
+ AttrNumber *attmap;
+ int i,
+ j;
+
+ attmap = (AttrNumber *) palloc0(sizeof(AttrNumber) * old->natts);
+ for (i = 1; i <= old->natts; i++)
+ {
+ if (old->attrs[i - 1]->attisdropped)
+ continue; /* leave the entry as zero */
+
+ for (j = 1; j <= new->natts; j++)
+ {
+ if (strcmp(NameStr(old->attrs[i - 1]->attname),
+ NameStr(new->attrs[j - 1]->attname)) == 0)
+ {
+ attmap[i - 1] = j;
+ break;
+ }
+ }
+ }
+ return attmap;
+}
+
+/*
+ * Generate a map for change_varattnos_of_a_node from a TupleDesc and a list
+ * of ColumnDefs
+ */
+AttrNumber *
+varattnos_map_schema(TupleDesc old, List *schema)
+{
+ AttrNumber *attmap;
+ int i;
+
+ attmap = (AttrNumber *) palloc0(sizeof(AttrNumber) * old->natts);
+ for (i = 1; i <= old->natts; i++)
+ {
+ if (old->attrs[i - 1]->attisdropped)
+ continue; /* leave the entry as zero */
+
+ attmap[i - 1] = findAttrByName(NameStr(old->attrs[i - 1]->attname),
+ schema);
+ }
+ return attmap;
+}
+
+
/*
* StoreCatalogInheritance
* Updates the system catalogs with proper inheritance information.
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index f4e58a3b8d1..0df3bec1694 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.352 2006/10/04 00:29:55 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.353 2006/10/11 16:42:59 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1286,12 +1286,10 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
InhRelation *inhRelation)
{
AttrNumber parent_attno;
-
Relation relation;
TupleDesc tupleDesc;
TupleConstr *constr;
AclResult aclresult;
-
bool including_defaults = false;
bool including_constraints = false;
bool including_indexes = false;
@@ -1342,15 +1340,18 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
including_indexes = false;
break;
default:
- elog(ERROR, "unrecognized CREATE TABLE LIKE option: %d", option);
+ elog(ERROR, "unrecognized CREATE TABLE LIKE option: %d",
+ option);
}
}
if (including_indexes)
- elog(ERROR, "TODO");
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("LIKE INCLUDING INDEXES is not implemented")));
/*
- * Insert the inherited attributes into the cxt for the new table
+ * Insert the copied attributes into the cxt for the new table
* definition.
*/
for (parent_attno = 1; parent_attno <= tupleDesc->natts;
@@ -1367,7 +1368,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
continue;
/*
- * Create a new inherited column.
+ * Create a new column, which is marked as NOT inherited.
*
* For constraints, ONLY the NOT NULL constraint is inherited by the
* new column definition per SQL99.
@@ -1389,7 +1390,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
cxt->columns = lappend(cxt->columns, def);
/*
- * Copy default if any, and the default has been requested
+ * Copy default, if present and the default has been requested
*/
if (attribute->atthasdef && including_defaults)
{
@@ -1419,10 +1420,14 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
}
}
+ /*
+ * Copy CHECK constraints if requested, being careful to adjust
+ * attribute numbers
+ */
if (including_constraints && tupleDesc->constr)
{
- int ccnum;
AttrNumber *attmap = varattnos_map_schema(tupleDesc, cxt->columns);
+ int ccnum;
for (ccnum = 0; ccnum < tupleDesc->constr->num_check; ccnum++)
{
@@ -1435,8 +1440,8 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
n->contype = CONSTR_CHECK;
n->name = pstrdup(ccname);
- n->raw_expr = ccbin_node;
- n->cooked_expr = NULL;
+ n->raw_expr = NULL;
+ n->cooked_expr = nodeToString(ccbin_node);
n->indexspace = NULL;
cxt->ckconstraints = lappend(cxt->ckconstraints, (Node *) n);
}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 29f68f7fff3..8afe97cb1e2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.331 2006/10/04 00:30:09 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.332 2006/10/11 16:42:59 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -414,9 +414,19 @@ typedef struct InhRelation
{
NodeTag type;
RangeVar *relation;
- List *options;
+ List *options; /* integer List of CreateStmtLikeOption */
} InhRelation;
+typedef enum CreateStmtLikeOption
+{
+ CREATE_TABLE_LIKE_INCLUDING_DEFAULTS,
+ CREATE_TABLE_LIKE_EXCLUDING_DEFAULTS,
+ CREATE_TABLE_LIKE_INCLUDING_CONSTRAINTS,
+ CREATE_TABLE_LIKE_EXCLUDING_CONSTRAINTS,
+ CREATE_TABLE_LIKE_INCLUDING_INDEXES,
+ CREATE_TABLE_LIKE_EXCLUDING_INDEXES
+} CreateStmtLikeOption;
+
/*
* IndexElem - index parameters (used in CREATE INDEX)
*
@@ -1055,16 +1065,6 @@ typedef struct CreateStmt
char *tablespacename; /* table space to use, or NULL */
} CreateStmt;
-typedef enum CreateStmtLikeOption
-{
- CREATE_TABLE_LIKE_INCLUDING_DEFAULTS,
- CREATE_TABLE_LIKE_EXCLUDING_DEFAULTS,
- CREATE_TABLE_LIKE_INCLUDING_CONSTRAINTS,
- CREATE_TABLE_LIKE_EXCLUDING_CONSTRAINTS,
- CREATE_TABLE_LIKE_INCLUDING_INDEXES,
- CREATE_TABLE_LIKE_EXCLUDING_INDEXES
-} CreateStmtLikeOption;
-
/* ----------
* Definitions for plain (non-FOREIGN KEY) constraints in CreateStmt
*
--
GitLab