Fix dumping pre-10 DBs by pg_dump10 if table "name" exists
Hello, hackers!
Recently my colleagues found a bug.
They could not migrate from PG9.5 to PG10 due to error during
pg_upgrage (the same as in the "reproduce" part below).
An investigation showed there is a table "name" in the same schema
where the dumped sequence is located and the PG tries to unpack the
literal "bigint" as a composite type and fails.
The bug was introduced by two commits, one of them changes
search_path, the second one introduces the "'bigint'::name"
without specifying schema:
da4d1c0c15ab9afdfeee8bad9a1a9989b6bd59b5 pg_dump: Fix some schema
issues when dumping sequences
2ea5b06c7a7056dca0af1610aadebe608fbcca08 Add CREATE SEQUENCE AS <data
type> clause
Steps to reproduce:
$ psql -h pre-10 postgres
Password for user postgres:
psql (11devel, server 9.5.8)
Type "help" for help.
postgres=# create table name(id serial);
CREATE TABLE
postgres=# \q
$ pg_dump -h pre-10 postgres
pg_dump: [archiver (db)] query failed: ERROR: malformed record
literal: "bigint"
LINE 1: SELECT 'bigint'::name AS sequence_type, start_value, increme...
^
DETAIL: Missing left parenthesis.
pg_dump: [archiver (db)] query was: SELECT 'bigint'::name AS
sequence_type, start_value, increment_by, max_value, min_value,
cache_value, is_cycled FROM name_id_seq
I've implemented a little fix (attached), don't think there is
something to be written to docs and tests.
--
Best regards,
Vitaly Burovoy
Attachments:
0001-Fix-dumping-schema-if-a-table-named-name-exists.patchapplication/octet-stream; name=0001-Fix-dumping-schema-if-a-table-named-name-exists.patchDownload
From 0dfb33e64a22d244dea784394b4f4a0735523fdc Mon Sep 17 00:00:00 2001
From: Vitaly Burovoy <vitaly.burovoy@gmail.com>
Date: Tue, 31 Oct 2017 09:13:40 +0000
Subject: [PATCH] Fix dumping schema if a table named "name" exists
---
src/bin/pg_dump/pg_dump.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8733426e8a..c988edfd2a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16555,7 +16555,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
appendPQExpBuffer(query,
- "SELECT 'bigint'::name AS sequence_type, "
+ "SELECT 'bigint'::pg_catalog.name AS sequence_type, "
"start_value, increment_by, max_value, min_value, "
"cache_value, is_cycled FROM %s",
fmtId(tbinfo->dobj.name));
@@ -16566,7 +16566,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
appendPQExpBuffer(query,
- "SELECT 'bigint'::name AS sequence_type, "
+ "SELECT 'bigint'::pg_catalog.name AS sequence_type, "
"0 AS start_value, increment_by, max_value, min_value, "
"cache_value, is_cycled FROM %s",
fmtId(tbinfo->dobj.name));
--
2.13.0
Vitaly Burovoy <vitaly.burovoy@gmail.com> writes:
Recently my colleagues found a bug.
- "SELECT 'bigint'::name AS sequence_type, " + "SELECT 'bigint'::pg_catalog.name AS sequence_type,
Good catch, but I think we could simplify this by just omitting the cast
altogether:
- "SELECT 'bigint'::name AS sequence_type, "
+ "SELECT 'bigint' AS sequence_type, pg_dump doesn't particularly care whether the column comes back marked
as 'name' or 'text' or 'unknown'.
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
On 10/31/17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Vitaly Burovoy <vitaly.burovoy@gmail.com> writes:
Recently my colleagues found a bug.
- "SELECT 'bigint'::name AS sequence_type, " + "SELECT 'bigint'::pg_catalog.name AS sequence_type,Good catch, but I think we could simplify this by just omitting the cast
altogether:- "SELECT 'bigint'::name AS sequence_type, " + "SELECT 'bigint' AS sequence_type,pg_dump doesn't particularly care whether the column comes back marked
as 'name' or 'text' or 'unknown'.regards, tom lane
OK, just for convenience I'm attaching your version of the fix.
I left an other "NULL::name AS rolname" at
src/bin/pg_dump/pg_dump.c:2978 because can't check (remoteVersion <
90000) it and it is under strict "selectSourceSchema(fout,
"pg_catalog");" schema set.
--
Best regards,
Vitaly Burovoy
Attachments:
0001-Fix-dumping-schema-if-a-table-named-name-exists.ver2.patchapplication/octet-stream; name=0001-Fix-dumping-schema-if-a-table-named-name-exists.ver2.patchDownload
From 94266f6cc58a7c8a3444c0fbeebb4c76b6beee6b Mon Sep 17 00:00:00 2001
From: Vitaly Burovoy <vitaly.burovoy@gmail.com>
Date: Tue, 31 Oct 2017 09:13:40 +0000
Subject: [PATCH] Fix dumping schema if a table named "name" exists
---
src/bin/pg_dump/pg_dump.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8733426e8a..d6659e5cc5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16555,7 +16555,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
appendPQExpBuffer(query,
- "SELECT 'bigint'::name AS sequence_type, "
+ "SELECT 'bigint' AS sequence_type, "
"start_value, increment_by, max_value, min_value, "
"cache_value, is_cycled FROM %s",
fmtId(tbinfo->dobj.name));
@@ -16566,7 +16566,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
appendPQExpBuffer(query,
- "SELECT 'bigint'::name AS sequence_type, "
+ "SELECT 'bigint' AS sequence_type, "
"0 AS start_value, increment_by, max_value, min_value, "
"cache_value, is_cycled FROM %s",
fmtId(tbinfo->dobj.name));
--
2.13.0
Vitaly Burovoy <vitaly.burovoy@gmail.com> writes:
I left an other "NULL::name AS rolname" at
src/bin/pg_dump/pg_dump.c:2978 because can't check (remoteVersion <
90000) it and it is under strict "selectSourceSchema(fout,
"pg_catalog");" schema set.
Yeah, there are quite a few unqualified casts in pg_dump, but AFAICS
all the rest are OK because the search_path is just pg_catalog.
But I did find psql's describe.c making a similar mistake :-(.
Pushed that along with your fix.
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
On 10/31/17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Yeah, there are quite a few unqualified casts in pg_dump, but AFAICS
all the rest are OK because the search_path is just pg_catalog.But I did find psql's describe.c making a similar mistake :-(.
Pushed that along with your fix.regards, tom lane
Oops. I missed it in "describe.c" because I grepped for exact "::name" string.
Thank you very much!
--
Best regards,
Vitaly Burovoy
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers