pgsql: Add pg_sequence system catalog

On 12/20/16 8:42 AM, Peter Eisentraut wrote:

Add pg_sequence system catalog

Looking into buildfarm failures on AIX from this.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 12/20/16 9:59 AM, Peter Eisentraut wrote:

On 12/20/16 8:42 AM, Peter Eisentraut wrote:

Add pg_sequence system catalog

Looking into buildfarm failures on AIX from this.

Puzzling.

The error is

create table test1 (id serial, t text);
ERROR: MINVALUE (4294967296) must be less than MAXVALUE (-4294967296)

Normal sequence creation works.

I think this error comes from the ALTER SEQUENCE OWNED BY call that the
serial column creation synthesizes. This would read the existing
sequence parameters from the catalog (AlterSequence()), then parse the
parameters (init_params()), where nothing changes except the owner. It
still runs the crosscheck min/max, which produces the error message.
The values complained about are 0x100000000 and 0xFFFFFFFF00000000, so
it's possible that it's reading the catalog off by 4 bytes or some bytes
are flipped somewhere else.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 12/20/16 11:12 AM, Peter Eisentraut wrote:

create table test1 (id serial, t text);
ERROR: MINVALUE (4294967296) must be less than MAXVALUE (-4294967296)

Normal sequence creation works.

I think this error comes from the ALTER SEQUENCE OWNED BY call that the
serial column creation synthesizes. This would read the existing
sequence parameters from the catalog (AlterSequence()), then parse the
parameters (init_params()), where nothing changes except the owner. It
still runs the crosscheck min/max, which produces the error message.
The values complained about are 0x100000000 and 0xFFFFFFFF00000000, so
it's possible that it's reading the catalog off by 4 bytes or some bytes
are flipped somewhere else.

It looks like the catalog data is written without padding (relid(4),
start(8), inc(8), max(8), min(8), ...) but read with padding (relid(4),
padding(4), start(8), inc(8), max(8), min(8), ...). The difference is
that the write goes through heap_form_tuple but the read uses the Form_
struct. What is suspicious is this from the configure output:

checking alignment of short... 2
checking alignment of int... 4
checking alignment of long... 4
checking alignment of long long int... 8
checking alignment of double... 4

We use DOUBLEALIGN to align int64 values, but on this platform, that is
not correct.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On 12/20/16 12:51 PM, Peter Eisentraut wrote:

checking alignment of short... 2
checking alignment of int... 4
checking alignment of long... 4
checking alignment of long long int... 8
checking alignment of double... 4

We use DOUBLEALIGN to align int64 values, but on this platform, that is
not correct.

This appears to be the explanation for that:
<https://groups.google.com/forum/#!msg/comp.unix.aix/f9JAyXtibb4/pB58Af467z8J&gt;

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

It looks like the catalog data is written without padding (relid(4),
start(8), inc(8), max(8), min(8), ...) but read with padding (relid(4),
padding(4), start(8), inc(8), max(8), min(8), ...). The difference is
that the write goes through heap_form_tuple but the read uses the Form_
struct. What is suspicious is this from the configure output:

checking alignment of short... 2
checking alignment of int... 4
checking alignment of long... 4
checking alignment of long long int... 8
checking alignment of double... 4

We use DOUBLEALIGN to align int64 values, but on this platform, that is
not correct.

Quickest solution might be to reorder the columns of pg_sequence to
avoid wasting padding bytes, ie, put seqcycle after seqrelid.

regards, tom lane

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

Peter,

* Peter Eisentraut (peter_e@gmx.net) wrote:

Add pg_sequence system catalog

The query this added to dumpSequence() seems to think that sequence
names are unique across databases:

appendPQExpBuffer(query,
"SELECT seqstart, seqincrement, "
"CASE WHEN seqincrement > 0 AND seqmax = %s THEN NULL "
" WHEN seqincrement < 0 AND seqmax = -1 THEN NULL "
" ELSE seqmax "
"END AS seqmax, "
"CASE WHEN seqincrement > 0 AND seqmin = 1 THEN NULL "
" WHEN seqincrement < 0 AND seqmin = %s THEN NULL "
" ELSE seqmin "
"END AS seqmin, "
"seqcache, seqcycle "
"FROM pg_class c "
"JOIN pg_sequence s ON (s.seqrelid = c.oid) "
"WHERE relname = ",
bufx, bufm);
appendStringLiteralAH(query, tbinfo->dobj.name, fout);

Note that tbinfo->dobj.name contains just the name, and trying to match
based on just 'relname' isn't going to work since sequences with the
same name can exist in difference schemas.

I'd suggest using our usual approach in pg_dump, which is matching based
on the OID, like so:

WHERE c.oid = '%u'::oid

The OID is in: tbinfo->dobj.catId.oid

Also, you should move the selectSourceSchema() into the per-version
branches and set it to 'pg_catalog' for PG10 and up, which would allow
you to avoid having to qualify the table names, et al. As is, this
query will also fail if a user has created a 'pg_class' or
'pg_sequence' table in their schema.

Thanks!

Stephen

Peter, all,

* Stephen Frost (sfrost@snowman.net) wrote:

* Peter Eisentraut (peter_e@gmx.net) wrote:

Add pg_sequence system catalog

The query this added to dumpSequence() seems to think that sequence
names are unique across databases:

Just FYI, I've added this to the PG10 open items list so it doesn't get
forgotten (and so I can swap it out of my head and into a wiki):

https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items

Thanks!

Stephen

On 1/19/17 11:03 AM, Stephen Frost wrote:

I'd suggest using our usual approach in pg_dump, which is matching based
on the OID, like so:

WHERE c.oid = '%u'::oid

The OID is in: tbinfo->dobj.catId.oid

Also, you should move the selectSourceSchema() into the per-version
branches and set it to 'pg_catalog' for PG10 and up, which would allow
you to avoid having to qualify the table names, et al.

Does the attached patch look correct to you?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:

On 1/19/17 11:03 AM, Stephen Frost wrote:

I'd suggest using our usual approach in pg_dump, which is matching based
on the OID, like so:

WHERE c.oid = '%u'::oid

The OID is in: tbinfo->dobj.catId.oid

Also, you should move the selectSourceSchema() into the per-version
branches and set it to 'pg_catalog' for PG10 and up, which would allow
you to avoid having to qualify the table names, et al.

Does the attached patch look correct to you?

On a one-over, yes, that looks correct and avoids the issue of running
in a user's schema.

It wouldn't hurt to add a comment as to why things are so different in
PG10 than other versions, ie:

/*
* In PG10, sequence meta-data was moved into pg_sequence, so switch to
* the pg_catalog schema instead of operating in a user schema and pull
* the necessary meta-data from there.
*/

Thanks!

Stephen

On 1/24/17 10:23 AM, Stephen Frost wrote:

It wouldn't hurt to add a comment as to why things are so different in
PG10 than other versions, ie:

/*
* In PG10, sequence meta-data was moved into pg_sequence, so switch to
* the pg_catalog schema instead of operating in a user schema and pull
* the necessary meta-data from there.
*/

I've committed this, but I've put the comment in the old sections and
explained how they are different from the new style, instead of vice versa.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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