From b5c119e7c0f3b0ff6885c0dc247f53902ef7f621 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 11 Dec 2004 19:59:09 +0300 Subject: [PATCH 1/2] Fix for BUG#6976: In Item_ref::Item_ref set maybe_null (and other fields fix_fields sets) to be the same as in (*ref), because Item_ref::fix_fields() will not be called. Previously maybe_null was 0 always and this produced a bogus state where maybe_null==0 && is_null() == true which broke evaluation for some upper-level Items, like AND and OR. mysql-test/r/group_by.result: Test for BUG#6976 mysql-test/t/group_by.test: Test for BUG#6976 sql/item.cc: Comment added sql/item.h: Fix for BUG#6976: in Item_ref::Item_ref(Item**...) fix all fields because fix_fields() will not be called. --- mysql-test/r/group_by.result | 12 ++++++++++++ mysql-test/t/group_by.test | 9 +++++++++ sql/item.cc | 1 + sql/item.h | 14 +++++++++++++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result index dba95614052..f636692c0d9 100644 --- a/mysql-test/r/group_by.result +++ b/mysql-test/r/group_by.result @@ -626,3 +626,15 @@ explain SELECT i, COUNT(DISTINCT(i)) FROM t1 GROUP BY j ORDER BY NULL; table type possible_keys key key_len ref rows Extra t1 ALL NULL NULL NULL NULL 6 Using filesort DROP TABLE t1; +create table t1 (a int); +insert into t1 values(null); +select min(a) is null from t1; +min(a) is null +1 +select min(a) is null or null from t1; +min(a) is null or null +1 +select 1 and min(a) is null from t1; +1 and min(a) is null +1 +drop table t1; diff --git a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test index 58bb4b3e268..5af78b924f8 100644 --- a/mysql-test/t/group_by.test +++ b/mysql-test/t/group_by.test @@ -447,3 +447,12 @@ INSERT INTO t1 VALUES (1,2),(2,3),(4,5),(3,5),(1,5),(23,5); SELECT i, COUNT(DISTINCT(i)) FROM t1 GROUP BY j ORDER BY NULL; explain SELECT i, COUNT(DISTINCT(i)) FROM t1 GROUP BY j ORDER BY NULL; DROP TABLE t1; + +#Test for BUG#6976: Aggregate functions have incorrect NULL-ness +create table t1 (a int); +insert into t1 values(null); +select min(a) is null from t1; +select min(a) is null or null from t1; +select 1 and min(a) is null from t1; +drop table t1; + diff --git a/sql/item.cc b/sql/item.cc index 739b5385b55..8737cc06bbd 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -687,6 +687,7 @@ bool Item_null::send(THD *thd, String *packet) /* This is used for HAVING clause Find field in select list having the same name + This is not always called, see also Item_ref::Item_ref */ bool Item_ref::fix_fields(THD *thd,TABLE_LIST *tables) diff --git a/sql/item.h b/sql/item.h index f6f9e1c0621..cc6a846d6c1 100644 --- a/sql/item.h +++ b/sql/item.h @@ -401,7 +401,19 @@ public: Item_ref(char *db_par,char *table_name_par,char *field_name_par) :Item_ident(db_par,table_name_par,field_name_par),ref(0) {} Item_ref(Item **item, char *table_name_par,char *field_name_par) - :Item_ident(NullS,table_name_par,field_name_par),ref(item) {} + :Item_ident(NullS,table_name_par,field_name_par),ref(item) + { + /* + This ctor is called from Item_XXX::split_sum_func, and fix_fields will + not be called for *this, so we must setup everything here. **ref is + already fixed at this point. + */ + max_length= (*ref)->max_length; + decimals= (*ref)->decimals; + binary= (*ref)->binary; + with_sum_func= (*ref)->with_sum_func; + maybe_null= (*ref)->maybe_null; + } enum Type type() const { return REF_ITEM; } bool eq(const Item *item, bool binary_cmp) const { return (*ref)->eq(item, binary_cmp); } From 6d7fe8520a938d92a6a7b0e569e8f56d926936ac Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 13 Dec 2004 20:06:06 +0300 Subject: [PATCH 2/2] Merging fix for BUG#6976 from 4.0 to 4.1 The problem in 4.1 was the same as in 4.0 - fix_fields() not called for created Item_ref. The fix is similar too - initialize Item_refs in ctor (but don't interfere with cases when Item_ref is used by subselects). sql/item.cc: Fix for BUG#6976 ported from 4.0 sql/item_cmpfunc.cc: Fix for BUG#6976 ported from 4.0 sql/item_func.cc: Fix for BUG#6976 ported from 4.0 sql/item_row.cc: Fix for BUG#6976 ported from 4.0 sql/item_strfunc.cc: Fix for BUG#6976 ported from 4.0 --- sql/item.cc | 4 ++-- sql/item.h | 21 +++++++++++++++++++++ sql/item_cmpfunc.cc | 3 ++- sql/item_func.cc | 2 +- sql/item_row.cc | 3 ++- sql/item_strfunc.cc | 2 +- 6 files changed, 29 insertions(+), 6 deletions(-) diff --git a/sql/item.cc b/sql/item.cc index 31c35e87cd4..85e200920f1 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1488,9 +1488,9 @@ bool Item_field::fix_fields(THD *thd, TABLE_LIST *tables, Item **ref) "forward reference in item list"); return -1; } - Item_ref *rf= new Item_ref(last->ref_pointer_array + counter, - (char *)table_name, (char *)field_name); + (char *)table_name, (char *)field_name, + this); if (!rf) return 1; thd->change_item_tree(ref, rf); diff --git a/sql/item.h b/sql/item.h index 3c4f80e3857..cf697f59727 100644 --- a/sql/item.h +++ b/sql/item.h @@ -835,6 +835,26 @@ public: :Item_ident(db_par, table_name_par, field_name_par), ref(0) {} Item_ref(Item **item, const char *table_name_par, const char *field_name_par) :Item_ident(NullS, table_name_par, field_name_par), ref(item) {} + + /* + This constructor is used when processing GROUP BY and referred Item is + available. We set all properties here because fix_fields() will not be + called for the created Item_ref. (see BUG#6976) + TODO check if we could get rid of *_name_par parameters and if we need to + perform similar initialization for other ctors. + TODO we probably fix a superset of problems like in BUG#6658. Check this + with Bar, and if we have a more broader set of problems like this. + */ + Item_ref(Item **item, const char *table_name_par, + const char *field_name_par, Item *src) + : Item_ident(NullS, table_name_par, field_name_par), ref(item) + { + collation.set(src->collation); + max_length= src->max_length; + decimals= src->decimals; + with_sum_func= src->with_sum_func; + maybe_null= src->maybe_null; + } /* Constructor need to process subselect with temporary tables (see Item) */ Item_ref(THD *thd, Item_ref *item) :Item_ident(thd, item), ref(item->ref) {} enum Type type() const { return REF_ITEM; } @@ -890,6 +910,7 @@ public: }; class Item_in_subselect; + class Item_ref_null_helper: public Item_ref { protected: diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 51212418b09..b225889d916 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2022,7 +2022,8 @@ void Item_cond::split_sum_func(THD *thd, Item **ref_pointer_array, { Item **ref= li.ref(); uint el= fields.elements; - Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name); + Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name, + item); fields.push_front(item); ref_pointer_array[el]= item; thd->change_item_tree(ref, new_item); diff --git a/sql/item_func.cc b/sql/item_func.cc index 98b204d1809..af53a771720 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -349,7 +349,7 @@ void Item_func::split_sum_func(THD *thd, Item **ref_pointer_array, else if (item->used_tables() || item->type() == SUM_FUNC_ITEM) { uint el= fields.elements; - Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name); + Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name, item); new_item->collation.set(item->collation); fields.push_front(item); ref_pointer_array[el]= item; diff --git a/sql/item_row.cc b/sql/item_row.cc index 289efe45300..b65b0b7b608 100644 --- a/sql/item_row.cc +++ b/sql/item_row.cc @@ -95,7 +95,8 @@ void Item_row::split_sum_func(THD *thd, Item **ref_pointer_array, else if ((*arg)->used_tables() || (*arg)->type() == SUM_FUNC_ITEM) { uint el= fields.elements; - Item *new_item= new Item_ref(ref_pointer_array + el, 0, (*arg)->name); + Item *new_item= new Item_ref(ref_pointer_array + el, 0, (*arg)->name, + *arg); fields.push_front(*arg); ref_pointer_array[el]= *arg; thd->change_item_tree(arg, new_item); diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index aaeeb9d8bb8..82cbd1fed72 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -1748,7 +1748,7 @@ void Item_func_make_set::split_sum_func(THD *thd, Item **ref_pointer_array, else if (item->used_tables() || item->type() == SUM_FUNC_ITEM) { uint el= fields.elements; - Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name); + Item *new_item= new Item_ref(ref_pointer_array + el, 0, item->name,item); fields.push_front(item); ref_pointer_array[el]= item; thd->change_item_tree(&item, new_item);