database/mariadb
database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2025-04-18 21:44:20 +03:00
Code

MDEV-33734 Improve the sequence increment inequality testing

Browse Source 
We add an extra condition that makes the inequality testing in
SEQUENCE::increment_value() mathematically watertight, and we cast to
and from unsigned in potential underflow and overflow addition and
subtractions to avoid undefined behaviour.

Let's start by distinguishing between c++ expressions and mathematical
expressions. by c++ expression I mean an expression with the outcome
determined by the compiler/runtime. by mathematical expression I mean
an expression whose value is mathematically determined. So a c++
expression -9223372036854775806 - 1000 at worst can evaluate to any
value due to underflow. A mathematical expression -9223372036854775806
- 1000 evaluates to -9223372036854776806.

The problem boils down to how to write a c++ expression equivalent to
an mathematical expression x + y < z where x and z can take any values
of long long int, and y < 0 is also a long long int. Ideally we want
to avoid underflow, but I'm not sure how this can be done.

The correct c++ form should be (x + y < z || x < z - y || x < z).
Let M=9223372036854775808 i.e. LONGLONG_MAX + 1. We have

-M < x < M - 1
-M < y < 0
-M < z < M - 1

Let's consider the case where x + y < z is true as a mathematical
expression.

If the first disjunct underflows, i.e. the mathematical expression x
+ y < -M. If the arbitrary value resulting from the underflow causes
the c++ expression to hold too, then we are done. Otherwise we move
onto the next expression x < z - y. If there's no overflow in z
- y then we are done. If there's overflow i.e. z - y > M - 1,
and the c++ expression evals to false, then we are onto x < z.
There's no over or underflow here, and it will eval to true. To see
this, note that

x + y < -M means x < -M - y < -M - (-M) = 0
z - y > M - 1 means z > y + M - 1 > - M + M - 1 = -1
so x < z.

Now let's consider the case where x + y < z is false as a mathematical
expression.

The first disjunct will not underflow in this case, so we move to (x <
z - y). This will not overflow. To see this, note that

x + y >= z means z - y <= x < M - 1

So it evals to false too. And the third disjunct x < z also evals to
false because x >= z - y > z.

I suspect that in either case the expression x < z does not determine
the final value of the disjunction in the vast majority cases, which
is why we leave it as the final one in case of the rare cases of both
an underflow and an overflow happening.

Here's an example of both underflow and overflow happening and the
added inequality x < z saves the day:

x = - M / 2
y = - M / 2 - 1
z = M / 2

x + y evals to M - 1 which is > z
z - y evals to - M + 1 which is < x

We can do the same to test x + y > z where the increment y is positive:

(x > z - y || x + y > z || x > z)

And the same analysis applies to unsigned cases.
This commit is contained in:
Yuchen Pei 2024-03-26 14:37:17 +11:00
parent 9b02b7c77a
commit 7bec41d25d
No known key found for this signature in database
GPG Key ID: 3DD1B35105743563
6 changed files with 51 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
mysql-test/suite/sql_sequence/auto_increment.result
View File

@ -31,8 +31,6 @@ set global auto_increment_increment= default, auto_increment_offset= default;
#
# MDEV-28152 Features for sequence
#
call mtr.add_suppression("signed integer overflow: 42 \\+ 9223372036854775800 cannot be represented in type 'long long int'");
call mtr.add_suppression("signed integer overflow: 9223372036854775800 \\+ 100000 cannot be represented in type 'long long int'");
set global auto_increment_increment= 100;
set global auto_increment_offset= 42;
create sequence s as bigint unsigned start with 9223372036854775800 increment 0;
@ -43,6 +41,10 @@ select next value for s;
next value for s
9223372036854775942
drop sequence s;
set global auto_increment_increment= 100;
set global auto_increment_offset= 5;
create sequence s as bigint start with -9223372036854775805 minvalue -9223372036854775807 maxvalue -9223372036854775800 increment 0;
drop sequence s;
set global auto_increment_increment= default, auto_increment_offset= default;
#
# End of 11.4 tests

11
mysql-test/suite/sql_sequence/auto_increment.test
View File

@ -34,16 +34,19 @@ set global auto_increment_increment= default, auto_increment_offset= default;
--echo # MDEV-28152 Features for sequence
--echo #
# These overflows of signed long long are ok because we are using them
# to represent unsigned long long:
call mtr.add_suppression("signed integer overflow: 42 \\+ 9223372036854775800 cannot be represented in type 'long long int'");
call mtr.add_suppression("signed integer overflow: 9223372036854775800 \\+ 100000 cannot be represented in type 'long long int'");
set global auto_increment_increment= 100;
set global auto_increment_offset= 42;
create sequence s as bigint unsigned start with 9223372036854775800 increment 0;
select next value for s;
select next value for s;
drop sequence s;
set global auto_increment_increment= 100;
set global auto_increment_offset= 5;
# Test underflow
create sequence s as bigint start with -9223372036854775805 minvalue -9223372036854775807 maxvalue -9223372036854775800 increment 0;
drop sequence s;
set global auto_increment_increment= default, auto_increment_offset= default;
--enable_ps2_protocol

1
mysql-test/suite/sql_sequence/next.result
View File

@ -826,7 +826,6 @@ alter sequence t1 cycle;
select next value for t1;
next value for t1
4294967294
call mtr.add_suppression("signed integer overflow: -9223372036854775807 \\+ -1000 cannot be represented in type 'long long int'");
create or replace sequence t1 as bigint start with -9223372036854775807 increment -1;
select next value for t1;
next value for t1

6
mysql-test/suite/sql_sequence/next.test
View File

@ -462,12 +462,8 @@ select next value for t1;
alter sequence t1 cycle;
select next value for t1;
# This overflow caused by the first disjunction bleow is unavoidable and
# taken care of by the second disjunction:
# if (value + increment < min_value || value < min_value - increment)
call mtr.add_suppression("signed integer overflow: -9223372036854775807 \\+ -1000 cannot be represented in type 'long long int'");
create or replace sequence t1 as bigint start with -9223372036854775807 increment -1;
select next value for t1;
--error ER_SEQUENCE_RUN_OUT
select next value for t1;

24
sql/sql_sequence.cc
View File

@ -743,8 +743,11 @@ void sequence_definition::adjust_values(longlong next_value)
global_system_variables.auto_increment_increment);
/*
Ensure that next_free_value has the right offset, so that we
can generate a serie by just adding real_increment.
Ensure that next_free_value has the right offset, so that we can
generate a serie by just adding real_increment. The goal is to
adjust next_free_value upwards such that
next_free_value % real_increment == offset
*/
off= next_free_value % real_increment;
if (off < 0)
@ -760,15 +763,18 @@ void sequence_definition::adjust_values(longlong next_value)
need to cast to_add.
*/
if ((is_unsigned &&
(ulonglong) next_free_value > (ulonglong) max_value - to_add) ||
(is_unsigned &&
(ulonglong) next_free_value + to_add > (ulonglong) max_value) ||
(!is_unsigned && next_free_value > max_value - to_add) ||
(!is_unsigned && next_free_value + to_add > max_value))
((ulonglong) next_free_value > (ulonglong) max_value - to_add ||
(ulonglong) next_free_value + to_add > (ulonglong) max_value ||
(ulonglong) next_free_value > (ulonglong) max_value)) ||
(!is_unsigned &&
(next_free_value > (longlong) ((ulonglong) max_value - to_add) ||
(longlong) ((ulonglong) next_free_value + to_add) > max_value ||
next_free_value > max_value)))
next_free_value= max_value+1;
else
{
next_free_value+= to_add;
next_free_value=
(longlong) ((ulonglong) next_free_value + (ulonglong) to_add);
if (is_unsigned)
DBUG_ASSERT((ulonglong) next_free_value % real_increment ==
(ulonglong) offset);
@ -898,7 +904,7 @@ longlong SEQUENCE::next_value(TABLE *table, bool second_round, int *error)
next_free_value= increment_value(next_free_value, real_increment);
if (within_bound(res_value, reserved_until, reserved_until,
real_increment > 0))
real_increment > 0))
{
write_unlock(table);
DBUG_RETURN(res_value);

32
sql/sql_sequence.h
View File

@ -205,32 +205,48 @@ private:
(ulonglong) max_value - (ulonglong) increment ||
/* in case max_value - increment underflows */
(ulonglong) value + (ulonglong) increment >
(ulonglong) max_value)
value= max_value + 1;
(ulonglong) max_value ||
/* in case both overflow and underflow happens (very
rarely, if not impossible) */
(ulonglong) value > (ulonglong) max_value)
/* Cast to ulonglong then back, in case max_value ==
LONGLONG_MAX as a ulonglong */
value= (longlong) ((ulonglong) max_value + 1);
else
value+= increment;
value = (longlong) ((ulonglong) value + (ulonglong) increment);
}
else
{
if ((ulonglong) value - (ulonglong) (-increment) <
(ulonglong) min_value ||
(ulonglong) value <
(ulonglong) min_value + (ulonglong) (-increment))
value= min_value - 1;
(ulonglong) min_value + (ulonglong) (-increment) ||
(ulonglong) value < (ulonglong) min_value)
/* Cast to ulonglong then back, in case min_value ==
LONGLONG_MAX + 1 as a ulonglong */
value= (longlong) ((ulonglong) min_value - 1);
else
value+= increment;
value = (longlong) ((ulonglong) value - (ulonglong) (-increment));
}
} else
if (increment > 0)
{
if (value > max_value - increment || value + increment > max_value)
if (value >
(longlong) ((ulonglong) max_value - (ulonglong) increment) ||
(longlong) ((ulonglong) value + (ulonglong) increment) >
max_value ||
value > max_value)
value= max_value + 1;
else
value+= increment;
}
else
{
if (value <= min_value || value + increment < min_value)
if ((longlong) ((ulonglong) value + (ulonglong) increment) <
min_value ||
value <
(longlong) ((ulonglong) min_value - (ulonglong) increment) ||
value < min_value)
value= min_value - 1;
else
value+= increment;