From e0c433c4a34efa6ec2ca216d201cf5dfd8515a36 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 6 Oct 2009 00:55:26 +0000 Subject: [PATCH] Change CREATE TABLE so that column default expressions coming from different inheritance parent tables are compared using equal(), instead of doing strcmp() on the nodeToString representation. The old implementation was always a tad cheesy, and it finally fails completely as of 8.4, now that the node tree might contain syntax location information. equal() knows it's supposed to ignore those fields, but strcmp() hardly can. Per recent report from Scott Ribe. --- src/backend/commands/tablecmds.c | 18 +++++++++--------- src/backend/nodes/copyfuncs.c | 4 ++-- src/backend/nodes/equalfuncs.c | 4 ++-- src/backend/nodes/outfuncs.c | 4 ++-- src/backend/parser/parse_utilcmd.c | 8 ++++---- src/include/nodes/parsenodes.h | 11 +++++------ src/test/regress/expected/inherit.out | 15 +++++++++++++++ src/test/regress/sql/inherit.sql | 11 +++++++++++ 8 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 697b80d1b4..2d728991b6 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.300 2009/10/05 19:24:37 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.301 2009/10/06 00:55:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -493,7 +493,7 @@ DefineRelation(CreateStmt *stmt, char relkind) cooked->contype = CONSTR_DEFAULT; cooked->name = NULL; cooked->attnum = attnum; - cooked->expr = stringToNode(colDef->cooked_default); + cooked->expr = colDef->cooked_default; cooked->is_local = true; /* not used for defaults */ cooked->inhcount = 0; /* ditto */ cookedDefaults = lappend(cookedDefaults, cooked); @@ -1168,8 +1168,8 @@ MergeAttributes(List *schema, List *supers, bool istemp, List *constraints = NIL; int parentsWithOids = 0; bool have_bogus_defaults = false; - char *bogus_marker = "Bogus!"; /* marks conflicting defaults */ int child_attno; + static Node bogus_marker = { 0 }; /* marks conflicting defaults */ /* * Check for and reject tables with too many columns. We perform this @@ -1353,7 +1353,7 @@ MergeAttributes(List *schema, List *supers, bool istemp, */ if (attribute->atthasdef) { - char *this_default = NULL; + Node *this_default = NULL; AttrDefault *attrdef; int i; @@ -1364,7 +1364,7 @@ MergeAttributes(List *schema, List *supers, bool istemp, { if (attrdef[i].adnum == parent_attno) { - this_default = attrdef[i].adbin; + this_default = stringToNode(attrdef[i].adbin); break; } } @@ -1382,10 +1382,10 @@ MergeAttributes(List *schema, List *supers, bool istemp, */ Assert(def->raw_default == NULL); if (def->cooked_default == NULL) - def->cooked_default = pstrdup(this_default); - else if (strcmp(def->cooked_default, this_default) != 0) + def->cooked_default = this_default; + else if (!equal(def->cooked_default, this_default)) { - def->cooked_default = bogus_marker; + def->cooked_default = &bogus_marker; have_bogus_defaults = true; } } @@ -1524,7 +1524,7 @@ MergeAttributes(List *schema, List *supers, bool istemp, { ColumnDef *def = lfirst(entry); - if (def->cooked_default == bogus_marker) + if (def->cooked_default == &bogus_marker) ereport(ERROR, (errcode(ERRCODE_INVALID_COLUMN_DEFINITION), errmsg("column \"%s\" inherits conflicting default values", diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 0264b2b339..7c3cb049b0 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -15,7 +15,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.439 2009/10/05 19:24:38 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.440 2009/10/06 00:55:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2054,7 +2054,7 @@ _copyColumnDef(ColumnDef *from) COPY_SCALAR_FIELD(is_local); COPY_SCALAR_FIELD(is_not_null); COPY_NODE_FIELD(raw_default); - COPY_STRING_FIELD(cooked_default); + COPY_NODE_FIELD(cooked_default); COPY_NODE_FIELD(constraints); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 5d3cbbda1e..bf978f871d 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -22,7 +22,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.362 2009/10/05 19:24:38 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.363 2009/10/06 00:55:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2072,7 +2072,7 @@ _equalColumnDef(ColumnDef *a, ColumnDef *b) COMPARE_SCALAR_FIELD(is_local); COMPARE_SCALAR_FIELD(is_not_null); COMPARE_NODE_FIELD(raw_default); - COMPARE_STRING_FIELD(cooked_default); + COMPARE_NODE_FIELD(cooked_default); COMPARE_NODE_FIELD(constraints); return true; diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 0a7aa250db..6751cb1a34 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.364 2009/09/17 20:49:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.365 2009/10/06 00:55:26 tgl Exp $ * * NOTES * Every node type that can appear in stored rules' parsetrees *must* @@ -1849,7 +1849,7 @@ _outColumnDef(StringInfo str, ColumnDef *node) WRITE_BOOL_FIELD(is_local); WRITE_BOOL_FIELD(is_not_null); WRITE_NODE_FIELD(raw_default); - WRITE_STRING_FIELD(cooked_default); + WRITE_NODE_FIELD(cooked_default); WRITE_NODE_FIELD(constraints); } diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 7a11eb96ed..f7a122c0d6 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -19,7 +19,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.25 2009/07/30 02:45:37 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.26 2009/10/06 00:55:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -644,7 +644,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt, */ if (attribute->atthasdef && including_defaults) { - char *this_default = NULL; + Node *this_default = NULL; AttrDefault *attrdef; int i; @@ -655,7 +655,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt, { if (attrdef[i].adnum == parent_attno) { - this_default = attrdef[i].adbin; + this_default = stringToNode(attrdef[i].adbin); break; } } @@ -666,7 +666,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt, * but it can't; so default is ready to apply to child. */ - def->cooked_default = pstrdup(this_default); + def->cooked_default = this_default; } } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 24066b9b38..b182dcfd53 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.403 2009/10/05 19:24:48 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.404 2009/10/06 00:55:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -443,10 +443,9 @@ typedef struct RangeFunction * * If the column has a default value, we may have the value expression * in either "raw" form (an untransformed parse tree) or "cooked" form - * (the nodeToString representation of an executable expression tree), - * depending on how this ColumnDef node was created (by parsing, or by - * inheritance from an existing relation). We should never have both - * in the same node! + * (a post-parse-analysis, executable expression tree), depending on + * how this ColumnDef node was created (by parsing, or by inheritance + * from an existing relation). We should never have both in the same node! * * The constraints list may contain a CONSTR_DEFAULT item in a raw * parsetree produced by gram.y, but transformCreateStmt will remove @@ -462,7 +461,7 @@ typedef struct ColumnDef bool is_local; /* column has local (non-inherited) def'n */ bool is_not_null; /* NOT NULL constraint specified? */ Node *raw_default; /* default value (untransformed parse tree) */ - char *cooked_default; /* nodeToString representation */ + Node *cooked_default; /* default value (transformed expr tree) */ List *constraints; /* other constraints on column */ } ColumnDef; diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 1b98ce94f1..69078ae50a 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -571,6 +571,21 @@ order by 1,2; bar2 | 4 | 4 (8 rows) +/* Test multiple inheritance of column defaults */ +CREATE TABLE firstparent (tomorrow date default now()::date + 1); +CREATE TABLE secondparent (tomorrow date default now() :: date + 1); +CREATE TABLE jointchild () INHERITS (firstparent, secondparent); -- ok +NOTICE: merging multiple inherited definitions of column "tomorrow" +CREATE TABLE thirdparent (tomorrow date default now()::date - 1); +CREATE TABLE otherchild () INHERITS (firstparent, thirdparent); -- not ok +NOTICE: merging multiple inherited definitions of column "tomorrow" +ERROR: column "tomorrow" inherits conflicting default values +HINT: To resolve the conflict, specify a default explicitly. +CREATE TABLE otherchild (tomorrow date default now()) + INHERITS (firstparent, thirdparent); -- ok, child resolves ambiguous default +NOTICE: merging multiple inherited definitions of column "tomorrow" +NOTICE: merging column "tomorrow" with inherited definition +DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild; /* Test inheritance of structure (LIKE) */ CREATE TABLE inhx (xx text DEFAULT 'text'); /* diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 1730a48575..e68c658ec7 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -121,6 +121,17 @@ update bar set f2 = f2 + 100 where f1 in (select f1 from foo); SELECT relname, bar.* FROM bar, pg_class where bar.tableoid = pg_class.oid order by 1,2; +/* Test multiple inheritance of column defaults */ + +CREATE TABLE firstparent (tomorrow date default now()::date + 1); +CREATE TABLE secondparent (tomorrow date default now() :: date + 1); +CREATE TABLE jointchild () INHERITS (firstparent, secondparent); -- ok +CREATE TABLE thirdparent (tomorrow date default now()::date - 1); +CREATE TABLE otherchild () INHERITS (firstparent, thirdparent); -- not ok +CREATE TABLE otherchild (tomorrow date default now()) + INHERITS (firstparent, thirdparent); -- ok, child resolves ambiguous default + +DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild; /* Test inheritance of structure (LIKE) */ CREATE TABLE inhx (xx text DEFAULT 'text');