Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support PostgreSQL 10 whose sequences do not know increment_by #28789

Closed
wants to merge 1 commit into from

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented Apr 18, 2017

Summary

Addresses #28780

  • Each sequence does not know increment_by or min_value since
    PostgreSQL 10. A new system catalog pg_sequences knows both. postgres/postgres@1753b1b

  • Since pg_sequences does not exist PostgreSQL 9.6 or earlier version
    This commit addresses by removing increment_by or min_value

  • setval 3rd argument needs to set to false only when the table has
    no rows to avoid nextval(<sequence_name>) returns 2 where 1 is
    expected.

  • Replaced min_value with 1 since it is necessary only when
    the table has no rows. If someone wants to reset sequence to non 1 when table has no rows new pull requests/commits required.

…min_value`

Addresses rails#28780

- Each sequence does not know  `increment_by` or `min_value` since
PostgreSQL 10. A new system catalog `pg_sequences` knows both.

postgres/postgres@1753b1b

- Since `pg_sequences` does not exist PostgreSQL 9.6 or earlier version
This commit addresses by removing `increment_by` or `min_value`

- `setval` 3rd argument needs to set to `false` only when the table has
no rows to avoid `nextval(<sequence_name>)` returns 2 where 1 is
expected.

- Replaced `min_value` with `1` since it is necessary only when
the table has no rows. If someone wants to reset sequence to non 1 when
table has no rows new pull requests/commits required.
@rails-bot
Copy link

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@pixeltrix
Copy link
Contributor

@yahonda will next_val automatically return min_value if you setval to 1?

@yahonda
Copy link
Member Author

yahonda commented Apr 19, 2017

No. The value set at setval must be equal or higher than the min_value. It shows this error:
ERROR: setval: value 1 is out of bounds for sequence "test_seq_min_value_100" (100..9223372036854775807)

  • PostgreSQL version
postgres=# create database test;
CREATE DATABASE
postgres=# \q
$ psql -d test
psql (10devel)
Type "help" for help.
  • create a sequence with minvalue 100
CREATE SEQUENCE
test=# select nextval('test_seq_min_value_100');
 nextval
---------
     100
(1 row)

test=# select nextval('test_seq_min_value_100');
 nextval
---------
     101
(1 row)

test=# select nextval('test_seq_min_value_100');
 nextval
---------
     102
(1 row)

test=# select nextval('test_seq_min_value_100');
 nextval
---------
     103
(1 row)

test=# select nextval('test_seq_min_value_100');
 nextval
---------
     104
(1 row)

test=# select nextval('test_seq_min_value_100');
 nextval
---------
     105
(1 row)

test=# select setval('test_seq_min_value_100', 1, true);
ERROR:  setval: value 1 is out of bounds for sequence "test_seq_min_value_100" (100..9223372036854775807)
test=# select setval('test_seq_min_value_100', 1, false);
ERROR:  setval: value 1 is out of bounds for sequence "test_seq_min_value_100" (100..9223372036854775807)

@pixeltrix
Copy link
Contributor

Dang - I think we need to address somehow in this PR

@yahonda
Copy link
Member Author

yahonda commented Apr 19, 2017

To get the min_value we will need to retrieve it from pg_sequences starting from PostgreSQL 10.

This pull request now aims to create the same statement available for whichever PostgreSQL version.
But we may need to consider to implement if postgresql_version >= 100000 kind of approach.

activerecord_unittest=# \x
Expanded display is on.
activerecord_unittest=# select * FROM "public"."topics_id_seq";
-[ RECORD 1 ]---
last_value | 100
log_cnt    | 0
is_called  | t
activerecord_unittest=# select * from pg_sequences where schemaname = 'public' and sequencename = 'topics_id_seq';
-[ RECORD 1 ]-+--------------------
schemaname    | public
sequencename  | topics_id_seq
sequenceowner | yahonda
data_type     | bigint
start_value   | 1
min_value     | 1
max_value     | 9223372036854775807
increment_by  | 1
cycle         | f
cache_size    | 1
last_value    | 100

activerecord_unittest=#

@yahonda
Copy link
Member Author

yahonda commented Apr 19, 2017

Let me update this pull request using if postgresql_version >= 100000 approach.

@pixeltrix
Copy link
Contributor

Is there any discussions on this topic in the PostgreSQL community - seems like a very abrupt change for users to deal with.

@yahonda
Copy link
Member Author

yahonda commented Apr 19, 2017

@rafaelfranca
Copy link
Member

@pixeltrix assigning to you since you already got the initial context

@pixeltrix
Copy link
Contributor

@yahonda is there a way of use ALTER SEQUENCE to do what we want?

@yahonda
Copy link
Member Author

yahonda commented Apr 20, 2017

alter sequence statement also requires start value is equal or higher than minvalue.

  • Error message
test=#
test=# alter sequence test_seq_for_alter_sequence_min_value_100 start 1;
ERROR:  START value (1) cannot be less than MINVALUE (100)
test=#
  • Test output
test=# create sequence test_seq_for_alter_sequence_min_value_100 minvalue 100;
CREATE SEQUENCE
test=# \x
Expanded display is on.
test=# select * from test_seq_for_alter_sequence_min_value_100;
-[ RECORD 1 ]---
last_value | 100
log_cnt    | 0
is_called  | f

test=# select * from pg_sequences where sequencename = 'test_seq_for_alter_sequence_min_value_100';
-[ RECORD 1 ]-+------------------------------------------
schemaname    | public
sequencename  | test_seq_for_alter_sequence_min_value_100
sequenceowner | yahonda
data_type     | bigint
start_value   | 100
min_value     | 100
max_value     | 9223372036854775807
increment_by  | 1
cycle         | f
cache_size    | 1
last_value    |

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 100

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 101

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 102

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 103

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 104

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 105

test=#
test=# alter sequence test_seq_for_alter_sequence_min_value_100 start 1;
ERROR:  START value (1) cannot be less than MINVALUE (100)
test=#

@pixeltrix
Copy link
Contributor

@yahonda there's a RESTART option that can either be blank or WITH <value>

@yahonda
Copy link
Member Author

yahonda commented Apr 20, 2017

Yeah, there is an interesting option. But actually restart is same as setting is_called to false at setval.

https://www.postgresql.org/docs/9.6/static/sql-altersequence.html

This is equivalent to calling the setval function with is_called = false:

test=# alter sequence test_seq_for_alter_sequence_min_value_100 restart 1;
ERROR:  RESTART value (1) cannot be less than MINVALUE (100)

What we can do is setting minvalue = 1 to override the current minvalue as follows.
There will be no differences from the current pull request implementation since it sets setval 2nd argument as 1 when the table is empty.

test=# alter sequence test_seq_for_alter_sequence_min_value_100 minvalue 1 restart 1;
ALTER SEQUENCE
test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 1

test=#
  • Test
test=# \q
[yahonda@li366-176 ~]$ psql -d test
psql (10devel)
Type "help" for help.

test=# create sequence test_seq_for_alter_sequence_min_value_100 minvalue 100;
CREATE SEQUENCE
test=# \x
Expanded display is on.
test=# select * from test_seq_for_alter_sequence_min_value_100;
-[ RECORD 1 ]---
last_value | 100
log_cnt    | 0
is_called  | f

test=# select * from pg_sequences where sequencename = 'test_seq_for_alter_sequence_min_value_100';
-[ RECORD 1 ]-+------------------------------------------
schemaname    | public
sequencename  | test_seq_for_alter_sequence_min_value_100
sequenceowner | yahonda
data_type     | bigint
start_value   | 100
min_value     | 100
max_value     | 9223372036854775807
increment_by  | 1
cycle         | f
cache_size    | 1
last_value    |

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 100

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 101

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 102

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 103

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 104

test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 105

test=# alter sequence test_seq_for_alter_sequence_min_value_100 restart 1;
ERROR:  RESTART value (1) cannot be less than MINVALUE (100)
test=# alter sequence test_seq_for_alter_sequence_min_value_100 minvalue 1 restart 1;
ALTER SEQUENCE
test=# select nextval('test_seq_for_alter_sequence_min_value_100');
-[ RECORD 1 ]
nextval | 1

test=#

@pixeltrix
Copy link
Contributor

I guess we have to go with a if postgresql_version >= 100000 approach then 😞

I suspect this will be the first of many such conditionals.

@yahonda
Copy link
Member Author

yahonda commented Apr 20, 2017

Agreed. Let me update the commit to handle if postgresql_version >= 100000.

@yahonda
Copy link
Member Author

yahonda commented May 2, 2017

Closing this pull request. Opened another pull request #28864 which handles min_value correctly.

@yahonda yahonda closed this May 2, 2017
yahonda added a commit to yahonda/rails that referenced this pull request May 30, 2017
Another fix for rails#28780 based on discussions at rails#28789

- In PostgreSQL 10 each sequence does not know its `min_value`.
A new system catalog `pg_sequence` shows it as `seqmin`.
Refer postgres/postgres@1753b1b

- `setval` 3rd argument needs to set to `false` only when the table has no rows
to avoid `nextval(<sequence_name>)` returns `2` where `1` is expected.

- `min_value` is only necessary when the table has no rows. It used to be necessary
since the 3rd argument of `setval` is always `false`.
@yahonda yahonda deleted the remove_increment_by branch June 16, 2017 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants