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.

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d3b89b12d7dbbddb2fc97acbc0330fce3ba3a386..03bbbb5bf51a9caecda6f9d4e1c949fb43b3ea66 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -493,7 +493,7 @@ DefineRelation(CreateStmt *stmt, char relkind)
                        cooked->contype = CONSTR_DEFAULT;
                        cooked->name = NULL;
                        cooked->attnum = attnum;
                        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);
                        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;
        List       *constraints = NIL;
        int                     parentsWithOids = 0;
        bool            have_bogus_defaults = false;

-       char       *bogus_marker = "Bogus!";            /* marks conflicting defaults */

        int                     child_attno;
        int                     child_attno;

+       static Node     bogus_marker = { 0 };           /* marks conflicting defaults */

 
        /*
         * Check for and reject tables with too many columns. We perform this
 
        /*
         * 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)
                        {
                         */
                        if (attribute->atthasdef)
                        {

-                               char       *this_default = NULL;
+                               Node       *this_default = NULL;

                                AttrDefault *attrdef;
                                int                     i;
 
                                AttrDefault *attrdef;
                                int                     i;
 


@@ -1364,7 +1364,7 @@ MergeAttributes(List *schema, List *supers, bool istemp,
                                {
                                        if (attrdef[i].adnum == parent_attno)
                                        {
                                {
                                        if (attrdef[i].adnum == parent_attno)
                                        {

-                                               this_default = attrdef[i].adbin;
+                                               this_default = stringToNode(attrdef[i].adbin);

                                                break;
                                        }
                                }
                                                break;
                                        }
                                }


@@ -1382,10 +1382,10 @@ MergeAttributes(List *schema, List *supers, bool istemp,
                                 */
                                Assert(def->raw_default == NULL);
                                if (def->cooked_default == NULL)
                                 */
                                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;
                                }
                        }
                                        have_bogus_defaults = true;
                                }
                        }


@@ -1524,7 +1524,7 @@ MergeAttributes(List *schema, List *supers, bool istemp,
                {
                        ColumnDef  *def = lfirst(entry);
 
                {
                        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",
                                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 067df4709b026777d588a939fd5cc15ab13fa870..f6d6074698cc3223104e889526a6f367cf0abe53 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2054,7 +2054,7 @@ _copyColumnDef(ColumnDef *from)
        COPY_SCALAR_FIELD(is_local);
        COPY_SCALAR_FIELD(is_not_null);
        COPY_NODE_FIELD(raw_default);
        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;
        COPY_NODE_FIELD(constraints);
 
        return newnode;

diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index ac641e6aa12a808103d99d6f5c0010196956012e..85d91dd58de80fe01c1ee555b55159f45ae8d271 100644 (file)
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -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_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;
        COMPARE_NODE_FIELD(constraints);
 
        return true;

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ae864ad2b0d1592f93532ebc6fb654627e4a61c4..988f55480c1386de24f268c1e03701b42e0ba03f 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -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_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);
 }
 
        WRITE_NODE_FIELD(constraints);
 }
 

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 2554ece7e3c8188a0d97ea8b66ef1410f8ca8220..0f7f2e8567135671c35281faab68531c1b4d3a78 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -644,7 +644,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
                 */
                if (attribute->atthasdef && including_defaults)
                {
                 */
                if (attribute->atthasdef && including_defaults)
                {

-                       char       *this_default = NULL;
+                       Node       *this_default = NULL;

                        AttrDefault *attrdef;
                        int                     i;
 
                        AttrDefault *attrdef;
                        int                     i;
 


@@ -655,7 +655,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
                        {
                                if (attrdef[i].adnum == parent_attno)
                                {
                        {
                                if (attrdef[i].adnum == parent_attno)
                                {

-                                       this_default = attrdef[i].adbin;
+                                       this_default = stringToNode(attrdef[i].adbin);

                                        break;
                                }
                        }
                                        break;
                                }
                        }


@@ -666,7 +666,7 @@ transformInhRelation(ParseState *pstate, CreateStmtContext *cxt,
                         * but it can't; so default is ready to apply to child.
                         */
 
                         * 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 b7a144b405fa3fa8fb2cda285e465cd55b48d879..bb5c014e07a9c8f38ca257edb271120c6a33be9f 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -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
  *
  * 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
  *
  * 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) */
        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;
 
        List       *constraints;        /* other constraints on column */
 } ColumnDef;
 

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 1b98ce94f1c200f8e303180cb32a4b7b723bd170..69078ae50ab506d0c2f3aadfcd97f51b12819fda 100644 (file)
--- 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)
 
  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');
 /*
 /* 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 1730a485756a83c63cd5355b395695964b736182..e68c658ec7730856192b647b56463fd94e9b9874 100644 (file)
--- 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;
 
 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');
 
 /* Test inheritance of structure (LIKE) */
 CREATE TABLE inhx (xx text DEFAULT 'text');