Hi,
On Wed, Nov 12, 2008 at 05:19:35PM +0200, Timour Katchaounov wrote:
> #At file:///home/tkatchaounov/mysql/bzr/6.0-b36752/
>
> 2700 Timour Katchaounov 2008-11-12
> BUG#36752 subquery materialization produces wrong results when comparing
> different types
>
One small comment, please see below:
> === modified file 'sql/item_subselect.cc'
> --- a/sql/item_subselect.cc 2008-10-20 09:16:47 +0000
> +++ b/sql/item_subselect.cc 2008-11-12 15:19:22 +0000
> @@ -3110,11 +3110,54 @@ bool subselect_hash_sj_engine::init_perm
> KEY_PART_INFO *cur_key_part= tmp_key->key_part;
> store_key **ref_key= tab->ref.key_copy;
> uchar *cur_ref_buff= tab->ref.key_buff;
> +
> + /*
> + Create an artificial condition to post-filter those rows matched by index
> + lookups that cannot be distinguished by the index lookup procedure, e.g.
> + because of truncation. Prepared statements execution requires that
> + fix_fields is called for every execution. In order to call fix_fields we
> + need to create a Name_resolution_context and a corresponding TABLE_LIST
> + for the temporary table for the subquery, so that all column references
> + to the materialized subquery table can be resolved correctly.
> + */
> + DBUG_ASSERT(cond == NULL);
> + if (!(cond= new Item_cond_and))
> + DBUG_RETURN(TRUE);
> + /*
> + Table reference for tmp_table that is used to resolve column references
> + (Item_fields) to columns in tmp_table.
> + */
> + TABLE_LIST *tmp_table_ref;
> + if (!(tmp_table_ref= (TABLE_LIST*) thd->calloc(sizeof(TABLE_LIST))))
> + DBUG_RETURN(TRUE);
> +
> + tmp_table_ref->init_one_table(NULL, 0, "materialized subselect", 22,
> + "materialized subselect", TL_READ);
> + tmp_table_ref->table= tmp_table;
> +
> + /* Name resolution context for all tmp_table columns created below. */
> + Name_resolution_context *context= new Name_resolution_context;
> + context->init();
> + context->first_name_resolution_table=
> + context->last_name_resolution_table= tmp_table_ref;
>
> for (uint i= 0; i < tmp_key_parts; i++, cur_key_part++, ref_key++)
> {
> - tab->ref.items[i]= item_in->left_expr->element_index(i);
> + Item_func_eq *eq_cond; /* New equi-join condition for the current column. */
> + /* Item for the corresponding field from the materialized temp table. */
> + Item_field *right_col_item;
> int null_count= test(cur_key_part->field->real_maybe_null());
> + tab->ref.items[i]= item_in->left_expr->element_index(i);
> +
> + if (!(right_col_item= new Item_field(thd, context, cur_key_part->field)) ||
> + !(eq_cond= new Item_func_eq(tab->ref.items[i], right_col_item)) ||
> + ((Item_cond_and*)cond)->add(eq_cond))
> + {
> + delete cond;
> + cond= NULL;
> + DBUG_RETURN(TRUE);
> + }
> +
> *ref_key= new store_key_item(thd, cur_key_part->field,
> /* TODO:
> the NULL byte is taken into account in
> @@ -3131,6 +3174,9 @@ bool subselect_hash_sj_engine::init_perm
> tab->ref.key_err= 1;
> tab->ref.key_parts= tmp_key_parts;
>
> + if (cond->fix_fields(thd, 0))
> + DBUG_RETURN(TRUE);
> +
Please either
- add a note in Item_cond::fix_field() that it must not try substituting
itself for another item, or
- make the code conform to fix_fields()'s calling convention
> DBUG_RETURN(FALSE);
> }
>
> @@ -3150,6 +3196,12 @@ bool subselect_hash_sj_engine::init_runt
> the subquery if not yet created.
> */
> materialize_engine->prepare();
> + /*
> + Repeat name resolution for 'cond' since cond is not part of any
> + clause of the query, and it is not 'fixed' during JOIN::prepare.
> + */
> + if (cond && !cond->fixed && cond->fix_fields(thd, 0))
> + return TRUE;
Same here.
> /* Let our engine reuse this query plan for materialization. */
> materialize_join= materialize_engine->join;
> materialize_join->change_result(result);
Ok to push after the above is addressed.
BR
Sergey
--
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog
