Horton

Thread

Commits

  1. In client support of v10 features, use standard schema handling.

  2. Fix underqualified cast-target type names in pg_dump and psql queries.

  3. Add CREATE SEQUENCE AS <data type> clause

  4. pg_dump: Fix some schema issues when dumping sequences

  1. Fix dumping pre-10 DBs by pg_dump10 if table "name" exists

    Vitaly Burovoy <vitaly.burovoy@gmail.com> — 2017-10-31T12:08:49Z

    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

  2. Re: Fix dumping pre-10 DBs by pg_dump10 if table "name" exists

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-31T15:00:38Z

    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

  3. Re: Fix dumping pre-10 DBs by pg_dump10 if table "name" exists

    Vitaly Burovoy <vitaly.burovoy@gmail.com> — 2017-10-31T15:52:52Z

    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

  4. Re: Fix dumping pre-10 DBs by pg_dump10 if table "name" exists

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-31T17:42:41Z

    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

  5. Re: Fix dumping pre-10 DBs by pg_dump10 if table "name" exists

    Vitaly Burovoy <vitaly.burovoy@gmail.com> — 2017-10-31T18:00:33Z

    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