Re: [HACKERS] dump difference between 9.3 and master after upgrade
On Sat, Jun 22, 2013 at 3:55 PM, Andrew Dunstan and...@dunslane.net wrote: 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. We probably do need to preserve attribute numbers across an upgrade, even for foreign tables. I think those could make their way into other places. Consider: rhaas=# create foreign data wrapper dummy; CREATE FOREIGN DATA WRAPPER rhaas=# create server s1 foreign data wrapper dummy; CREATE SERVER rhaas=# create foreign table ft1 (a int, b text) server s1; CREATE FOREIGN TABLE rhaas=# create table sam (x ft1); CREATE TABLE If the new and old clusters don't agree on the attribute numbers for ft1, post-upgrade attempts to access sam.x will likely crash the server. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dump difference between 9.3 and master after upgrade
Robert Haas robertmh...@gmail.com writes: We probably do need to preserve attribute numbers across an upgrade, even for foreign tables. I think those could make their way into other places. Hm ... this argument would also apply to composite types; do we get it right for those? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dump difference between 9.3 and master after upgrade
On 06/24/2013 03:39 PM, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: We probably do need to preserve attribute numbers across an upgrade, even for foreign tables. I think those could make their way into other places. Hm ... this argument would also apply to composite types; do we get it right for those? Yes we do. It's handled at pg_dump.c starting about line 8936. It looks like we need to add cases of foreign tables to the block that starts around line 13117. We should also have a test of this in the core regression tests so that doing the wrong thing might be caught by regular upgrade testing. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dump difference between 9.3 and master after upgrade
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
Re: [HACKERS] dump difference between 9.3 and master after upgrade
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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dump difference between 9.3 and master after upgrade
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. The cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] dump difference between 9.3 and master after upgrade
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? cheers andrew --- /home/bf/bfr/root/upgrade/HEAD/origin-REL9_3_STABLE.sql 2013-06-18 08:45:22.518761202 -0400 +++ /home/bf/bfr/root/upgrade/HEAD/converted-REL9_3_STABLE-to-HEAD.sql 2013-06-18 08:46:01.648782111 -0400 @@ -385,6 +385,7 @@ -- CREATE FOREIGN TABLE ft1 ( +pg.dropped.1 integer, c1 integer NOT NULL, c2 integer NOT NULL, c3 text, @@ -413,6 +414,7 @@ CREATE FOREIGN TABLE ft2 ( c1 integer NOT NULL, c2 integer NOT NULL, +pg.dropped.3 integer, c3 text, c4 timestamp with time zone, c5 timestamp without time zone, -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
