cybertec-postgresql / db_migrator

Other
21 stars 8 forks source link

Sequence nextval computation #38

Closed fljdin closed 1 year ago

fljdin commented 1 year ago

Hello,

As you will see in examples below, sequence nextval may encounter various unwanted values


Oracle

SQL> CREATE USER test IDENTIFIED BY "test";
SQL> CREATE SEQUENCE test.seq1 START WITH 10 INCREMENT BY 10 NOCACHE;
SQL> SELECT test.seq1.nextval FROM DUAL;

   NEXTVAL
----------
        10

SQL> SELECT test.seq1.nextval FROM DUAL;

   NEXTVAL
----------
        20
SELECT db_migrate(
   plugin => 'ora_migrator',
   server => 'oracle',
   only_schemas => ARRAY['TEST']
);

SELECT nextval('test.seq1');
 nextval 
---------
     31
-- should be 30

MySQL

mysql> CREATE DATABASE test;
mysql> CREATE TABLE test.table1(id INT AUTO_INCREMENT, PRIMARY KEY(id));
mysql> INSERT INTO test.table1 (id) VALUES (null), (null);
mysql> SELECT * FROM test.table1;
+----+
| id |
+----+
|  1 |
|  2 |
+----+
SELECT db_migrate(
   plugin => 'mysql_migrator',
   server => 'mysql',
   only_schemas => ARRAY['test']
);

SELECT nextval('test.table1_seq');
 nextval
---------
       4
-- should be 3

MS SQL

1> CREATE SCHEMA test;
2> GO
1> CREATE SEQUENCE test.seq1 START WITH 10 INCREMENT BY 10 NO CACHE;
2> GO
1> SELECT NEXT VALUE FOR test.seq1;
2> GO

--------------------
                  10
1> SELECT NEXT VALUE FOR test.seq1;
2> GO

--------------------
                  20
SELECT db_migrate(
   plugin => 'mssql_migrator',
   server => 'mssql',
   only_schemas => ARRAY['test']
);

SELECT nextval('test.seq1');
 nextval
---------
      21
-- should be 30
laurenz commented 1 year ago

Hm. Do you think that this would solve the problem:

--- a/db_migrator--1.1.0.sql
+++ b/db_migrator--1.1.0.sql
@@ -1122,9 +1122,9 @@ BEGIN
          stmt := format('CREATE SEQUENCE %I.%I INCREMENT %s %s %s START %s CACHE %s %sCYCLE',
                      sch, seq, incr,
                      CASE WHEN minv IS NULL THEN 'NO MINVALUE' ELSE 'MINVALUE ' || minv END,
                      CASE WHEN maxv IS NULL THEN 'NO MAXVALUE' ELSE 'MAXVALUE ' || maxv END,
-                     lastval + 1, cachesiz,
+                     lastval + incr, cachesiz,
                      CASE WHEN cycl THEN '' ELSE 'NO ' END);

          IF NOT execute_statements(
             operation => 'create sequence',
fljdin commented 1 year ago

This may works in most case, yes. With a CACHED sequence, a more important gap occurs and may be acceptable.

laurenz commented 1 year ago

Hm, I don't understand. In ora_migrator, I set sequences.last_value from dba_sequences.last_value, which is documented as:

Last sequence number written to disk. If a sequence uses caching, the number written to disk is the last number placed in the sequence cache. This number is likely to be greater than the last sequence number that was used.

The first sentence seems to suggest that the current code is fine. But the "likely to" in the second sentence makes me worry.

Can you explain in more detail where you see a problem?

fljdin commented 1 year ago

I've made some new tests on Oracle with a CACHEd sequence :

SQL> CREATE SEQUENCE test.seq1 START WITH 10 INCREMENT BY 10 CACHE 20;
SQL> SELECT test.seq1.nextval FROM DUAL;

   NEXTVAL
----------
        10

SQL> SELECT test.seq1.nextval FROM DUAL;

   NEXTVAL
----------
        20
SELECT db_migrate(
   plugin => 'ora_migrator',
   server => 'oracle',
   only_schemas => ARRAY['TEST']
);

SELECT nextval('test.seq1');
 nextval 
---------
     221
-- should be 30

It seems that the cached values are deducted from the value contained in the dba_sequences.last_value column. In my opinion, db_migrator should not increment by 1 the sequence during staging refresh. Plugins (oracle, mysql or mssql) should be aware of internal management of sequences to provide a value as close as possible to the expected one.

laurenz commented 1 year ago

I think it is fine if the sequence skips ahead some, but the INCREMENT should be respected. My patch from earlier in the discussion should take care of that, right?