From 21809f9a450df1bc44cef36377f96b516ac4a9ae Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Tue, 29 Dec 2020 13:38:16 +1000 Subject: MDEV-17556 Assertion `bitmap_is_set_all(&table->s->all_set)' failed The assertion failed in handler::ha_reset upon SELECT under READ UNCOMMITTED from table with index on virtual column. This was the debug-only failure, though the problem is mush wider: * MY_BITMAP is a structure containing my_bitmap_map, the latter is a raw bitmap. * read_set, write_set and vcol_set of TABLE are the pointers to MY_BITMAP * The rest of MY_BITMAPs are stored in TABLE and TABLE_SHARE * The pointers to the stored MY_BITMAPs, like orig_read_set etc, and sometimes all_set and tmp_set, are assigned to the pointers. * Sometimes tmp_use_all_columns is used to substitute the raw bitmap directly with all_set.bitmap * Sometimes even bitmaps are directly modified, like in TABLE::update_virtual_field(): bitmap_clear_all(&tmp_set) is called. The last three bullets in the list, when used together (which is mostly always) make the program flow cumbersome and impossible to follow, notwithstanding the errors they cause, like this MDEV-17556, where tmp_set pointer was assigned to read_set, write_set and vcol_set, then its bitmap was substituted with all_set.bitmap by dbug_tmp_use_all_columns() call, and then bitmap_clear_all(&tmp_set) was applied to all this. To untangle this knot, the rule should be applied: * Never substitute bitmaps! This patch is about this. orig_*, all_set bitmaps are never substituted already. This patch changes the following function prototypes: * tmp_use_all_columns, dbug_tmp_use_all_columns to accept MY_BITMAP** and to return MY_BITMAP * instead of my_bitmap_map* * tmp_restore_column_map, dbug_tmp_restore_column_maps to accept MY_BITMAP* instead of my_bitmap_map* These functions now will substitute read_set/write_set/vcol_set directly, and won't touch underlying bitmaps. --- sql/opt_range.cc | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'sql/opt_range.cc') diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 2204500e3b5..30c74799b6f 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -3264,8 +3264,7 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond) void store_key_image_to_rec(Field *field, uchar *ptr, uint len) { - /* Do the same as print_key() does */ - my_bitmap_map *old_map; + /* Do the same as print_key() does */ if (field->real_maybe_null()) { @@ -3277,10 +3276,10 @@ void store_key_image_to_rec(Field *field, uchar *ptr, uint len) field->set_notnull(); ptr++; } - old_map= dbug_tmp_use_all_columns(field->table, - field->table->write_set); + MY_BITMAP *old_map= dbug_tmp_use_all_columns(field->table, + &field->table->write_set); field->set_key_image(ptr, len); - dbug_tmp_restore_column_map(field->table->write_set, old_map); + dbug_tmp_restore_column_map(&field->table->write_set, old_map); } #ifdef WITH_PARTITION_STORAGE_ENGINE @@ -3495,7 +3494,7 @@ bool prune_partitions(THD *thd, TABLE *table, Item *pprune_cond) PART_PRUNE_PARAM prune_param; MEM_ROOT alloc; RANGE_OPT_PARAM *range_par= &prune_param.range_param; - my_bitmap_map *old_sets[2]; + MY_BITMAP *old_sets[2]; prune_param.part_info= part_info; init_sql_alloc(&alloc, "prune_partitions", @@ -3512,7 +3511,7 @@ bool prune_partitions(THD *thd, TABLE *table, Item *pprune_cond) } dbug_tmp_use_all_columns(table, old_sets, - table->read_set, table->write_set); + &table->read_set, &table->write_set); range_par->thd= thd; range_par->table= table; /* range_par->cond doesn't need initialization */ @@ -3609,7 +3608,7 @@ all_used: retval= FALSE; // some partitions are used mark_all_partitions_as_used(prune_param.part_info); end: - dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); + dbug_tmp_restore_column_maps(&table->read_set, &table->write_set, old_sets); thd->no_errors=0; thd->mem_root= range_par->old_root; free_root(&alloc,MYF(0)); // Return memory & allocator @@ -14852,8 +14851,8 @@ static void print_sel_arg_key(Field *field, const uchar *key, String *out) { TABLE *table= field->table; - my_bitmap_map *old_sets[2]; - dbug_tmp_use_all_columns(table, old_sets, table->read_set, table->write_set); + MY_BITMAP *old_sets[2]; + dbug_tmp_use_all_columns(table, old_sets, &table->read_set, &table->write_set); if (field->real_maybe_null()) { @@ -14873,7 +14872,7 @@ print_sel_arg_key(Field *field, const uchar *key, String *out) field->val_str(out); end: - dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); + dbug_tmp_restore_column_maps(&table->read_set, &table->write_set, old_sets); } @@ -14968,9 +14967,9 @@ print_key(KEY_PART *key_part, const uchar *key, uint used_length) const uchar *key_end= key+used_length; uint store_length; TABLE *table= key_part->field->table; - my_bitmap_map *old_sets[2]; + MY_BITMAP *old_sets[2]; - dbug_tmp_use_all_columns(table, old_sets, table->read_set, table->write_set); + dbug_tmp_use_all_columns(table, old_sets, &table->read_set, &table->write_set); for (; key < key_end; key+=store_length, key_part++) { @@ -14997,7 +14996,7 @@ print_key(KEY_PART *key_part, const uchar *key, uint used_length) if (key+store_length < key_end) fputc('/',DBUG_FILE); } - dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); + dbug_tmp_restore_column_maps(&table->read_set, &table->write_set, old_sets); } @@ -15005,16 +15004,16 @@ static void print_quick(QUICK_SELECT_I *quick, const key_map *needed_reg) { char buf[MAX_KEY/8+1]; TABLE *table; - my_bitmap_map *old_sets[2]; + MY_BITMAP *old_sets[2]; DBUG_ENTER("print_quick"); if (!quick) DBUG_VOID_RETURN; DBUG_LOCK_FILE; table= quick->head; - dbug_tmp_use_all_columns(table, old_sets, table->read_set, table->write_set); + dbug_tmp_use_all_columns(table, old_sets, &table->read_set, &table->write_set); quick->dbug_dump(0, TRUE); - dbug_tmp_restore_column_maps(table->read_set, table->write_set, old_sets); + dbug_tmp_restore_column_maps(&table->read_set, &table->write_set, old_sets); fprintf(DBUG_FILE,"other_keys: 0x%s:\n", needed_reg->print(buf)); -- cgit v1.2.1 From 33ede50f207552df835d7606f990fa9ccc4e0d12 Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Thu, 28 Jan 2021 20:46:13 +0300 Subject: MDEV-22251: get_key_scans_params: Conditional jump or move depends on uninitialised value Apply the patch based on the patch by Varun Gupta: PARAM::is_ror_scan might be used unitialized when check_quick_select() is invoked for a "degenerate" SEL_ARG tree (e.g. one having type SEL_ARG::IMPOSSIBLE). Make check_quick_select() always initialize PARAM::is_ror_scan. --- sql/opt_range.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'sql/opt_range.cc') diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 7785c768fbc..f3f184367c9 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -10385,6 +10385,7 @@ ha_rows check_quick_select(PARAM *param, uint idx, bool index_only, uint keynr= param->real_keynr[idx]; DBUG_ENTER("check_quick_select"); + param->is_ror_scan= FALSE; /* Handle cases when we don't have a valid non-empty list of range */ if (!tree) DBUG_RETURN(HA_POS_ERROR); -- cgit v1.2.1