On 06/20/2013 11:16 AM, I wrote:
On 06/20/2013 10:43 AM, Robert Haas wrote:
On Tue, Jun 18, 2013 at 12:18 PM, Andrew Dunstan
<and...@dunslane.net> wrote:
As I was updating my cross version upgrade tester to include support
for the
9.3 branch, I noted this dump difference between the dump of the
original
9.3 database and the dump of the converted database, which looks
odd. Is it
correct?
It looks sketchy to me, but I'm not sure exactly what you're comparing.
Essentially, cross version upgrade testing runs pg_dumpall from the
new version on the old cluster, runs pg_upgrade, and then runs
pg_dumpall on the upgraded cluster, and compares the two outputs. This
is what we get when the new version is HEAD and the old version is 9.3.
The reason this hasn't been caught by the standard same-version
upgrade tests is that this module uses a more extensive cluster, which
has had not only the core regression tests run but also all the
contrib and pl regression tests, and this problem seems to be exposed
by the postgres_fdw tests.
At first glance to me like pg_dump in binary-upgrade mode is not
suppressing something that it should be suppressing.
Yeah, after examination I don't see why we should output anything for
dropped columns of a foreign table in binary upgrade mode. This looks to
me like it's been a bug back to 9.1 where we introduced foreign tables.
I think something like the attached should fix it, although I'm not sure
if that's the right place for the fix.
cheers
andrew
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ec956ad..b25b475 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6670,7 +6670,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
* 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.
+ * physical column order, unless it's a Foreign Table.
*
* This function exists because there are scattered nonobvious places that
* must be kept in sync with this decision.
@@ -6679,7 +6679,7 @@ bool
shouldPrintColumn(TableInfo *tbinfo, int colno)
{
if (binary_upgrade)
- return true;
+ return (tbinfo->relkind != RELKIND_FOREIGN_TABLE || !tbinfo->attisdropped[colno]);
return (tbinfo->attislocal[colno] && !tbinfo->attisdropped[colno]);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
