Re: ERROR: column "id" inherits conflicting default values - Mailing list pgsql-general

I wrote:
> I guess we're going to have to rewrite that code to not store the cooked
> defaults in string form.  If they were node trees then equal() would do
> the right thing.

The attached patch should fix this.

            regards, tom lane

Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.288.2.1
diff -c -r1.288.2.1 tablecmds.c
*** src/backend/commands/tablecmds.c    7 Aug 2009 15:28:07 -0000    1.288.2.1
--- src/backend/commands/tablecmds.c    6 Oct 2009 00:25:31 -0000
***************
*** 486,492 ****
              cooked->contype = CONSTR_DEFAULT;
              cooked->name = NULL;
              cooked->attnum = attnum;
!             cooked->expr = stringToNode(colDef->cooked_default);
              cooked->is_local = true;    /* not used for defaults */
              cooked->inhcount = 0;        /* ditto */
              cookedDefaults = lappend(cookedDefaults, cooked);
--- 486,492 ----
              cooked->contype = CONSTR_DEFAULT;
              cooked->name = NULL;
              cooked->attnum = attnum;
!             cooked->expr = colDef->cooked_default;
              cooked->is_local = true;    /* not used for defaults */
              cooked->inhcount = 0;        /* ditto */
              cookedDefaults = lappend(cookedDefaults, cooked);
***************
*** 1136,1143 ****
      List       *constraints = NIL;
      int            parentsWithOids = 0;
      bool        have_bogus_defaults = false;
-     char       *bogus_marker = "Bogus!";        /* marks conflicting defaults */
      int            child_attno;

      /*
       * Check for and reject tables with too many columns. We perform this
--- 1136,1143 ----
      List       *constraints = NIL;
      int            parentsWithOids = 0;
      bool        have_bogus_defaults = false;
      int            child_attno;
+     static Node    bogus_marker = { 0 };        /* marks conflicting defaults */

      /*
       * Check for and reject tables with too many columns. We perform this
***************
*** 1321,1327 ****
               */
              if (attribute->atthasdef)
              {
!                 char       *this_default = NULL;
                  AttrDefault *attrdef;
                  int            i;

--- 1321,1327 ----
               */
              if (attribute->atthasdef)
              {
!                 Node       *this_default = NULL;
                  AttrDefault *attrdef;
                  int            i;

***************
*** 1332,1338 ****
                  {
                      if (attrdef[i].adnum == parent_attno)
                      {
!                         this_default = attrdef[i].adbin;
                          break;
                      }
                  }
--- 1332,1338 ----
                  {
                      if (attrdef[i].adnum == parent_attno)
                      {
!                         this_default = stringToNode(attrdef[i].adbin);
                          break;
                      }
                  }
***************
*** 1350,1359 ****
                   */
                  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 = bogus_marker;
                      have_bogus_defaults = true;
                  }
              }
--- 1350,1359 ----
                   */
                  Assert(def->raw_default == NULL);
                  if (def->cooked_default == NULL)
!                     def->cooked_default = this_default;
!                 else if (!equal(def->cooked_default, this_default))
                  {
!                     def->cooked_default = &bogus_marker;
                      have_bogus_defaults = true;
                  }
              }
***************
*** 1492,1498 ****
          {
              ColumnDef  *def = lfirst(entry);

!             if (def->cooked_default == bogus_marker)
                  ereport(ERROR,
                          (errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
                    errmsg("column \"%s\" inherits conflicting default values",
--- 1492,1498 ----
          {
              ColumnDef  *def = lfirst(entry);

!             if (def->cooked_default == &bogus_marker)
                  ereport(ERROR,
                          (errcode(ERRCODE_INVALID_COLUMN_DEFINITION),
                    errmsg("column \"%s\" inherits conflicting default values",
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.432
diff -c -r1.432 copyfuncs.c
*** src/backend/nodes/copyfuncs.c    18 Jun 2009 01:27:02 -0000    1.432
--- src/backend/nodes/copyfuncs.c    6 Oct 2009 00:25:33 -0000
***************
*** 2073,2079 ****
      COPY_SCALAR_FIELD(is_local);
      COPY_SCALAR_FIELD(is_not_null);
      COPY_NODE_FIELD(raw_default);
!     COPY_STRING_FIELD(cooked_default);
      COPY_NODE_FIELD(constraints);

      return newnode;
--- 2073,2079 ----
      COPY_SCALAR_FIELD(is_local);
      COPY_SCALAR_FIELD(is_not_null);
      COPY_NODE_FIELD(raw_default);
!     COPY_NODE_FIELD(cooked_default);
      COPY_NODE_FIELD(constraints);

      return newnode;
Index: src/backend/nodes/equalfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.355
diff -c -r1.355 equalfuncs.c
*** src/backend/nodes/equalfuncs.c    18 Jun 2009 01:27:02 -0000    1.355
--- src/backend/nodes/equalfuncs.c    6 Oct 2009 00:25:33 -0000
***************
*** 2052,2058 ****
      COMPARE_SCALAR_FIELD(is_local);
      COMPARE_SCALAR_FIELD(is_not_null);
      COMPARE_NODE_FIELD(raw_default);
!     COMPARE_STRING_FIELD(cooked_default);
      COMPARE_NODE_FIELD(constraints);

      return true;
--- 2052,2058 ----
      COMPARE_SCALAR_FIELD(is_local);
      COMPARE_SCALAR_FIELD(is_not_null);
      COMPARE_NODE_FIELD(raw_default);
!     COMPARE_NODE_FIELD(cooked_default);
      COMPARE_NODE_FIELD(constraints);

      return true;
Index: src/backend/nodes/outfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/outfuncs.c,v
retrieving revision 1.360
diff -c -r1.360 outfuncs.c
*** src/backend/nodes/outfuncs.c    11 Jun 2009 14:48:58 -0000    1.360
--- src/backend/nodes/outfuncs.c    6 Oct 2009 00:25:33 -0000
***************
*** 1837,1843 ****
      WRITE_BOOL_FIELD(is_local);
      WRITE_BOOL_FIELD(is_not_null);
      WRITE_NODE_FIELD(raw_default);
!     WRITE_STRING_FIELD(cooked_default);
      WRITE_NODE_FIELD(constraints);
  }

--- 1837,1843 ----
      WRITE_BOOL_FIELD(is_local);
      WRITE_BOOL_FIELD(is_not_null);
      WRITE_NODE_FIELD(raw_default);
!     WRITE_NODE_FIELD(cooked_default);
      WRITE_NODE_FIELD(constraints);
  }

Index: src/backend/parser/parse_utilcmd.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_utilcmd.c,v
retrieving revision 2.21
diff -c -r2.21 parse_utilcmd.c
*** src/backend/parser/parse_utilcmd.c    11 Jun 2009 14:49:00 -0000    2.21
--- src/backend/parser/parse_utilcmd.c    6 Oct 2009 00:25:33 -0000
***************
*** 642,648 ****
           */
          if (attribute->atthasdef && including_defaults)
          {
!             char       *this_default = NULL;
              AttrDefault *attrdef;
              int            i;

--- 642,648 ----
           */
          if (attribute->atthasdef && including_defaults)
          {
!             Node       *this_default = NULL;
              AttrDefault *attrdef;
              int            i;

***************
*** 653,659 ****
              {
                  if (attrdef[i].adnum == parent_attno)
                  {
!                     this_default = attrdef[i].adbin;
                      break;
                  }
              }
--- 653,659 ----
              {
                  if (attrdef[i].adnum == parent_attno)
                  {
!                     this_default = stringToNode(attrdef[i].adbin);
                      break;
                  }
              }
***************
*** 664,670 ****
               * but it can't; so default is ready to apply to child.
               */

!             def->cooked_default = pstrdup(this_default);
          }
      }

--- 664,670 ----
               * but it can't; so default is ready to apply to child.
               */

!             def->cooked_default = this_default;
          }
      }

Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.395
diff -c -r1.395 parsenodes.h
*** src/include/nodes/parsenodes.h    18 Jun 2009 01:27:02 -0000    1.395
--- src/include/nodes/parsenodes.h    6 Oct 2009 00:25:33 -0000
***************
*** 443,452 ****
   *
   * 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!
   *
   * The constraints list may contain a CONSTR_DEFAULT item in a raw
   * parsetree produced by gram.y, but transformCreateStmt will remove
--- 443,451 ----
   *
   * If the column has a default value, we may have the value expression
   * in either "raw" form (an untransformed parse tree) or "cooked" form
!  * (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,468 ****
      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 */
      List       *constraints;    /* other constraints on column */
  } ColumnDef;

--- 461,467 ----
      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) */
!     Node       *cooked_default; /* default value (transformed expr tree) */
      List       *constraints;    /* other constraints on column */
  } ColumnDef;

Index: src/test/regress/expected/inherit.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/inherit.out,v
retrieving revision 1.26
diff -c -r1.26 inherit.out
*** src/test/regress/expected/inherit.out    11 Jun 2008 21:53:49 -0000    1.26
--- src/test/regress/expected/inherit.out    6 Oct 2009 00:25:34 -0000
***************
*** 571,576 ****
--- 571,591 ----
   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');
  /*
Index: src/test/regress/sql/inherit.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/inherit.sql,v
retrieving revision 1.12
diff -c -r1.12 inherit.sql
*** src/test/regress/sql/inherit.sql    9 May 2008 23:32:05 -0000    1.12
--- src/test/regress/sql/inherit.sql    6 Oct 2009 00:25:34 -0000
***************
*** 121,126 ****
--- 121,137 ----
  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');