Materialized View patch broke pg_dump

Started by Bernd Helmlealmost 13 years ago9 messages
#1Bernd Helmle
mailings@oopsware.de

It looks like the recent matview patch broke pg_dump in a way, which make
it impossible to dump 9.1 and 9.2 databases.

it fails with

pg_dump: [Archivierer (DB)] query failed: ERROR: function
pg_relation_is_scannable(oid) does not exist

Looking into this issue, it seems the version check in getTables() of
pg_dump.c is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

--
Thanks

Bernd

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Kevin Grittner
kgrittn@ymail.com
In reply to: Bernd Helmle (#1)
Re: Materialized View patch broke pg_dump

Bernd Helmle <mailings@oopsware.de> wrote:

It looks like the recent matview patch broke pg_dump in a way, which make it
impossible to dump 9.1 and 9.2 databases.

it fails with

pg_dump: [Archivierer (DB)] query failed: ERROR:  function
pg_relation_is_scannable(oid) does not exist

Looking into this issue, it seems the version check in getTables() of pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Right.  Will fix.

--
Kevin Grittner
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

#3Kevin Grittner
kgrittn@ymail.com
In reply to: Bernd Helmle (#1)
Re: Materialized View patch broke pg_dump

Bernd Helmle <mailings@oopsware.de> wrote:

Looking into this issue, it seems the version check in getTables() of pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Fixed.

Thanks for the report!

--
Kevin Grittner
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

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Kevin Grittner (#3)
Re: Materialized View patch broke pg_dump

On 03/06/2013 10:55 AM, Kevin Grittner wrote:

Bernd Helmle <mailings@oopsware.de> wrote:

Looking into this issue, it seems the version check in getTables() of pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Fixed.

Thanks for the report!

I noticed this morning that I am still getting failures on 9.0, 9.1 and
9.2 which cause my cross-version upgrade testing to fail for git tip.
For all I know this might apply to all back branches, but these are the
only ones tested for upgrade, so that's all I can report on reliably.

I'm chasing it up to find out exactly what's going on, but figured some
extra eyeballs would help.

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

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#4)
1 attachment(s)
Re: Materialized View patch broke pg_dump

On 03/11/2013 10:43 AM, Andrew Dunstan wrote:

On 03/06/2013 10:55 AM, Kevin Grittner wrote:

Bernd Helmle <mailings@oopsware.de> wrote:

Looking into this issue, it seems the version check in getTables()
of pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Fixed.

Thanks for the report!

I noticed this morning that I am still getting failures on 9.0, 9.1
and 9.2 which cause my cross-version upgrade testing to fail for git
tip. For all I know this might apply to all back branches, but these
are the only ones tested for upgrade, so that's all I can report on
reliably.

I'm chasing it up to find out exactly what's going on, but figured
some extra eyeballs would help.

The problem is that pg_dump is sending an empty query in versions less
than 9.3, and choking on that. Suggested fix attached - there's really
no reason to be doing anything re mat views in versions < 9.3.

cheers

andrew

Attachments:

matviewdumpfix.patchtext/x-patch; name=matviewdumpfix.patchDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8404458..9458429 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1769,7 +1769,7 @@ makeTableDataInfo(TableInfo *tbinfo, bool oids)
 static void
 buildMatViewRefreshDependencies(Archive *fout)
 {
-	PQExpBuffer query = createPQExpBuffer();
+	PQExpBuffer query;
 	PGresult   *res;
 	int			ntups,
 				i;
@@ -1777,38 +1777,41 @@ buildMatViewRefreshDependencies(Archive *fout)
 				i_objid,
 				i_refobjid;
 
+	/* No Mat Views before 9.3. */
+	if (fout->remoteVersion < 90300)
+		return;
+
 	/* Make sure we are in proper schema */
 	selectSourceSchema(fout, "pg_catalog");
 
-	if (fout->remoteVersion >= 90300)
-	{
-		appendPQExpBuffer(query, "with recursive w as "
-						  "( "
-						  "select d1.objid, d2.refobjid, c2.relkind as refrelkind "
-						  "from pg_depend d1 "
-						  "join pg_class c1 on c1.oid = d1.objid "
-						  "and c1.relkind = 'm' "
-						  "join pg_rewrite r1 on r1.ev_class = d1.objid "
-						  "join pg_depend d2 on d2.classid = 'pg_rewrite'::regclass "
-						  "and d2.objid = r1.oid "
-						  "and d2.refobjid <> d1.objid "
-						  "join pg_class c2 on c2.oid = d2.refobjid "
-						  "and c2.relkind in ('m','v') "
-						  "where d1.classid = 'pg_class'::regclass "
-						  "union "
-						  "select w.objid, d3.refobjid, c3.relkind "
-						  "from w "
-						  "join pg_rewrite r3 on r3.ev_class = w.refobjid "
-						  "join pg_depend d3 on d3.classid = 'pg_rewrite'::regclass "
-						  "and d3.objid = r3.oid "
-						  "and d3.refobjid <> w.refobjid "
-						  "join pg_class c3 on c3.oid = d3.refobjid "
-						  "and c3.relkind in ('m','v') "
-						  ") "
-						  "select 'pg_class'::regclass::oid as classid, objid, refobjid "
-						  "from w "
-						  "where refrelkind = 'm'");
-	}
+	query = createPQExpBuffer();
+
+	appendPQExpBuffer(query, "with recursive w as "
+					  "( "
+					  "select d1.objid, d2.refobjid, c2.relkind as refrelkind "
+					  "from pg_depend d1 "
+					  "join pg_class c1 on c1.oid = d1.objid "
+					  "and c1.relkind = 'm' "
+					  "join pg_rewrite r1 on r1.ev_class = d1.objid "
+					  "join pg_depend d2 on d2.classid = 'pg_rewrite'::regclass "
+					  "and d2.objid = r1.oid "
+					  "and d2.refobjid <> d1.objid "
+					  "join pg_class c2 on c2.oid = d2.refobjid "
+					  "and c2.relkind in ('m','v') "
+					  "where d1.classid = 'pg_class'::regclass "
+					  "union "
+					  "select w.objid, d3.refobjid, c3.relkind "
+					  "from w "
+					  "join pg_rewrite r3 on r3.ev_class = w.refobjid "
+					  "join pg_depend d3 on d3.classid = 'pg_rewrite'::regclass "
+					  "and d3.objid = r3.oid "
+					  "and d3.refobjid <> w.refobjid "
+					  "join pg_class c3 on c3.oid = d3.refobjid "
+					  "and c3.relkind in ('m','v') "
+					  ") "
+					  "select 'pg_class'::regclass::oid as classid, objid, refobjid "
+					  "from w "
+					  "where refrelkind = 'm'");
 
 	res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 
#6Fujii Masao
masao.fujii@gmail.com
In reply to: Andrew Dunstan (#5)
Re: Materialized View patch broke pg_dump

On Tue, Mar 12, 2013 at 12:43 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/11/2013 10:43 AM, Andrew Dunstan wrote:

On 03/06/2013 10:55 AM, Kevin Grittner wrote:

Bernd Helmle <mailings@oopsware.de> wrote:

Looking into this issue, it seems the version check in getTables() of
pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Fixed.

Thanks for the report!

I noticed this morning that I am still getting failures on 9.0, 9.1 and
9.2 which cause my cross-version upgrade testing to fail for git tip. For
all I know this might apply to all back branches, but these are the only
ones tested for upgrade, so that's all I can report on reliably.

I'm chasing it up to find out exactly what's going on, but figured some
extra eyeballs would help.

The problem is that pg_dump is sending an empty query in versions less than
9.3, and choking on that. Suggested fix attached - there's really no reason
to be doing anything re mat views in versions < 9.3.

This is the same problem that I reported in another thread.
/messages/by-id/CAHGQGwH+4vtyq==L6HRuPxTggfqrnLf0mWj75BfisOske28gMA@mail.gmail.com

The patch looks good to me.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Fujii Masao (#6)
Re: Materialized View patch broke pg_dump

On 03/11/2013 12:30 PM, Fujii Masao wrote:

On Tue, Mar 12, 2013 at 12:43 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/11/2013 10:43 AM, Andrew Dunstan wrote:

On 03/06/2013 10:55 AM, Kevin Grittner wrote:

Bernd Helmle <mailings@oopsware.de> wrote:

Looking into this issue, it seems the version check in getTables() of
pg_dump.c
is wrong. Shouldn't the check be

if (fout->remoteVersion >= 90300)
{

}

since this is where pg_relation_is_scannable() is introduced?

Fixed.

Thanks for the report!

I noticed this morning that I am still getting failures on 9.0, 9.1 and
9.2 which cause my cross-version upgrade testing to fail for git tip. For
all I know this might apply to all back branches, but these are the only
ones tested for upgrade, so that's all I can report on reliably.

I'm chasing it up to find out exactly what's going on, but figured some
extra eyeballs would help.

The problem is that pg_dump is sending an empty query in versions less than
9.3, and choking on that. Suggested fix attached - there's really no reason
to be doing anything re mat views in versions < 9.3.

This is the same problem that I reported in another thread.
/messages/by-id/CAHGQGwH+4vtyq==L6HRuPxTggfqrnLf0mWj75BfisOske28gMA@mail.gmail.com

Oh, I missed that. Yes, either of these would work.

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

#8Kevin Grittner
kgrittn@ymail.com
In reply to: Andrew Dunstan (#7)
Re: Materialized View patch broke pg_dump

Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/11/2013 12:30 PM, Fujii Masao wrote:

Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/11/2013 10:43 AM, Andrew Dunstan wrote:

I noticed this morning that I am still getting failures on
9.0, 9.1 and 9.2 which cause my cross-version upgrade testing
to fail for git tip. For
all I know this might apply to all back branches, but these
are the only ones tested for upgrade, so that's all I can
report on reliably.

I'm chasing it up to find out exactly what's going on, but
figured some extra eyeballs would help.

The problem is that pg_dump is sending an empty query in
versions less than 9.3, and choking on that. Suggested fix
attached - there's really no reason to be doing anything re mat
views in versions < 9.3. This is the same problem that I
reported in another thread.

/messages/by-id/CAHGQGwH+4vtyq==
L6HRuPxTggfqrnLf0mWj75BfisOske28gMA@mail.gmail.com

Oh, I missed that. Yes, either of these would work.

I've been out of town and not able to keep up here for a few days.
Will push a fix today unless I find that someone has beaten me to
it as I work through the rest of the messages.

--
Kevin Grittner
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

#9Kevin Grittner
kgrittn@ymail.com
In reply to: Kevin Grittner (#8)
Re: Materialized View patch broke pg_dump

Kevin Grittner <kgrittn@ymail.com> wrote:

Andrew Dunstan <andrew@dunslane.net> wrote:

On 03/11/2013 12:30 PM, Fujii Masao wrote:

Andrew Dunstan <andrew@dunslane.net> wrote:

The problem is that pg_dump is sending an empty query in
versions less than 9.3, and choking on that. Suggested fix
attached - there's really no reason to be doing anything re mat
views in versions < 9.3.

This is the same problem that I reported in another thread.

/messages/by-id/CAHGQGwH+4vtyq==L6HRuPxTggfqrnLf0mWj75BfisOske28gMA@mail.gmail.com

Oh, I missed that. Yes, either of these would work.

Will push a fix today unless I find that someone has beaten me to
it as I work through the rest of the messages.

I agree that either would work.  I preferred Andrew's patch because
it kept knowledge of this issue more localized.

Pushed.  Thanks to you both, and apologies for the error.

Next time I do anything with pg_dump I will know better what
constitutes decent testing.

--
Kevin Grittner
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