1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug #45807: crash accessing partitioned table and sql_mode

Browse Source 
contains ONLY_FULL_GROUP_BY

The partitioning code needs to issue a Item::fix_fields()
on the partitioning expression in order to prepare 
it for being evaluated.
It does this by creating a special table and a table list 
for the scope of the partitioning expression.
But when checking ONLY_FULL_GROUP_BY the 
Item_field::fix_fields() was relying that there always be
cached_table set and was trying to use it to get the 
select_lex of the SELECT the field's table is in.
But the cached_table was not set by the partitioning code
that creates the artificial TABLE_LIST used to resolve the
partitioning expression and this resulted in a crash.
 
Fixed by rectifying the following errors :
1. Item_field::fix_fields() : the code that check for 
ONLY_FULL_GROUP_BY relies on having tables with 
cacheable_table set. This is mostly true, the only 
two exceptions being the partitioning context table
and the trigger context table.
Fixed by taking the current parsing context if no pointer
to the TABLE_LIST instance is present in the cached_table.

2. fix_fields_part_func() : 

2a. The code that adds the table being created to the 
scope for the partitioning expression is mostly a copy 
of the add_table_to_list and friends with one exception :
it was not marking the table as cacheable (something that
normal add_table_to_list is doing). This caused the 
problem in the check for ONLY_FULL_GROUP_BY in 
Item_field::fix_fields() to appear.
Fixed by setting the correct members to make the table
cacheable.
The ideal structural fix for this is to use a unified 
interface for adding a table to a table list 
(add_table_to_list?) : noted in a TODO comment

2b. The Item::fix_fields() was called with a NULL destination
pointer. This causes uninitalized memory reads in the 
overloaded ::fix_fields() function (namely 
Item_field::fix_fields()) as it expects a non-zero pointer 
there. Fixed by passing the source pointer similarly to how 
it's done in JOIN::prepare().

mysql-test/r/partition.result:
  Bug #45807: test case
mysql-test/t/partition.test:
  Bug #45807: test case
sql/item.cc:
  Bug #45807: fix the ONLY_FULL_GROUP_BY check code to 
  handle correctly non-cacheable tables.
sql/sql_partition.cc:
  Bug #45807: fix the Item::fix_fields() context
  initializatio for the partitioning expression in 
  CREATE TABLE.
This commit is contained in:
Georgi Kodinov 2009-07-02 17:42:00 +03:00
parent a01b9e2922
commit ae8950f1e8
4 changed files with 39 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
mysql-test/r/partition.result
View File

@ -1982,5 +1982,14 @@ SELECT b, c FROM t1 WHERE b = 1 GROUP BY b, c;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 range bc bc 10 NULL 7 Using where; Using index for group-by
DROP TABLE t1;
#
# Bug #45807: crash accessing partitioned table and sql_mode
# contains ONLY_FULL_GROUP_BY
#
SET SESSION SQL_MODE='ONLY_FULL_GROUP_BY';
CREATE TABLE t1(id INT,KEY(id)) ENGINE=MYISAM
PARTITION BY HASH(id) PARTITIONS 2;
DROP TABLE t1;
SET SESSION SQL_MODE=DEFAULT;
End of 5.1 tests
SET @@global.general_log= @old_general_log;

11
mysql-test/t/partition.test
View File

@ -1976,6 +1976,17 @@ SELECT b, c FROM t1 WHERE b = 1 GROUP BY b, c;
DROP TABLE t1;
--echo #
--echo # Bug #45807: crash accessing partitioned table and sql_mode
--echo # contains ONLY_FULL_GROUP_BY
--echo #
SET SESSION SQL_MODE='ONLY_FULL_GROUP_BY';
CREATE TABLE t1(id INT,KEY(id)) ENGINE=MYISAM
PARTITION BY HASH(id) PARTITIONS 2;
DROP TABLE t1;
SET SESSION SQL_MODE=DEFAULT;
--echo End of 5.1 tests
SET @@global.general_log= @old_general_log;

10
sql/item.cc
View File

@ -4406,16 +4406,22 @@ mark_non_agg_field:
Fields from outer selects added to the aggregate function
outer_fields list as its unknown at the moment whether it's
aggregated or not.
We're using either the select lex of the cached table (if present)
or the field's resolution context. context->select_lex is
safe for use because it's either the SELECT we want to use
(the current level) or a stub added by non-SELECT queries.
*/
SELECT_LEX *select_lex= cached_table ?
cached_table->select_lex : context->select_lex;
if (!thd->lex->in_sum_func)
cached_table->select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
else
{
if (outer_fixed)
thd->lex->in_sum_func->outer_fields.push_back(this);
else if (thd->lex->in_sum_func->nest_level !=
thd->lex->current_select->nest_level)
cached_table->select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
select_lex->full_group_by_flag|= NON_AGG_FIELD_USED;
}
}
return FALSE;

13
sql/sql_partition.cc
View File

@ -918,6 +918,9 @@ bool fix_fields_part_func(THD *thd, Item* func_expr, TABLE *table,
Set-up the TABLE_LIST object to be a list with a single table
Set the object to zero to create NULL pointers and set alias
and real name to table name and get database name from file name.
TODO: Consider generalizing or refactoring Lex::add_table_to_list() so
it can be used in all places where we create TABLE_LIST objects.
Also consider creating appropriate constructors for TABLE_LIST.
*/
bzero((void*)&tables, sizeof(TABLE_LIST));
@ -925,6 +928,13 @@ bool fix_fields_part_func(THD *thd, Item* func_expr, TABLE *table,
tables.table= table;
tables.next_local= 0;
tables.next_name_resolution_table= 0;
/*
Cache the table in Item_fields. All the tables can be cached except
the trigger pseudo table.
*/
tables.cacheable_table= TRUE;
context= thd->lex->current_context();
tables.select_lex= context->select_lex;
strmov(db_name_string, table->s->normalized_path.str);
dir_length= dirname_length(db_name_string);
db_name_string[dir_length - 1]= 0;
@ -932,7 +942,6 @@ bool fix_fields_part_func(THD *thd, Item* func_expr, TABLE *table,
db_name= &db_name_string[home_dir_length];
tables.db= db_name;
context= thd->lex->current_context();
table->map= 1; //To ensure correct calculation of const item
table->get_fields_in_item_tree= TRUE;
save_table_list= context->table_list;
@ -965,7 +974,7 @@ bool fix_fields_part_func(THD *thd, Item* func_expr, TABLE *table,
save_use_only_table_context= thd->lex->use_only_table_context;
thd->lex->use_only_table_context= TRUE;
error= func_expr->fix_fields(thd, (Item**)0);
error= func_expr->fix_fields(thd, (Item**)&func_expr);
thd->lex->use_only_table_context= save_use_only_table_context;