Skip to content

Commit

Permalink
MDEV-32575 MSAN / Valgrind errors in test_if_cheaper_ordering upon re…
Browse files Browse the repository at this point in the history
…aching in_predicate_conversion_threshold

When converting an IN-list to a subquery, a temporary table stores the IN-list
values and participates in join optimization. The problem is the bitmap
of usable keys for the temporary table is initialized after the optimization
phase, during execution. It happens when the table is opened
via `ha_heap::open()`, after the subroutine `set_keys_for_scanning()`
is called. Trying to access the bitmap earlier, during optimization, leads
to MSAN/Valgrind errors.

This fix removes the dependency on `set_keys_for_scanning()`. The key bitmap
is now dynamically composed on demand in `keys_to_use_for_scanning()`,
ensuring correctness without imposing strict call-order constraints.

Reviewer: Oleksandr Byelkin <[email protected]>
  • Loading branch information
Olernov committed Jan 27, 2025
1 parent 3a6af45 commit 95a0325
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 19 deletions.
12 changes: 12 additions & 0 deletions mysql-test/main/opt_tvc.result
Original file line number Diff line number Diff line change
Expand Up @@ -780,3 +780,15 @@ deallocate prepare stmt;
drop table t1;
set in_predicate_conversion_threshold=default;
# End of 10.3 tests
#
# MDEV-32575 MSAN / Valgrind errors in test_if_cheaper_ordering
# upon reaching in_predicate_conversion_threshold
#
create table t1 (a int, b int, c int, primary key (a, b));
insert into t1 (a, b) values (1,1),(1,2),(1,3),(1,4),(1,5),(1,6),(1,7),(1,8);
set in_predicate_conversion_threshold = 2;
select * from t1 where a in (1, 2) and b = 2 order by a, b;
a b c
1 2 NULL
drop table t1;
# End of 11.4 tests
15 changes: 15 additions & 0 deletions mysql-test/main/opt_tvc.test
Original file line number Diff line number Diff line change
Expand Up @@ -474,3 +474,18 @@ drop table t1;
set in_predicate_conversion_threshold=default;

--echo # End of 10.3 tests

--echo #
--echo # MDEV-32575 MSAN / Valgrind errors in test_if_cheaper_ordering
--echo # upon reaching in_predicate_conversion_threshold
--echo #

create table t1 (a int, b int, c int, primary key (a, b));
insert into t1 (a, b) values (1,1),(1,2),(1,3),(1,4),(1,5),(1,6),(1,7),(1,8);

set in_predicate_conversion_threshold = 2;
select * from t1 where a in (1, 2) and b = 2 order by a, b;

drop table t1;

--echo # End of 11.4 tests
31 changes: 14 additions & 17 deletions storage/heap/ha_heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ int ha_heap::open(const char *name, int mode, uint test_if_locked)
}

ref_length= sizeof(HEAP_PTR);
/* Initialize variables for the opened table */
set_keys_for_scanning();
/*
We cannot run update_key_stats() here because we do not have a
lock on the table. The 'records' count might just be changed
Expand Down Expand Up @@ -186,32 +184,34 @@ handler *ha_heap::clone(const char *name, MEM_ROOT *mem_root)


/*
Compute which keys to use for scanning
Return set of keys usable for scanning
SYNOPSIS
set_keys_for_scanning()
no parameter
keys_to_use_for_scanning()
(no parameters)
DESCRIPTION
Set the bitmap btree_keys, which is used when the upper layers ask
which keys to use for scanning. For each btree index the
corresponding bit is set.
This function populates the bitmap `btree_keys`, where each bit represents
a key that can be used for scanning the table. The bitmap is dynamically
updated on every call, ensuring it reflects the current state of the
table's keys. Caching is avoided because the set of usable keys for
MEMORY tables may change during optimization or execution.
RETURN
void
Pointer to the updated bitmap of keys (`btree_keys`)
*/

void ha_heap::set_keys_for_scanning(void)
const key_map *ha_heap::keys_to_use_for_scanning()
{
btree_keys.clear_all();
for (uint i= 0 ; i < table->s->keys ; i++)
{
if (table->key_info[i].algorithm == HA_KEY_ALG_BTREE)
btree_keys.set_bit(i);
}
return &btree_keys;
}


int ha_heap::can_continue_handler_scan()
{
int error= 0;
Expand Down Expand Up @@ -515,8 +515,7 @@ int ha_heap::disable_indexes(key_map map, bool persist)
if (!persist)
{
DBUG_ASSERT(map.is_clear_all());
if (!(error= heap_disable_indexes(file)))
set_keys_for_scanning();
error= heap_disable_indexes(file);
}
else
{
Expand All @@ -534,8 +533,7 @@ int ha_heap::disable_indexes(key_map map, bool persist)
enable_indexes()
DESCRIPTION
Enable indexes and set keys to use for scanning.
The indexes might have been disabled by disable_index() before.
Enable indexes taht might have been disabled by disable_index() before.
The function works only if both data and indexes are empty,
since the heap storage engine cannot repair the indexes.
To be sure, call handler::delete_all_rows() before.
Expand All @@ -555,8 +553,7 @@ int ha_heap::enable_indexes(key_map map, bool persist)
if (!persist)
{
DBUG_ASSERT(map.is_prefix(table->s->keys));
if (!(error= heap_enable_indexes(file)))
set_keys_for_scanning();
error= heap_enable_indexes(file);
}
else
{
Expand Down
3 changes: 1 addition & 2 deletions storage/heap/ha_heap.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class ha_heap final : public handler
HA_READ_NEXT | HA_READ_PREV | HA_READ_ORDER | HA_READ_RANGE :
HA_ONLY_WHOLE_INDEX | HA_KEY_SCAN_NOT_ROR);
}
const key_map *keys_to_use_for_scanning() override { return &btree_keys; }
const key_map *keys_to_use_for_scanning() override;
uint max_supported_keys() const override { return MAX_KEY; }
uint max_supported_key_part_length() const override { return MAX_KEY_LENGTH; }
IO_AND_CPU_COST scan_time() override;
Expand Down Expand Up @@ -121,5 +121,4 @@ class ha_heap final : public handler
int find_unique_row(uchar *record, uint unique_idx) override;
private:
void update_key_stats();
void set_keys_for_scanning(void);
};

0 comments on commit 95a0325

Please sign in to comment.