Skip to content

MDEV-38830: SIGSEGV and UBSAN null-pointer-use in TABLE::evaluate_upd…#4907

Open
bnestere wants to merge 1 commit into12.3from
12.3-MDEV-38830
Open

MDEV-38830: SIGSEGV and UBSAN null-pointer-use in TABLE::evaluate_upd…#4907
bnestere wants to merge 1 commit into12.3from
12.3-MDEV-38830

Conversation

@bnestere
Copy link
Copy Markdown
Contributor

@bnestere bnestere commented Apr 6, 2026

MDEV-38716 fixes were incomplete. It still allowed a table's default_fields to be left un-restored after an ALTER table finished its copy_data_between_fields().

This patch actually matches the original conception of MDEV-38716. It was later changed after finding a test failure in maria.alter, where I thought the patch broke that test. But actually, maria.alter itself relies on somewhat broken/inconsistent server behavior.

It is the MDEV-19055 extension of the test which broke. To summarize the broken part of the test, first, it creates a temporary table t2, adds a new column of type DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, and adds a constraint check on that new type:

CREATE TEMPORARY TABLE t2 (a TIME) ENGINE=Aria;
ALTER TABLE t2 ADD b DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP;
ALTER TABLE t2 ADD CHECK (b = 4);

The next part results in inconsistent behavior:

INSERT IGNORE INTO t2 () VALUES (),(),(),();

What is not defined is "what should this do"?

Prior to this patch / MDEV-38716, this would create 4 new rows, each with a zeroed out timestamp for b. This is due to a the default_field not resetting after the ALTER TABLE, thus the DEFAULT CURRENT_TIMESTAMP clause is lost after the ALTER TABLE ADD CHECK.

With this patch, because the default_field is restored after the ALTER, there is no affect of the INSERT IGNORE INTO t2. I.e., t2 is empty after this insert.

This change in results further changes how an ALTER TABLE behaves later in the test:

SET SQL_MODE= 'STRICT_ALL_TABLES';
SELECT count(a),sum(a) FROM t2;
--error ER_TRUNCATED_WRONG_VALUE
ALTER TABLE t2 CHANGE IF EXISTS d c INT;

Without this patch, there are rows in t2, and so this ALTER TABLE results in an error. With this patch, t2 is empty, and therefore there is no data that was truncated to warn about.

Also note, the result of

INSERT IGNORE INTO t2 () VALUES (),(),(),();

is very inconsistent overall. For example, if t2 is a regular table (rather than a temporary table), the pre-patch result is also NULL. However, if values are explicitly provided (in any case), the result is truncated and rows are added to the table. E.g.

INSERT IGNORE INTO t2 (b) VALUES (1),(2),(3),(5);

will result in 4 rows with truncated timestamps in any case.

See also MDEV-30115 for more problems with INSERT IGNORE and CHECK.

…ate_default_function on UPDATE

MDEV-38716 fixes were incomplete. It still allowed a table's
default_fields to be left un-restored after an ALTER table finished its
copy_data_between_fields().

This patch actually matches the original conception of MDEV-38716. It
was later changed after finding a test failure in maria.alter, where I
thought the patch broke that test. But actually, maria.alter itself
relies on somewhat broken/inconsistent server behavior.

It is the MDEV-19055 extension of the test which broke. To summarize the
broken part of the test, first, it creates a temporary table t2, adds a
new column of type DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, and adds
a constraint check on that new type:

CREATE TEMPORARY TABLE t2 (a TIME) ENGINE=Aria;
ALTER TABLE t2 ADD b DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP;
ALTER TABLE t2 ADD CHECK (b = 4);

The next part results in inconsistent behavior:

INSERT IGNORE INTO t2 () VALUES (),(),(),();

What is not defined is "what should this do"?

Prior to this patch / MDEV-38716, this would create 4 new rows, each
with a zeroed out timestamp for `b`. This is due to a the default_field
not resetting after the ALTER TABLE, thus the DEFAULT CURRENT_TIMESTAMP
clause is lost after the ALTER TABLE ADD CHECK.

With this patch, because the default_field is restored after the ALTER,
there is no affect of the INSERT IGNORE INTO t2. I.e., t2 is empty after
this insert.

This change in results further changes how an ALTER TABLE behaves later
in the test:

SET SQL_MODE= 'STRICT_ALL_TABLES';
SELECT count(a),sum(a) FROM t2;
--error ER_TRUNCATED_WRONG_VALUE
ALTER TABLE t2 CHANGE IF EXISTS d c INT;

Without this patch, there are rows in t2, and so this ALTER TABLE
results in an error. With this patch, t2 is empty, and therefore there
is no data that was truncated to warn about.

Also note, the result of

INSERT IGNORE INTO t2 () VALUES (),(),(),();

is very inconsistent overall. For example, if t2 is a regular table
(rather than a temporary table), the pre-patch result is also NULL.
However, if values are explicitly provided (in any case), the result is
truncated and rows are added to the table. E.g.

INSERT IGNORE INTO t2 (b) VALUES (1),(2),(3),(5);

will result in 4 rows with truncated timestamps in any case.

See also MDEV-30115 for more problems with INSERT IGNORE and CHECK.

Reviewed-by: Monty <monty@mariadb.com>
Signed-off-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant