Обсуждение: small query, about skipping dump in dumpAttrDef

Hi All,

In dumpAttrDef() function we are skipping dump if table definition is not dumped(i.e. by checking 
tbinfo->dobj.dump), its absolutely alright to do this.

But, in dumpConstraint() we doing same by checking constraint dump flag(coninfo->dobj.dump) instead of table dump
flag(tbinfo->dobj.dump).

IMHO, for a code consistency we should use attribute dump flag(adinfo->dobj.dump) instead of table dump flag as shown
below:
=================================================================================================
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 36863df..8ac0776 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -14479,8 +14479,8 @@ dumpAttrDef(Archive *fout, DumpOptions *dopt, AttrDefInfo *adinfo)
PQExpBuffer q;
PQExpBuffer delq;

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

/* Skip if not "separate"; it was dumped in the table's definition */

=================================================================================================

Comments? Thoughts?

Regards,
Amul Sul

amul sul <sul_amul@yahoo.co.in> writes:
> In dumpAttrDef() function we are skipping dump if table definition is not dumped(i.e. by checking 
> tbinfo->dobj.dump), its absolutely alright to do this.

> But, in dumpConstraint() we doing same by checking constraint dump flag(coninfo->dobj.dump) instead of table dump
flag(tbinfo->dobj.dump).

> IMHO, for a code consistency we should use attribute dump flag(adinfo->dobj.dump) instead of table dump flag as shown
below:

I don't see much point in doing it that way, since that code could not
possibly work right if the constraint has a different dump flag from the
table: it would either omit a required component of the table or emit an
ALTER TABLE against a nonexistent table.  So this change could not fix any
bug, and might introduce some if the flag values weren't carefully kept
equal.  Note also that the normal "not separate" code path for dumping
defaults hasn't got any test on the default's flag, so this would be
adding inconsistency compared to that.
        regards, tom lane