Patch: fix pg_dump for inherited defaults & not-null flags

Attached is a proposed patch to deal with the issue described here:
http://archives.postgresql.org/pgsql-bugs/2012-02/msg00000.php

Even though we'd previously realized that comparing the text of
inherited CHECK expressions is an entirely unsafe way to detect
expression equivalence (cf comments for guessConstraintInheritance),
pg_dump is still doing that for inherited DEFAULT expressions, with
the predictable result that it does the wrong thing in this sort
of example.

Furthermore, as I looked more closely at the code, I realized that
there is another pretty fundamental issue: if an inherited column has a
default expression or NOT NULL bit that it did not inherit from its
parent, flagInhAttrs forces the column to be treated as non-inherited,
so that it will be emitted as part of the child table's CREATE TABLE
command.  This is *wrong* if the column is not attislocal; it will
result in the column incorrectly having the attislocal property after
restore.  (Note: such a situation could only arise if the user had
altered the column's default or NOT NULL property with ALTER TABLE after
creation.)

All of this logic predates the invention of attislocal, and really is
attempting to make up for the lack of that bit, so it's not all that
surprising that it falls down.

So the attached patch makes the emit-column-or-not behavior depend only
on attislocal (except for binary upgrade which has its own kluge
solution for the problem; though note that the whether-to-dump tests now
exactly match the special cases for binary upgrade, which they did not
before).  Also, I've dropped the former attempts to exploit inheritance
of defaults, and so the code will now emit a default explicitly for each
child in an inheritance hierarchy, even if it didn't really need to.
Since the backend doesn't track whether defaults are inherited, this
doesn't cause any failure to restore the catalog state properly.

Although this is a bug fix, it's a nontrivial change in the logic and
so I'm hesitant to back-patch into stable branches.  Given the lack of
prior complaints, maybe it would be best to leave it unfixed in existing
branches?  Not sure.  Thoughts?

            regards, tom lane

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 5a72d0da917a368ebbdbf164406c183693eb939c..266441df61d2c1eedd7faa39b8987254fe076f1c 100644
*** a/src/bin/pg_dump/common.c
--- b/src/bin/pg_dump/common.c
*************** flagInhTables(TableInfo *tblinfo, int nu
*** 281,287 ****

  /* flagInhAttrs -
   *     for each dumpable table in tblinfo, flag its inherited attributes
!  * so when we dump the table out, we don't dump out the inherited attributes
   *
   * modifies tblinfo
   */
--- 281,293 ----

  /* flagInhAttrs -
   *     for each dumpable table in tblinfo, flag its inherited attributes
!  *
!  * What we need to do here is detect child columns that inherit NOT NULL
!  * bits from their parents (so that we needn't specify that again for the
!  * child) and child columns that have DEFAULT NULL when their parents had
!  * some non-null default.  In the latter case, we make up a dummy AttrDefInfo
!  * object so that we'll correctly emit the necessary DEFAULT NULL clause;
!  * otherwise the backend will apply an inherited default to the column.
   *
   * modifies tblinfo
   */
*************** flagInhAttrs(TableInfo *tblinfo, int num
*** 297,303 ****
          TableInfo  *tbinfo = &(tblinfo[i]);
          int            numParents;
          TableInfo **parents;
-         TableInfo  *parent;

          /* Sequences and views never have parents */
          if (tbinfo->relkind == RELKIND_SEQUENCE ||
--- 303,308 ----
*************** flagInhAttrs(TableInfo *tblinfo, int num
*** 314,445 ****
          if (numParents == 0)
              continue;            /* nothing to see here, move along */

!         /*----------------------------------------------------------------
!          * For each attr, check the parent info: if no parent has an attr
!          * with the same name, then it's not inherited. If there *is* an
!          * attr with the same name, then only dump it if:
!          *
!          * - it is NOT NULL and zero parents are NOT NULL
!          *     OR
!          * - it has a default value AND the default value does not match
!          *     all parent default values, or no parents specify a default.
!          *
!          * See discussion on -hackers around 2-Apr-2001.
!          *----------------------------------------------------------------
!          */
          for (j = 0; j < tbinfo->numatts; j++)
          {
-             bool        foundAttr;        /* Attr was found in a parent */
              bool        foundNotNull;    /* Attr was NOT NULL in a parent */
!             bool        defaultsMatch;    /* All non-empty defaults match */
!             bool        defaultsFound;    /* Found a default in a parent */
!             AttrDefInfo *attrDef;
!
!             foundAttr = false;
!             foundNotNull = false;
!             defaultsMatch = true;
!             defaultsFound = false;

!             attrDef = tbinfo->attrdefs[j];

              for (k = 0; k < numParents; k++)
              {
                  int            inhAttrInd;

-                 parent = parents[k];
                  inhAttrInd = strInArray(tbinfo->attnames[j],
                                          parent->attnames,
                                          parent->numatts);
!
!                 if (inhAttrInd != -1)
                  {
-                     AttrDefInfo *inhDef = parent->attrdefs[inhAttrInd];
-
-                     foundAttr = true;
                      foundNotNull |= parent->notnull[inhAttrInd];
!                     if (inhDef != NULL)
!                     {
!                         defaultsFound = true;
!
!                         /*
!                          * If any parent has a default and the child doesn't,
!                          * we have to emit an explicit DEFAULT NULL clause for
!                          * the child, else the parent's default will win.
!                          */
!                         if (attrDef == NULL)
!                         {
!                             attrDef = (AttrDefInfo *) pg_malloc(sizeof(AttrDefInfo));
!                             attrDef->dobj.objType = DO_ATTRDEF;
!                             attrDef->dobj.catId.tableoid = 0;
!                             attrDef->dobj.catId.oid = 0;
!                             AssignDumpId(&attrDef->dobj);
!                             attrDef->adtable = tbinfo;
!                             attrDef->adnum = j + 1;
!                             attrDef->adef_expr = pg_strdup("NULL");
!
!                             attrDef->dobj.name = pg_strdup(tbinfo->dobj.name);
!                             attrDef->dobj.namespace = tbinfo->dobj.namespace;
!
!                             attrDef->dobj.dump = tbinfo->dobj.dump;
!
!                             attrDef->separate = false;
!                             addObjectDependency(&tbinfo->dobj,
!                                                 attrDef->dobj.dumpId);
!
!                             tbinfo->attrdefs[j] = attrDef;
!                         }
!                         if (strcmp(attrDef->adef_expr, inhDef->adef_expr) != 0)
!                         {
!                             defaultsMatch = false;
!
!                             /*
!                              * Whenever there is a non-matching parent
!                              * default, add a dependency to force the parent
!                              * default to be dumped first, in case the
!                              * defaults end up being dumped as separate
!                              * commands.  Otherwise the parent default will
!                              * override the child's when it is applied.
!                              */
!                             addObjectDependency(&attrDef->dobj,
!                                                 inhDef->dobj.dumpId);
!                         }
!                     }
                  }
              }

!             /*
!              * Based on the scan of the parents, decide if we can rely on the
!              * inherited attr
!              */
!             if (foundAttr)        /* Attr was inherited */
              {
!                 /* Set inherited flag by default */
!                 tbinfo->inhAttrs[j] = true;
!                 tbinfo->inhAttrDef[j] = true;
!                 tbinfo->inhNotNull[j] = true;

!                 /*
!                  * Clear it if attr had a default, but parents did not, or
!                  * mismatch
!                  */
!                 if ((attrDef != NULL) && (!defaultsFound || !defaultsMatch))
                  {
!                     tbinfo->inhAttrs[j] = false;
!                     tbinfo->inhAttrDef[j] = false;
                  }
!
!                 /*
!                  * Clear it if NOT NULL and none of the parents were NOT NULL
!                  */
!                 if (tbinfo->notnull[j] && !foundNotNull)
                  {
!                     tbinfo->inhAttrs[j] = false;
!                     tbinfo->inhNotNull[j] = false;
                  }

!                 /* Clear it if attr has local definition */
!                 if (tbinfo->attislocal[j])
!                     tbinfo->inhAttrs[j] = false;
              }
          }
      }
--- 319,388 ----
          if (numParents == 0)
              continue;            /* nothing to see here, move along */

!         /* For each column, search for matching column names in parent(s) */
          for (j = 0; j < tbinfo->numatts; j++)
          {
              bool        foundNotNull;    /* Attr was NOT NULL in a parent */
!             bool        foundDefault;    /* Found a default in a parent */

!             /* no point in examining dropped columns */
!             if (tbinfo->attisdropped[j])
!                 continue;

+             foundNotNull = false;
+             foundDefault = false;
              for (k = 0; k < numParents; k++)
              {
+                 TableInfo  *parent = parents[k];
                  int            inhAttrInd;

                  inhAttrInd = strInArray(tbinfo->attnames[j],
                                          parent->attnames,
                                          parent->numatts);
!                 if (inhAttrInd >= 0)
                  {
                      foundNotNull |= parent->notnull[inhAttrInd];
!                     foundDefault |= (parent->attrdefs[inhAttrInd] != NULL);
                  }
              }

!             /* Remember if we found inherited NOT NULL */
!             tbinfo->inhNotNull[j] = foundNotNull;
!
!             /* Manufacture a DEFAULT NULL clause if necessary */
!             if (foundDefault && tbinfo->attrdefs[j] == NULL)
              {
!                 AttrDefInfo *attrDef;

!                 attrDef = (AttrDefInfo *) pg_malloc(sizeof(AttrDefInfo));
!                 attrDef->dobj.objType = DO_ATTRDEF;
!                 attrDef->dobj.catId.tableoid = 0;
!                 attrDef->dobj.catId.oid = 0;
!                 AssignDumpId(&attrDef->dobj);
!                 attrDef->dobj.name = pg_strdup(tbinfo->dobj.name);
!                 attrDef->dobj.namespace = tbinfo->dobj.namespace;
!                 attrDef->dobj.dump = tbinfo->dobj.dump;
!
!                 attrDef->adtable = tbinfo;
!                 attrDef->adnum = j + 1;
!                 attrDef->adef_expr = pg_strdup("NULL");
!
!                 /* Will column be dumped explicitly? */
!                 if (shouldPrintColumn(tbinfo, j))
                  {
!                     attrDef->separate = false;
!                     /* No dependency needed: NULL cannot have dependencies */
                  }
!                 else
                  {
!                     /* column will be suppressed, print default separately */
!                     attrDef->separate = true;
!                     /* ensure it comes out after the table */
!                     addObjectDependency(&attrDef->dobj,
!                                         tbinfo->dobj.dumpId);
                  }

!                 tbinfo->attrdefs[j] = attrDef;
              }
          }
      }
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c91e0983f4de2d0a27a999dea19e05800cda2daa..eb3b2f74c61ebf21a65a803d3877e2237204a9b5 100644
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** getTableAttrs(Archive *fout, TableInfo *
*** 5833,5838 ****
--- 5833,5841 ----
               * attstattarget doesn't exist in 7.1.  It does exist in 7.2, but
               * we don't dump it because we can't tell whether it's been
               * explicitly set or was just a default.
+              *
+              * attislocal doesn't exist before 7.3, either; in old databases
+              * we always assume that inherited columns had no local definition.
               */
              appendPQExpBuffer(q, "SELECT a.attnum, a.attname, a.atttypmod, "
                                "-1 AS attstattarget, a.attstorage, "
*************** getTableAttrs(Archive *fout, TableInfo *
*** 5900,5913 ****
          tbinfo->attlen = (int *) pg_malloc(ntups * sizeof(int));
          tbinfo->attalign = (char *) pg_malloc(ntups * sizeof(char));
          tbinfo->attislocal = (bool *) pg_malloc(ntups * sizeof(bool));
-         tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
-         tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
          tbinfo->attoptions = (char **) pg_malloc(ntups * sizeof(char *));
          tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
          tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
!         tbinfo->inhAttrs = (bool *) pg_malloc(ntups * sizeof(bool));
!         tbinfo->inhAttrDef = (bool *) pg_malloc(ntups * sizeof(bool));
          tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
          hasdefaults = false;

          for (j = 0; j < ntups; j++)
--- 5903,5914 ----
          tbinfo->attlen = (int *) pg_malloc(ntups * sizeof(int));
          tbinfo->attalign = (char *) pg_malloc(ntups * sizeof(char));
          tbinfo->attislocal = (bool *) pg_malloc(ntups * sizeof(bool));
          tbinfo->attoptions = (char **) pg_malloc(ntups * sizeof(char *));
          tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
          tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
!         tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
          tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
+         tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
          hasdefaults = false;

          for (j = 0; j < ntups; j++)
*************** getTableAttrs(Archive *fout, TableInfo *
*** 5936,5943 ****
              if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
                  hasdefaults = true;
              /* these flags will be set in flagInhAttrs() */
-             tbinfo->inhAttrs[j] = false;
-             tbinfo->inhAttrDef[j] = false;
              tbinfo->inhNotNull[j] = false;
          }

--- 5937,5942 ----
*************** getTableAttrs(Archive *fout, TableInfo *
*** 6000,6011 ****
              {
                  int            adnum;

                  attrdefs[j].dobj.objType = DO_ATTRDEF;
                  attrdefs[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, 0));
                  attrdefs[j].dobj.catId.oid = atooid(PQgetvalue(res, j, 1));
                  AssignDumpId(&attrdefs[j].dobj);
                  attrdefs[j].adtable = tbinfo;
!                 attrdefs[j].adnum = adnum = atoi(PQgetvalue(res, j, 2));
                  attrdefs[j].adef_expr = pg_strdup(PQgetvalue(res, j, 3));

                  attrdefs[j].dobj.name = pg_strdup(tbinfo->dobj.name);
--- 5999,6026 ----
              {
                  int            adnum;

+                 adnum = atoi(PQgetvalue(res, j, 2));
+
+                 if (adnum <= 0 || adnum > ntups)
+                 {
+                     write_msg(NULL, "invalid adnum value %d for table \"%s\"\n",
+                               adnum, tbinfo->dobj.name);
+                     exit_nicely();
+                 }
+
+                 /*
+                  * dropped columns shouldn't have defaults, but just in case,
+                  * ignore 'em
+                  */
+                 if (tbinfo->attisdropped[adnum - 1])
+                     continue;
+
                  attrdefs[j].dobj.objType = DO_ATTRDEF;
                  attrdefs[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, 0));
                  attrdefs[j].dobj.catId.oid = atooid(PQgetvalue(res, j, 1));
                  AssignDumpId(&attrdefs[j].dobj);
                  attrdefs[j].adtable = tbinfo;
!                 attrdefs[j].adnum = adnum;
                  attrdefs[j].adef_expr = pg_strdup(PQgetvalue(res, j, 3));

                  attrdefs[j].dobj.name = pg_strdup(tbinfo->dobj.name);
*************** getTableAttrs(Archive *fout, TableInfo *
*** 6016,6024 ****
                  /*
                   * Defaults on a VIEW must always be dumped as separate ALTER
                   * TABLE commands.    Defaults on regular tables are dumped as
!                  * part of the CREATE TABLE if possible.  To check if it's
!                  * safe, we mark the default as needing to appear before the
!                  * CREATE.
                   */
                  if (tbinfo->relkind == RELKIND_VIEW)
                  {
--- 6031,6038 ----
                  /*
                   * Defaults on a VIEW must always be dumped as separate ALTER
                   * TABLE commands.    Defaults on regular tables are dumped as
!                  * part of the CREATE TABLE if possible, which it won't be
!                  * if the column is not going to be emitted explicitly.
                   */
                  if (tbinfo->relkind == RELKIND_VIEW)
                  {
*************** getTableAttrs(Archive *fout, TableInfo *
*** 6027,6045 ****
                      addObjectDependency(&attrdefs[j].dobj,
                                          tbinfo->dobj.dumpId);
                  }
!                 else
                  {
                      attrdefs[j].separate = false;
                      addObjectDependency(&tbinfo->dobj,
                                          attrdefs[j].dobj.dumpId);
                  }
!
!                 if (adnum <= 0 || adnum > ntups)
                  {
!                     write_msg(NULL, "invalid adnum value %d for table \"%s\"\n",
!                               adnum, tbinfo->dobj.name);
!                     exit_nicely();
                  }
                  tbinfo->attrdefs[adnum - 1] = &attrdefs[j];
              }
              PQclear(res);
--- 6041,6067 ----
                      addObjectDependency(&attrdefs[j].dobj,
                                          tbinfo->dobj.dumpId);
                  }
!                 else if (shouldPrintColumn(tbinfo, adnum - 1))
                  {
                      attrdefs[j].separate = false;
+                     /*
+                      * Mark the default as needing to appear before the table,
+                      * so that any dependencies it has must be emitted before
+                      * the CREATE TABLE.  If this is not possible, we'll change
+                      * to "separate" mode while sorting dependencies.
+                      */
                      addObjectDependency(&tbinfo->dobj,
                                          attrdefs[j].dobj.dumpId);
                  }
!                 else
                  {
!                     /* column will be suppressed, print default separately */
!                     attrdefs[j].separate = true;
!                     /* needed in case pre-7.3 DB: */
!                     addObjectDependency(&attrdefs[j].dobj,
!                                         tbinfo->dobj.dumpId);
                  }
+
                  tbinfo->attrdefs[adnum - 1] = &attrdefs[j];
              }
              PQclear(res);
*************** getTableAttrs(Archive *fout, TableInfo *
*** 6223,6228 ****
--- 6245,6272 ----
      destroyPQExpBuffer(q);
  }

+ /*
+  * Test whether a column should be printed as part of table's CREATE TABLE.
+  * Column number is zero-based.
+  *
+  * Normally this is always true, but it's false for dropped columns, as well
+  * as those that were inherited without any local definition.  (If we print
+  * such a column it will mistakenly get pg_attribute.attislocal set to true.)
+  * However, in binary_upgrade mode, we must print all such columns anyway and
+  * fix the attislocal/attisdropped state later, so as to keep control of the
+  * physical column order.
+  *
+  * This function exists because there are scattered nonobvious places that
+  * have got to be kept in sync with this decision.
+  */
+ bool
+ shouldPrintColumn(TableInfo *tbinfo, int colno)
+ {
+     if (binary_upgrade)
+         return true;
+     return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]);
+ }
+

  /*
   * getTSParsers:
*************** dumpTableSchema(Archive *fout, TableInfo
*** 12504,12541 ****
                            fmtId(tbinfo->dobj.name));

          /*
!          * In case of a binary upgrade, we dump the table normally and attach
!          * it to the type afterward.
           */
          if (tbinfo->reloftype && !binary_upgrade)
              appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
          actual_atts = 0;
          for (j = 0; j < tbinfo->numatts; j++)
          {
              /*
!              * Normally, dump if it's one of the table's own attrs, and not
!              * dropped.  But for binary upgrade, dump all the columns.
               */
!             if ((!tbinfo->inhAttrs[j] && !tbinfo->attisdropped[j]) ||
!                 binary_upgrade)
              {
                  /*
!                  * Default value --- suppress if inherited (except in
!                  * binary-upgrade case, where we're not doing normal
!                  * inheritance) or if it's to be printed separately.
                   */
!                 bool        has_default = (tbinfo->attrdefs[j] != NULL
!                                 && (!tbinfo->inhAttrDef[j] || binary_upgrade)
!                                            && !tbinfo->attrdefs[j]->separate);

                  /*
                   * Not Null constraint --- suppress if inherited, except in
!                  * binary-upgrade case.
                   */
!                 bool        has_notnull = (tbinfo->notnull[j]
!                               && (!tbinfo->inhNotNull[j] || binary_upgrade));

!                 if (tbinfo->reloftype && !has_default && !has_notnull && !binary_upgrade)
                      continue;

                  /* Format properly if not first attr */
--- 12548,12587 ----
                            fmtId(tbinfo->dobj.name));

          /*
!          * Attach to type, if reloftype; except in case of a binary upgrade,
!          * we dump the table normally and attach it to the type afterward.
           */
          if (tbinfo->reloftype && !binary_upgrade)
              appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
+
+         /* Dump the attributes */
          actual_atts = 0;
          for (j = 0; j < tbinfo->numatts; j++)
          {
              /*
!              * Normally, dump if it's locally defined in this table, and not
!              * dropped.  But for binary upgrade, we'll dump all the columns,
!              * and then fix up the dropped and nonlocal cases below.
               */
!             if (shouldPrintColumn(tbinfo, j))
              {
                  /*
!                  * Default value --- suppress if to be printed separately.
                   */
!                 bool        has_default = (tbinfo->attrdefs[j] != NULL &&
!                                            !tbinfo->attrdefs[j]->separate);

                  /*
                   * Not Null constraint --- suppress if inherited, except in
!                  * binary-upgrade case where that won't work.
                   */
!                 bool        has_notnull = (tbinfo->notnull[j] &&
!                                            (!tbinfo->inhNotNull[j] ||
!                                             binary_upgrade));

!                 /* Skip column if fully defined by reloftype */
!                 if (tbinfo->reloftype &&
!                     !has_default && !has_notnull && !binary_upgrade)
                      continue;

                  /* Format properly if not first attr */
*************** dumpTableSchema(Archive *fout, TableInfo
*** 12799,12814 ****
              }
          }

!         /* Loop dumping statistics and storage statements */
          for (j = 0; j < tbinfo->numatts; j++)
          {
              /*
               * Dump per-column statistics information. We only issue an ALTER
               * TABLE statement if the attstattarget entry for this column is
               * non-negative (i.e. it's not the default value)
               */
!             if (tbinfo->attstattarget[j] >= 0 &&
!                 !tbinfo->attisdropped[j])
              {
                  appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
                                    fmtId(tbinfo->dobj.name));
--- 12845,12880 ----
              }
          }

!         /*
!          * Dump additional per-column properties that we can't handle in the
!          * main CREATE TABLE command.
!          */
          for (j = 0; j < tbinfo->numatts; j++)
          {
+             /* None of this applies to dropped columns */
+             if (tbinfo->attisdropped[j])
+                 continue;
+
+             /*
+              * If we didn't dump the column definition explicitly above, and
+              * it is NOT NULL and did not inherit that property from a parent,
+              * we have to mark it separately.
+              */
+             if (!shouldPrintColumn(tbinfo, j) &&
+                 tbinfo->notnull[j] && !tbinfo->inhNotNull[j])
+             {
+                 appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
+                                   fmtId(tbinfo->dobj.name));
+                 appendPQExpBuffer(q, "ALTER COLUMN %s SET NOT NULL;\n",
+                                   fmtId(tbinfo->attnames[j]));
+             }
+
              /*
               * Dump per-column statistics information. We only issue an ALTER
               * TABLE statement if the attstattarget entry for this column is
               * non-negative (i.e. it's not the default value)
               */
!             if (tbinfo->attstattarget[j] >= 0)
              {
                  appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
                                    fmtId(tbinfo->dobj.name));
*************** dumpTableSchema(Archive *fout, TableInfo
*** 12822,12828 ****
               * Dump per-column storage information.  The statement is only
               * dumped if the storage has been changed from the type's default.
               */
!             if (!tbinfo->attisdropped[j] && tbinfo->attstorage[j] != tbinfo->typstorage[j])
              {
                  switch (tbinfo->attstorage[j])
                  {
--- 12888,12894 ----
               * Dump per-column storage information.  The statement is only
               * dumped if the storage has been changed from the type's default.
               */
!             if (tbinfo->attstorage[j] != tbinfo->typstorage[j])
              {
                  switch (tbinfo->attstorage[j])
                  {
*************** dumpAttrDef(Archive *fout, AttrDefInfo *
*** 12935,12952 ****
      PQExpBuffer q;
      PQExpBuffer delq;

!     /* Only print it if "separate" mode is selected */
!     if (!tbinfo->dobj.dump || !adinfo->separate || dataOnly)
          return;

!     /* Don't print inherited defaults, either, except for binary upgrade */
!     if (tbinfo->inhAttrDef[adnum - 1] && !binary_upgrade)
          return;

      q = createPQExpBuffer();
      delq = createPQExpBuffer();

!     appendPQExpBuffer(q, "ALTER TABLE %s ",
                        fmtId(tbinfo->dobj.name));
      appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n",
                        fmtId(tbinfo->attnames[adnum - 1]),
--- 13001,13018 ----
      PQExpBuffer q;
      PQExpBuffer delq;

!     /* Skip if table definition not to be dumped */
!     if (!tbinfo->dobj.dump || dataOnly)
          return;

!     /* Skip if not "separate"; it was dumped in the table's definition */
!     if (!adinfo->separate)
          return;

      q = createPQExpBuffer();
      delq = createPQExpBuffer();

!     appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
                        fmtId(tbinfo->dobj.name));
      appendPQExpBuffer(q, "ALTER COLUMN %s SET DEFAULT %s;\n",
                        fmtId(tbinfo->attnames[adnum - 1]),
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 44f7c6bdf0a8495ba8cccf277bf4ef93cc3df312..421847e5d5aa1b9a0ea10183020987a42d1e8742 100644
*** a/src/bin/pg_dump/pg_dump.h
--- b/src/bin/pg_dump/pg_dump.h
*************** typedef struct _tableInfo
*** 277,293 ****
      char      **attoptions;        /* per-attribute options */
      Oid           *attcollation;    /* per-attribute collation selection */
      char      **attfdwoptions;    /* per-attribute fdw options */
!
!     /*
!      * Note: we need to store per-attribute notnull, default, and constraint
!      * stuff for all interesting tables so that we can tell which constraints
!      * were inherited.
!      */
!     bool       *notnull;        /* Not null constraints on attributes */
!     struct _attrDefInfo **attrdefs;        /* DEFAULT expressions */
!     bool       *inhAttrs;        /* true if each attribute is inherited */
!     bool       *inhAttrDef;        /* true if attr's default is inherited */
      bool       *inhNotNull;        /* true if NOT NULL is inherited */
      struct _constraintInfo *checkexprs; /* CHECK constraints */

      /*
--- 277,285 ----
      char      **attoptions;        /* per-attribute options */
      Oid           *attcollation;    /* per-attribute collation selection */
      char      **attfdwoptions;    /* per-attribute fdw options */
!     bool       *notnull;        /* NOT NULL constraints on attributes */
      bool       *inhNotNull;        /* true if NOT NULL is inherited */
+     struct _attrDefInfo **attrdefs;        /* DEFAULT expressions */
      struct _constraintInfo *checkexprs; /* CHECK constraints */

      /*
*************** typedef struct _tableInfo
*** 300,306 ****

  typedef struct _attrDefInfo
  {
!     DumpableObject dobj;
      TableInfo  *adtable;        /* link to table of attribute */
      int            adnum;
      char       *adef_expr;        /* decompiled DEFAULT expression */
--- 292,298 ----

  typedef struct _attrDefInfo
  {
!     DumpableObject dobj;        /* note: dobj.name is name of table */
      TableInfo  *adtable;        /* link to table of attribute */
      int            adnum;
      char       *adef_expr;        /* decompiled DEFAULT expression */
*************** extern void getTriggers(Archive *fout, T
*** 556,561 ****
--- 548,554 ----
  extern ProcLangInfo *getProcLangs(Archive *fout, int *numProcLangs);
  extern CastInfo *getCasts(Archive *fout, int *numCasts);
  extern void getTableAttrs(Archive *fout, TableInfo *tbinfo, int numTables);
+ extern bool shouldPrintColumn(TableInfo *tbinfo, int colno);
  extern TSParserInfo *getTSParsers(Archive *fout, int *numTSParsers);
  extern TSDictInfo *getTSDictionaries(Archive *fout, int *numTSDicts);
  extern TSTemplateInfo *getTSTemplates(Archive *fout, int *numTSTemplates);
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4d1ae94ef5142ed27d55a0a90544edaf4658530d..6f61c0d848fc5c9e414859a2b3fca57e011a4481 100644
*** a/src/bin/pg_dump/pg_dump_sort.c
--- b/src/bin/pg_dump/pg_dump_sort.c
*************** DOTypeNameCompare(const void *p1, const
*** 188,193 ****
--- 188,202 ----
          if (cmpval != 0)
              return cmpval;
      }
+     else if (obj1->objType == DO_ATTRDEF)
+     {
+         AttrDefInfo *adobj1 = *(AttrDefInfo * const *) p1;
+         AttrDefInfo *adobj2 = *(AttrDefInfo * const *) p2;
+
+         cmpval = (adobj1->adnum - adobj2->adnum);
+         if (cmpval != 0)
+             return cmpval;
+     }

      /* Usually shouldn't get here, but if we do, sort by OID */
      return oidcmp(obj1->catId.oid, obj2->catId.oid);