sequence data type

On 08/31/2016 06:22 AM, Peter Eisentraut wrote:

Here is a patch that adds the notion of a data type to a sequence. So
it might be CREATE SEQUENCE foo AS integer. The types are restricted to
int{2,4,8} as now.

This patch does not apply cleanly to current master (=600dc4c).
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

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

On 04/09/16 06:41, Vik Fearing wrote:

On 08/31/2016 06:22 AM, Peter Eisentraut wrote:

Here is a patch that adds the notion of a data type to a sequence. So
it might be CREATE SEQUENCE foo AS integer. The types are restricted to
int{2,4,8} as now.

This patch does not apply cleanly to current master (=600dc4c).

Must admit I first thought of: "2, 4, 8, who do we appreciate!"

MORE SERIOUSLY:

Would a possibly future expansion be to include numeric?

Of hand, I can't see any value for using other than integers of 2, 4, &
8 bytes (not a criticism! - may simply be a failure of imagination on my
part).

I am curious as to the use cases for other possibilities.

Cheers,
Gavin

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

On 9/3/16 2:41 PM, Vik Fearing wrote:

On 08/31/2016 06:22 AM, Peter Eisentraut wrote:

Here is a patch that adds the notion of a data type to a sequence. So
it might be CREATE SEQUENCE foo AS integer. The types are restricted to
int{2,4,8} as now.

This patch does not apply cleanly to current master (=600dc4c).

Updated patch attached.

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

On 9/3/16 6:01 PM, Gavin Flower wrote:

I am curious as to the use cases for other possibilities.

A hex or base64 type might be interesting, should those ever get created...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532) mobile: 512-569-9461

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

On 9/10/16, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 9/3/16 6:01 PM, Gavin Flower wrote:

I am curious as to the use cases for other possibilities.

A hex or base64 type might be interesting, should those ever get created...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX

Hex or base64 are not data types. They are just different
representation types of binary sequences.
Even for bigints these representations are done after writing numbers
as byte sequences.

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

On Sun, Sep 11, 2016 at 12:38 PM, Vitaly Burovoy
<vitaly.burovoy@gmail.com> wrote:

On 9/10/16, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

On 9/3/16 6:01 PM, Gavin Flower wrote:

I am curious as to the use cases for other possibilities.

A hex or base64 type might be interesting, should those ever get created...
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX

Hex or base64 are not data types. They are just different
representation types of binary sequences.
Even for bigints these representations are done after writing numbers
as byte sequences.

Moved to next CF. The patch did not actually receive that much reviews.
--
Michael

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

Hi Vik and Vinayak,

This is a gentle reminder.

you both are assigned as reviewer's to the current patch in the 11-2016
commitfest.
But you haven't shared your review yet. Please share your review about
the patch. This will help us in smoother operation of commitfest.

Please Ignore if you already shared your review.

Regards,
Hari Babu
Fujitsu Australia

On Tue, Nov 22, 2016 at 10:54 PM, Haribabu Kommi <kommi.haribabu@gmail.com>
wrote:

Hi Vik and Vinayak,

This is a gentle reminder.

you both are assigned as reviewer's to the current patch in the 11-2016
commitfest.
But you haven't shared your review yet. Please share your review about
the patch. This will help us in smoother operation of commitfest.

Please Ignore if you already shared your review.

As the patch doesn't received a full review and it is not applying to HEAD
properly.
Moved to next CF with "waiting on author" status.

Regards,
Hari Babu
Fujitsu Australia

On 9/8/16 4:06 PM, Peter Eisentraut wrote:

On 9/3/16 2:41 PM, Vik Fearing wrote:

On 08/31/2016 06:22 AM, Peter Eisentraut wrote:

Here is a patch that adds the notion of a data type to a sequence. So
it might be CREATE SEQUENCE foo AS integer. The types are restricted to
int{2,4,8} as now.

This patch does not apply cleanly to current master (=600dc4c).

Updated patch attached.

Another updated patch, with quite a bit of rebasing and some minor code
polishing.

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

On 12/31/2016 01:27 AM, Peter Eisentraut wrote:

Another updated patch, with quite a bit of rebasing and some minor
code polishing.

Patch applies cleanly and the tests pass
The feature seems to work as expected.

I've tried this out and it behaves as expected and desired. I also
tested the PG dump changes when dumping from both 8.3 and 9.5 and tables
with serial types and standalone sequences restore as I would expect.

The only concern I have with the functionality is that there isn't a way
to change the type of a sequence.

For example

create table foo(id serial4);
--hmm I"m running out of id space
alter table foo alter column id type int8;
alter sequence foo_id_seq maxvalue
9223372036854775807;

2017-01-08 14:29:27.073 EST [4935] STATEMENT: alter sequence foo_id_seq
maxvalue
9223372036854775807;
ERROR: MAXVALUE (9223372036854775807) is too large for sequence data
type integer

Since we allow for min/maxvalue to be changed I feel we should also
allow the type to be changed.

@@ -1236,7 +1239,15 @@ init_params(ParseState *pstate, List *options,
bool isInit,
{
DefElem *defel = (DefElem *) lfirst(option);

-               if (strcmp(defel->defname, "increment") == 0)
+               if (strcmp(defel->defname, "as") == 0)
+               {
+                       if (as_type)
+                               ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+                                                errmsg("conflicting or 
redundant options")));
+                       as_type = defel;
+               }
+               else if (strcmp(defel->defname, "increment") == 0)

Should we including parser_errposition(pstate, defel->location))); like
for the other errors below this?

Other than that the patch looks good

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

On 1/8/17 2:39 PM, Steve Singer wrote:

The only concern I have with the functionality is that there isn't a way
to change the type of a sequence.

If we implement the NEXT VALUE FOR expression (or anything similar that
returns a value from the sequence), then the return type of that
expression would be the type of the sequence. Then, changing the type
of the sequence would require reparsing all expressions involving the
sequence. This could probably be sorted out somehow, but I don't want
to be too lax now and cause problems for later features. There is a
similar case, namely changing the return type of a function, which we
also prohibit.

@@ -1236,7 +1239,15 @@ init_params(ParseState *pstate, List *options,
bool isInit,
{
DefElem *defel = (DefElem *) lfirst(option);

-               if (strcmp(defel->defname, "increment") == 0)
+               if (strcmp(defel->defname, "as") == 0)
+               {
+                       if (as_type)
+                               ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+                                                errmsg("conflicting or 
redundant options")));
+                       as_type = defel;
+               }
+               else if (strcmp(defel->defname, "increment") == 0)

Should we including parser_errposition(pstate, defel->location))); like
for the other errors below this?

Yes, good catch.

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

On Tue, Jan 10, 2017 at 5:03 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 1/8/17 2:39 PM, Steve Singer wrote:

The only concern I have with the functionality is that there isn't a way
to change the type of a sequence.

If we implement the NEXT VALUE FOR expression (or anything similar that
returns a value from the sequence), then the return type of that
expression would be the type of the sequence. Then, changing the type
of the sequence would require reparsing all expressions involving the
sequence. This could probably be sorted out somehow, but I don't want
to be too lax now and cause problems for later features. There is a
similar case, namely changing the return type of a function, which we
also prohibit.

@@ -1236,7 +1239,15 @@ init_params(ParseState *pstate, List *options,
bool isInit,
{
DefElem *defel = (DefElem *) lfirst(option);

-               if (strcmp(defel->defname, "increment") == 0)
+               if (strcmp(defel->defname, "as") == 0)
+               {
+                       if (as_type)
+                               ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+                                                errmsg("conflicting or
redundant options")));
+                       as_type = defel;
+               }
+               else if (strcmp(defel->defname, "increment") == 0)

Should we including parser_errposition(pstate, defel->location))); like
for the other errors below this?

Yes, good catch.

+           ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("MINVALUE (%s) is too large for sequence data type %s",
+                       bufm, format_type_be(seqform->seqtypid))));
"too large" for a minimum value, really? You may want to add a comment
to say that the _MAX values are used for symmetry.

+           /* We use the _MAX constants for symmetry. */
+           if (seqform->seqtypid == INT2OID)
+               seqform->seqmin = -PG_INT16_MAX;
+           else if (seqform->seqtypid == INT4OID)
+               seqform->seqmin = -PG_INT32_MAX;
+           else
+               seqform->seqmin = -PG_INT64_MAX;
Hm. Is symmetry an important properly for sequences? It seems to me
that if we map with the data types we had better use the MIN values
instead for consistency. So the concept of this patch is rather weird
and would introduce an exception with the rest of the system just for
sequences.

(There are no tests for cycles).
--
Michael

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

Peter Eisentraut wrote:

This could probably be sorted out somehow, but I don't want
to be too lax now and cause problems for later features. There is a
similar case, namely changing the return type of a function, which we
also prohibit.

Consider the case of a table with a SERIAL column which later
has to become a BIGINT due to growth.
Currently a user would just alter the column's type and does
need to do anything with the sequence.

With the patch, it becomes a problem because

- ALTER SEQUENCE seqname MAXVALUE new_value
will fail because new_value is beyond the range of INT4.

- ALTER SEQUENCE seqname TYPE BIGINT
does not exist (yet?)

- DROP SEQUENCE seqname (with the idea of recreating the
sequence immediately after) will be rejected because the table
depends on the sequence.

What should a user do to upgrade the SERIAL column?

BTW, I notice that a direct UPDATE of pg_sequence happens
to work (now that we have pg_sequence thanks to your other
recent contributions on sequences), but I guess it falls under the
rule mentioned in
https://www.postgresql.org/docs/devel/static/catalogs.html

"You can drop and recreate the tables, add columns, insert and update values,
and severely mess up your system that way. Normally, one should not change
the system catalogs by hand, there are normally SQL commands to do that"

Previously, UPDATE seqname SET property=value was rejected with
a specific error "cannot change sequence".

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

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

"Daniel" == Daniel Verite <daniel@manitou-mail.org> writes:

Daniel> Consider the case of a table with a SERIAL column which later
Daniel> has to become a BIGINT due to growth. Currently a user would
Daniel> just alter the column's type and does need to do anything with
Daniel> the sequence.

Daniel> With the patch, it becomes a problem because

Daniel> - ALTER SEQUENCE seqname MAXVALUE new_value
Daniel> will fail because new_value is beyond the range of INT4.

Daniel> - ALTER SEQUENCE seqname TYPE BIGINT
Daniel> does not exist (yet?)

Daniel> - DROP SEQUENCE seqname (with the idea of recreating the
Daniel> sequence immediately after) will be rejected because the table
Daniel> depends on the sequence.

Daniel> What should a user do to upgrade the SERIAL column?

Something along the lines of:

begin;
alter table tablename alter column id drop default;
alter sequence tablename_id_seq owned by none;
create sequence tablename_id_seq2 as bigint owned by tablename.id;
select setval('tablename_id_seq2', last_value, is_called) from tablename_id_seq;
drop sequence tablename_id_seq;
alter table tablename alter column id type bigint;
alter table tablename alter column id set default nextval('tablename_id_seq2');
commit;

Not impossible, but not at all obvious and quite involved. (And -1 for
this feature unless this issue is addressed.)

--
Andrew (irc:RhodiumToad)

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

Michael Paquier wrote:

Hm. Is symmetry an important properly for sequences? It seems to me
that if we map with the data types we had better use the MIN values
instead for consistency. So the concept of this patch is rather weird
and would introduce an exception with the rest of the system just for
sequences.

Besides there's a related compatibility break in that,
if a sequence is created in an existing release like this:

CREATE SEQUENCE s MINVALUE -9223372036854775808;

And then it's dumped/reloaded on a backend that has
the patch applied, it fails with:

MINVALUE (-9223372036854775808) is too large for sequence data type bigint

This value (-2^63) is legal in current releases, although it happens
to be off-by-1 compared to the default minvalue for a sequence going
downwards. Arguably it's the default that is weird.

I've started the thread at [1]/messages/by-id/4865a75e-f490-4e9b-b8e7-3d78694c3178@manitou-mail.org to discuss whether it makes sense
in the first place that our CREATE SEQUENCE takes -(2^63)+1 as the
default minvalue rather than -2^63, independantly of this patch.

I think the current patch transforms this oddity into an actual
issue by making it impossible to reach the real minimum of
a sequence with regard to the type tied to it (-2^15 for smallint,
-2^31 for int, -2^63 for bigint), whereas in HEAD you can still
adjust minvalue to fix the off-by-1 against the bigint range, if you
happen to care about it, the only problem being that you first
need to figure this out by yourself.

[1]: /messages/by-id/4865a75e-f490-4e9b-b8e7-3d78694c3178@manitou-mail.org
/messages/by-id/4865a75e-f490-4e9b-b8e7-3d78694c3178@manitou-mail.org

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

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

Peter Eisentraut wrote:

So in order to correctly answer the question, is the sequence about to
run out, you need to look not only at
the sequence but also any columns it is associated with. check_postgres
figures this out, but it's complicated and slow, and not easy to do
manually.

It strikes me that this is a compelling argument for setting a sensible
MAXVALUE when creating a sequence for a SERIAL, rather than binding
the sequence to a datatype.

In existing releases the SERIAL code sets the maxvalue to 2^63 even
though it knows that the column is limited to 2^31.
It looks like setting it to 2^31 would be enough for the sake of
monitoring.

More generally, what is of interest for the monitoring is how close
the sequence's last_value is from its max_value, independently of an
underlying type.

2^{15,31,63} are specific cases of particular interest, but there's
no reason to only check for these limits when you can do it
for every possible limit.

For instance if a user has a business need to limit an ID to 1 billion,
they should alter the sequence to a MAXVALUE of 1 billion, and be
interested in how close they are from that, not from 2^31.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

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

Here is an updated patch that allows changing the sequence type. This
was clearly a concern for reviewers, and the presented use cases seemed
convincing.

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

On Wed, Jan 25, 2017 at 11:53 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

Here is an updated patch that allows changing the sequence type. This
was clearly a concern for reviewers, and the presented use cases seemed
convincing.

I have been torturing this patch and it looks rather solid to me. Here
are a couple of comments:

@@ -15984,6 +15992,9 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
"CREATE SEQUENCE %s\n",
fmtId(tbinfo->dobj.name));

+   if (strcmp(seqtype, "bigint") != 0)
+       appendPQExpBuffer(query, "    AS %s\n", seqtype);
Wouldn't it be better to assign that unconditionally? There is no
reason that a dump taken from pg_dump version X will work on X - 1 (as
there is no reason to not make the life of users uselessly difficult
as that's your point), but that seems better to me than rely on the
sequence type hardcoded in all the pre-10 dump queries for sequences.
That would bring also more consistency to the CREATE SEQUENCE queries
of test_pg_dump/t/001_base.pl.

Could you increase the regression test coverage to cover some of the
new code paths? For example such cases are not tested:
=# create sequence toto as smallint;
CREATE SEQUENCE
=# alter sequence toto as smallint maxvalue 1000;
ERROR: 22023: RESTART value (2147483646) cannot be greater than MAXVALUE (1000)
LOCATION: init_params, sequence.c:1537
=# select setval('toto', 1);
setval
--------
1
(1 row)
=# alter sequence toto as smallint;
ERROR: 22023: MAXVALUE (2147483647) is too large for sequence data
type smallint
LOCATION: init_params, sequence.c:1407

+   if ((seqform->seqtypid == INT2OID && seqform->seqmin < PG_INT16_MIN)
+       || (seqform->seqtypid == INT4OID && seqform->seqmin < PG_INT32_MIN)
+       || (seqform->seqtypid == INT8OID && seqform->seqmin < PG_INT64_MIN))
+   {
+       char        bufm[100];
+
+       snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
+
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("MINVALUE (%s) is too large for sequence data type %s",
+                       bufm, format_type_be(seqform->seqtypid))));
+   }
"large" does not apply to values lower than the minimum, no? The int64
path is never going to be reached (same for the max value), it doesn't
hurt to code it I agree.

Testing serial columns, the changes are consistent with the previous releases.
--
Michael

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

On 1/25/17 11:57 PM, Michael Paquier wrote:

@@ -15984,6 +15992,9 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
"CREATE SEQUENCE %s\n",
fmtId(tbinfo->dobj.name));

+   if (strcmp(seqtype, "bigint") != 0)
+       appendPQExpBuffer(query, "    AS %s\n", seqtype);
Wouldn't it be better to assign that unconditionally? There is no
reason that a dump taken from pg_dump version X will work on X - 1 (as
there is no reason to not make the life of users uselessly difficult
as that's your point), but that seems better to me than rely on the
sequence type hardcoded in all the pre-10 dump queries for sequences.

Generally, we don't add default values, to keep the dumps from being too
verbose.

(I also think that being able to restore dumps to older versions would
be nice, but that's another discussion.)

Could you increase the regression test coverage to cover some of the
new code paths? For example such cases are not tested:
=# create sequence toto as smallint;
CREATE SEQUENCE
=# alter sequence toto as smallint maxvalue 1000;
ERROR: 22023: RESTART value (2147483646) cannot be greater than MAXVALUE (1000)
LOCATION: init_params, sequence.c:1537

Yeah, I had taken some notes along the way to add more test coverage, so
since you're interested, attached is a patch. It's independent of the
current patch and overlaps slightly. The example you show isn't really
a new code path, just triggered differently, but the enhanced tests will
cover it nonetheless.

=# alter sequence toto as smallint;
ERROR: 22023: MAXVALUE (2147483647) is too large for sequence data
type smallint
LOCATION: init_params, sequence.c:1407

+   if ((seqform->seqtypid == INT2OID && seqform->seqmin < PG_INT16_MIN)
+       || (seqform->seqtypid == INT4OID && seqform->seqmin < PG_INT32_MIN)
+       || (seqform->seqtypid == INT8OID && seqform->seqmin < PG_INT64_MIN))
+   {
+       char        bufm[100];
+
+       snprintf(bufm, sizeof(bufm), INT64_FORMAT, seqform->seqmin);
+
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                errmsg("MINVALUE (%s) is too large for sequence data type %s",
+                       bufm, format_type_be(seqform->seqtypid))));
+   }
"large" does not apply to values lower than the minimum, no? The int64
path is never going to be reached (same for the max value), it doesn't
hurt to code it I agree.

Yeah, I think that should be rephrased as something like "out of
bounds", which is the term used elsewhere.

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