From fb221786dc847c9617e89e6e0e5ca3b6325ff169 Mon Sep 17 00:00:00 2001 From: Arcadiy Ivanov Date: Fri, 4 Sep 2026 16:10:23 -0400 Subject: [PATCH] MDEV-41026 ER_DUP_KEY on '(temporary)' converting a subquery cache A subquery expression cache whose parameter is NULL for many rows fills its in-memory table with rows that all carry a NULL key. `TABLE::add_tmp_key()` builds that key without `HA_NULL_ARE_EQUAL`, `ha_heap::create()` propagates the absence, and `hp_write_key()` then skips the duplicate check for any key value containing NULL, so such rows are legal there. They accumulate rather than being exceptional: `ref.null_rejecting` is set, so every NULL parameter is a miss and adds one more NULL-keyed row. The on-disk creators disagreed. Both the key and the unique constraint forced `HA_NULL_ARE_EQUAL` on, making the destination of a conversion stricter about NULLs than the table being converted from. `create_internal_tmp_table_from_heap()` re-inserts every stored row and treats a duplicate among them as fatal, so the second NULL-keyed row was rejected. The user saw either Can't write; duplicate key in table '(temporary)' or, when the key is too wide for the on-disk engine to index and is built as a unique constraint over a row hash instead, Can't write, because of unique constraint, to table '(temporary)' Make `KEY::flags & HA_NULL_ARE_EQUAL` the single source of truth and read it at all four creator sites rather than deciding it there. For the flag to be truthful the GROUP BY key has to carry it when it is built as a unique constraint as well; the assignment sat inside the branch that only the real-key case takes, which also lays the group buffer out around the NULL flag. The MyISAM halves of both creators compile only with `-DUSE_ARIA_FOR_TMP_TABLES=OFF`, which is not the default. They are changed to match and verified to compile warning-free in that configuration, but **no test run has executed them**. The test covers both on-disk shapes: a narrow key that stays a key, and a key too wide for Aria that becomes a unique constraint. Each block reports whether the conversion happened at all, so the test cannot pass by quietly ceasing to overflow. The count itself is not asserted: `--ps-protocol` executes the statement twice and so converts twice. Neither cached query aborts on error, so a regression in either shape is reported rather than hidden behind the other one. --- .../main/subquery_cache_null_convert.result | 65 ++++++++++ .../main/subquery_cache_null_convert.test | 111 ++++++++++++++++++ sql/sql_select.cc | 41 ++++--- 3 files changed, 200 insertions(+), 17 deletions(-) create mode 100644 mysql-test/main/subquery_cache_null_convert.result create mode 100644 mysql-test/main/subquery_cache_null_convert.test diff --git a/mysql-test/main/subquery_cache_null_convert.result b/mysql-test/main/subquery_cache_null_convert.result new file mode 100644 index 0000000000000..cbb172c83185a --- /dev/null +++ b/mysql-test/main/subquery_cache_null_convert.result @@ -0,0 +1,65 @@ +# +# A subquery cache whose parameter is NULL for many rows fills its +# in-memory table with rows that all have a NULL key. A HEAP unique +# index does not treat two NULLs as equal, so those rows are legal +# there. When the table outgrows the memory limit and is converted to +# the on-disk temporary engine, the copy of the already stored rows +# must not start rejecting them as duplicates. The on-disk table can +# take either shape - a real key, or a unique constraint over a row +# hash when the key is too wide for the engine - and both have to keep +# the NULL semantics the in-memory table was using. +# +# Neither cached query aborts on error, so a regression in either +# shape is reported rather than hidden behind the other one. +# +SET @save_max_heap_table_size = @@max_heap_table_size; +SET @save_optimizer_switch = @@optimizer_switch; +# +# A key narrow enough to stay a key on disk +# +CREATE TABLE t1 (a VARCHAR(64)) ENGINE=Aria; +INSERT INTO t1 VALUES ('aaa'); +INSERT INTO t1 SELECT NULL FROM t1 LIMIT 200; +SELECT COUNT(*), COUNT(a) FROM t1; +COUNT(*) COUNT(a) +712 512 +SET max_heap_table_size = 16*1024; +# The expected answer, established without the cache +SET optimizer_switch = 'subquery_cache=off'; +SELECT COUNT(*) FROM t1 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +COUNT(*) +512 +SET optimizer_switch = @save_optimizer_switch; +# The same query with the cache on, which is what overflows and converts +FLUSH STATUS; +SELECT COUNT(*) FROM t1 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +COUNT(*) +512 +# errno=0 +# converted to disk: 1 +DROP TABLE t1; +# +# A key too wide for the on-disk engine, which turns it into a unique +# constraint over a row hash instead +# +CREATE TABLE t2 (a VARCHAR(700) CHARACTER SET utf8mb4) ENGINE=Aria; +INSERT INTO t2 VALUES ('aaa'); +INSERT INTO t2 SELECT NULL FROM t2 LIMIT 200; +SELECT COUNT(*), COUNT(a) FROM t2; +COUNT(*) COUNT(a) +712 512 +# The expected answer, established without the cache +SET optimizer_switch = 'subquery_cache=off'; +SELECT COUNT(*) FROM t2 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +COUNT(*) +512 +SET optimizer_switch = @save_optimizer_switch; +# The same query with the cache on +FLUSH STATUS; +SELECT COUNT(*) FROM t2 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +COUNT(*) +512 +# errno=0 +# converted to disk: 1 +DROP TABLE t2; +SET max_heap_table_size = @save_max_heap_table_size; diff --git a/mysql-test/main/subquery_cache_null_convert.test b/mysql-test/main/subquery_cache_null_convert.test new file mode 100644 index 0000000000000..eaa7addc897e3 --- /dev/null +++ b/mysql-test/main/subquery_cache_null_convert.test @@ -0,0 +1,111 @@ +--echo # +--echo # A subquery cache whose parameter is NULL for many rows fills its +--echo # in-memory table with rows that all have a NULL key. A HEAP unique +--echo # index does not treat two NULLs as equal, so those rows are legal +--echo # there. When the table outgrows the memory limit and is converted to +--echo # the on-disk temporary engine, the copy of the already stored rows +--echo # must not start rejecting them as duplicates. The on-disk table can +--echo # take either shape - a real key, or a unique constraint over a row +--echo # hash when the key is too wide for the engine - and both have to keep +--echo # the NULL semantics the in-memory table was using. +--echo # +--echo # Neither cached query aborts on error, so a regression in either +--echo # shape is reported rather than hidden behind the other one. +--echo # + +SET @save_max_heap_table_size = @@max_heap_table_size; +SET @save_optimizer_switch = @@optimizer_switch; + +--echo # +--echo # A key narrow enough to stay a key on disk +--echo # +CREATE TABLE t1 (a VARCHAR(64)) ENGINE=Aria; + +# 512 rows carrying the same value, so that the cache accumulates enough +# hits to still be considered worth keeping when it overflows, followed by +# 200 rows with a NULL parameter. Every NULL is a cache miss - a lookup on +# a null-rejecting ref never matches - so each of them adds another +# NULL-keyed row, and the table overflows on those. +INSERT INTO t1 VALUES ('aaa'); +--disable_query_log +--let $i = 0 +while ($i < 9) +{ + --inc $i + INSERT INTO t1 SELECT a FROM t1; +} +--enable_query_log +INSERT INTO t1 SELECT NULL FROM t1 LIMIT 200; +SELECT COUNT(*), COUNT(a) FROM t1; + +SET max_heap_table_size = 16*1024; + +--echo # The expected answer, established without the cache +SET optimizer_switch = 'subquery_cache=off'; +SELECT COUNT(*) FROM t1 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +SET optimizer_switch = @save_optimizer_switch; + +--echo # The same query with the cache on, which is what overflows and converts +FLUSH STATUS; +--disable_abort_on_error +SELECT COUNT(*) FROM t1 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +--let $err = $mysql_errno +--enable_abort_on_error +--echo # errno=$err +# Whether the conversion happened at all is what matters here; the count +# itself is not stable, since --ps-protocol executes the statement twice and +# so converts twice. +--let $disk_tables = query_get_value(SHOW STATUS LIKE 'Created_tmp_disk_tables', Value, 1) +--let $converted = 0 +if ($disk_tables) +{ + --let $converted = 1 +} +--echo # converted to disk: $converted + +DROP TABLE t1; + +--echo # +--echo # A key too wide for the on-disk engine, which turns it into a unique +--echo # constraint over a row hash instead +--echo # +CREATE TABLE t2 (a VARCHAR(700) CHARACTER SET utf8mb4) ENGINE=Aria; + +INSERT INTO t2 VALUES ('aaa'); +--disable_query_log +--let $i = 0 +while ($i < 9) +{ + --inc $i + INSERT INTO t2 SELECT a FROM t2; +} +--enable_query_log +INSERT INTO t2 SELECT NULL FROM t2 LIMIT 200; +SELECT COUNT(*), COUNT(a) FROM t2; + +--echo # The expected answer, established without the cache +SET optimizer_switch = 'subquery_cache=off'; +SELECT COUNT(*) FROM t2 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +SET optimizer_switch = @save_optimizer_switch; + +--echo # The same query with the cache on +FLUSH STATUS; +--disable_abort_on_error +SELECT COUNT(*) FROM t2 WHERE a NOT IN ( SELECT 'x' UNION SELECT 'xx' ); +--let $err = $mysql_errno +--enable_abort_on_error +--echo # errno=$err +# Whether the conversion happened at all is what matters here; the count +# itself is not stable, since --ps-protocol executes the statement twice and +# so converts twice. +--let $disk_tables = query_get_value(SHOW STATUS LIKE 'Created_tmp_disk_tables', Value, 1) +--let $converted = 0 +if ($disk_tables) +{ + --let $converted = 1 +} +--echo # converted to disk: $converted + +DROP TABLE t2; + +SET max_heap_table_size = @save_max_heap_table_size; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fcc18df49212c..9ad6611c38955 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -22563,6 +22563,16 @@ bool Create_tmp_table::finalize(THD *thd, (ha_base_keytype) m_key_part_info->type == HA_KEYTYPE_VARTEXT2) ? 0 : FIELDFLAG_BINARY; m_key_part_info->key_part_flag= 0; + /* + The key is where every temporary-engine creator reads the NULL + semantics from, so a group key has to carry HA_NULL_ARE_EQUAL + whether it ends up as a real key or as a unique constraint: two + NULLs belong in the same group either way. The branch below sets + it for the real-key case, which also needs the group buffer laid + out around the NULL flag. + */ + if (m_using_unique_constraint && maybe_null && field->null_bit) + keyinfo->flags|= HA_NULL_ARE_EQUAL; if (!m_using_unique_constraint) { cur_group->buff=(char*) m_group_buff; @@ -23177,7 +23187,8 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, bzero((char*) &uniquedef,sizeof(uniquedef)); uniquedef.keysegs= keyinfo->user_defined_key_parts; uniquedef.seg=seg; - uniquedef.null_are_equal=1; + /* Propagated, not decided; see the comment on keydef->flag below. */ + uniquedef.null_are_equal= MY_TEST(keyinfo->flags & HA_NULL_ARE_EQUAL); keyinfo->flags|= HA_UNIQUE_HASH; keyinfo->algorithm= HA_KEY_ALG_UNIQUE_HASH; @@ -23198,11 +23209,15 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, /* Create a key */ bzero((char*) keydef,sizeof(*keydef)); /* - We are using a GROUP BY on something that contains NULL - In this case we have to tell Aria that two NULL should - on INSERT be regarded at the same value. + Take the NULL semantics from the key rather than deciding them + here. A key that carries HA_NULL_ARE_EQUAL wants two NULLs in + the same group; a key that does not wants them kept apart, and + the in-memory table it may be converted from has been accepting + NULL-keyed rows on that basis. A stricter destination breaks + create_internal_tmp_table_from_heap(), which re-inserts every + stored row and treats a duplicate among them as fatal. */ - keydef->flag= (keyinfo->flags & HA_NOSAME) | HA_NULL_ARE_EQUAL; + keydef->flag= (keyinfo->flags & (HA_NOSAME | HA_NULL_ARE_EQUAL)); keydef->keysegs= keyinfo->user_defined_key_parts; keydef->seg= seg; keydef++; @@ -23351,7 +23366,6 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, if (share->keys) { // Get keys for ni_create - bool using_unique_constraint=0; HA_KEYSEG *seg= (HA_KEYSEG*) alloc_root(&table->mem_root, sizeof(*seg) * share->user_defined_key_parts); @@ -23373,11 +23387,11 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, share->keys= 0; share->key_parts= share->ext_key_parts= 0; share->uniques= 1; - using_unique_constraint=1; bzero((char*) &uniquedef,sizeof(uniquedef)); uniquedef.keysegs=keyinfo->user_defined_key_parts; uniquedef.seg=seg; - uniquedef.null_are_equal=1; + /* Propagated, not decided; see create_internal_tmp_table() for Aria. */ + uniquedef.null_are_equal= MY_TEST(keyinfo->flags & HA_NULL_ARE_EQUAL); /* Create extra column for hash value */ bzero((uchar*) *recinfo,sizeof(**recinfo)); @@ -23393,8 +23407,8 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, { /* Create an unique key */ bzero((char*) &keydef,sizeof(keydef)); - keydef.flag= ((keyinfo->flags & HA_NOSAME) | HA_BINARY_PACK_KEY | - HA_PACK_KEY); + keydef.flag= ((keyinfo->flags & (HA_NOSAME | HA_NULL_ARE_EQUAL)) | + HA_BINARY_PACK_KEY | HA_PACK_KEY); keydef.keysegs= keyinfo->user_defined_key_parts; keydef.seg= seg; } @@ -23426,13 +23440,6 @@ bool create_internal_tmp_table(TABLE *table, KEY *org_keyinfo, { seg->null_bit= field->null_bit; seg->null_pos= (uint) (field->null_ptr - (uchar*) table->record[0]); - /* - We are using a GROUP BY on something that contains NULL - In this case we have to tell MyISAM that two NULL should - on INSERT be regarded at the same value - */ - if (!using_unique_constraint) - keydef.flag|= HA_NULL_ARE_EQUAL; } } if (share->keys)