Hi, Nikita, On Jan 06, Nikita Malyavin wrote:

revision-id: 22e2f082400 (mariadb-10.5.23-39-g22e2f082400) parent(s): e472b682e06 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2023-12-20 02:46:09 +0100 message:

MDEV-25370 Update for portion changes autoincrement key in bi-temp table

According to the standard, the autoincrement column (i.e. *identity column*) should be advanced each insert implicitly made by UPDATE/DELETE ... FOR PORTION.

When you refer to the standard, specify the exact reference so that someone could verify it. E.g. as I've commented in MDEV, "SQL:2016, Part 2, 15.13 Effect of replacing rows in base tables, paragraph 10) b) i)" Also, the standard doesn't have autoincrement, it'd be better to say "According to the standard, the identity column (which autoincrement is a functional analog of) should be advanced...."

This is very unconvenient use in several notable cases. Concider a WITHOUT OVERLAPS key with an autoinc column: id int auto_increment, unique(id, p without overlaps)

An update or delete with FOR PORTION creates a sense that id will remain unchanged in such case.

This commit makes an exception the autoinc columns that are part of WITHOUT OVERLAPS keys and infers the following rule for the newly inserted rows:

Better to say here not that we're making an exception and violating the standard (because we are not), but that in the standard IDENTITY uses its own independent sequence generator, which knows nothing about periods. While autoincrement always requires an index (that is, our "identity" column is a pair {autoincrement, index}, not autoincrement alone), and the index can be declared WITHOUT OVERLAPS, so our "identity" column is period-aware. This is an extension of the standard, and we can freely define how it behaves in the cases not covered by the standard, that is in the WITHOUT OVERLAPS case.

===== Let V_j a set of values for currently updating/deleting row. Let VI_j a set of values to be inserted. * Case: (a) if V_i is an identity column that is also a part of a unique constraint that includes <without overlap specification>, then let VI_i be V_i (b) otherwise, let VI_i be the value deduced as described by the rules of ISO/IEC 9075-2, clauses 15.7 Effect of deleting rows from base tables and 15.13 Effect of replacing rows in base tables. =====

That's confusing, I'm sure that anyone who doesn't remember the corresponding part of the sql standard will think it's a direct quote from it. You've imitated the style rather convincingly :) It's also rather unnecessary, we don't have identity columns, so nobody cares how they should behave in this case. Perhaps it'd be better if you simply write that if the key, that defines autoincrement values, is declared WITHOUT OVERLAPS, then new rows created by UPDATE/DELETE FOR PORTION OF will use old autoincrement values, as they belong to non-overlapping ranges. But INSERT will generate a new autoincrement value, it will not try to find the smallest value in the overlapping ranges.

Note that this also infers that if an identity column is not a part of any unique constraint that includes <without overlap specification>, then its value will be DEFAULT and, that is, auto-incremented.

Apart from WITHOUT OVERLAPS there is also another notable case, referred by the reporter - a unique key that has an autoincrement column and a field from the period specification: id int auto_increment, unique(id, s), period for p(s, e)

for this case, no exception is made, and the autoincrementing rules will be proceeded accordung to the standard (i.e. the value will be advanced on implicit inserts).

diff --git a/mysql-test/suite/period/r/overlaps.result b/mysql-test/suite/period/r/overlaps.result --- a/mysql-test/suite/period/r/overlaps.result +++ b/mysql-test/suite/period/r/overlaps.result @@ -449,3 +449,86 @@ VALUES ('2000-01-01 00:00:00.000000', '2001-01-01 00:00:00.000000', 'abc'); ERROR 23000: Duplicate entry 'abc-2001-01-01 00:00:00.000000-2000-01-01 00:00:00.000000' for key 'index_name' DROP TABLE t1; +create or replace table cars(id int auto_increment, +price int, s date, e date, +period for p(s,e), +primary key(id, p without overlaps)); +insert into cars(price, s, e) values (1000, '2018-01-01', '2020-01-01'); +select * from cars; +id price s e +1 1000 2018-01-01 2020-01-01 +update cars for portion of p from '2019-01-01' to '2019-12-01' set price= 1100; +select * from cars; +id price s e +1 1000 2018-01-01 2019-01-01 +1 1000 2019-12-01 2020-01-01 +1 1100 2019-01-01 2019-12-01 +delete from cars for portion of p from '2019-12-10' to '2019-12-20'; +select * from cars; +id price s e +1 1000 2018-01-01 2019-01-01 +1 1000 2019-12-01 2019-12-10 +1 1000 2019-12-20 2020-01-01 +1 1100 2019-01-01 2019-12-01

ok

+# AUTO_INCREMENT field is separate from WITHOUT OVERLAPS +create or replace table cars(id int primary key auto_increment, +car_id int, +price int, s date, e date, +period for p(s,e), +unique(car_id, p without overlaps)); +insert cars(car_id, price, s, e) values (1, 1000, '2018-01-01', '2020-01-01'); +select * from cars; +id car_id price s e +1 1 1000 2018-01-01 2020-01-01 +update cars for portion of p from '2019-01-01' to '2019-12-01' set price= 1100; +select * from cars; +id car_id price s e +1 1 1100 2019-01-01 2019-12-01 +2 1 1000 2018-01-01 2019-01-01 +3 1 1000 2019-12-01 2020-01-01 +delete from cars for portion of p from '2019-12-10' to '2019-12-20'; +select * from cars; +id car_id price s e +1 1 1100 2019-01-01 2019-12-01 +2 1 1000 2018-01-01 2019-01-01 +4 1 1000 2019-12-01 2019-12-10 +5 1 1000 2019-12-20 2020-01-01

right

+# AUTO_INCREMENT field is both standalone and in WITHOUT OVERLAPS +create or replace table cars(id int primary key auto_increment, +price int, s date, e date, +period for p(s,e), +unique(id, p without overlaps)); +insert cars(price, s, e) values (1000, '2018-01-01', '2020-01-01'); +select * from cars; +id price s e +1 1000 2018-01-01 2020-01-01 +update cars for portion of p from '2019-01-01' to '2019-12-01' set price= 1100; +ERROR 23000: Duplicate entry '1' for key 'PRIMARY'

uhm, I don't know. I'd expect that here PRIMARY would define autoincrement values.

+truncate cars; +insert cars(price, s, e) values (1000, '2018-01-01', '2020-01-01'); +delete from cars for portion of p from '2019-12-10' to '2019-12-20'; +ERROR 23000: Duplicate entry '1' for key 'PRIMARY' +# AUTO_INCREMENT field is non-unique

not sure here. I think here unique should define autoincrement values. That is, in both cases, the most restrictive index should define.

+create or replace table cars(id int auto_increment, key(id), +car_id int, +price int, s date, e date, +period for p(s,e), +unique(car_id, p without overlaps)); +insert cars(car_id, price, s, e) values (1, 1000, '2018-01-01', '2020-01-01'); +select * from cars; +id car_id price s e +1 1 1000 2018-01-01 2020-01-01 +update cars for portion of p from '2019-01-01' to '2019-12-01' set price= 1100; +select * from cars; +id car_id price s e +1 1 1100 2019-01-01 2019-12-01 +2 1 1000 2018-01-01 2019-01-01 +3 1 1000 2019-12-01 2020-01-01 +delete from cars for portion of p from '2019-12-10' to '2019-12-20'; +select * from cars; +id car_id price s e +1 1 1100 2019-01-01 2019-12-01 +2 1 1000 2018-01-01 2019-01-01 +4 1 1000 2019-12-01 2019-12-10 +5 1 1000 2019-12-20 2020-01-01

please also add test for INSERT. Just add a second insert like insert cars(price, s, e) values (1000, '2021-01-01', '2022-01-01'); to show that id will *not* be 1 here, even though 1 would not conflict with anything in the specified date range. Simply to have the expected behavior recorded as a test result.

+drop table cars; diff --git a/sql/table.cc b/sql/table.cc --- a/sql/table.cc +++ b/sql/table.cc @@ -8917,6 +8916,21 @@ int TABLE::update_generated_fields() return res; }

+void TABLE::period_prepare_autoinc() +{ + if (!found_next_number_field) + return; + + key_map::Iterator it(found_next_number_field->part_of_key_not_clustered); + for (uint i; (i= it++) != key_map::Iterator::BITMAP_END;) + { + KEY &key= key_info[i]; + if (key.without_overlaps) + return; + }

No, I don't think you need to check *all* indexes that the field is part of. I believe you should only check the "main" index, the one that defines autoincrement field behavior. That is only if (key_info[s->next_number_index].without_overlaps) return;

+ next_number_field= found_next_number_field; +} + int TABLE::period_make_insert(Item *src, Field *dst) { THD *thd= in_use;

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org