summaryrefslogtreecommitdiff
path: root/sql/item_sum.h
diff options
context:
space:
mode:
authorGuilhem Bichot <guilhem@mysql.com>2010-12-07 16:59:32 +0100
committerGuilhem Bichot <guilhem@mysql.com>2010-12-07 16:59:32 +0100
commit39b0af1e8c4c97f0f9e67e2a6334e177bcfd9421 (patch)
treee698fffe32383d4435cdb9cda69f36ac8dfdd49a /sql/item_sum.h
parent209df6a69fdebcaede67b8060eaa67fe639dc4ab (diff)
downloadmariadb-git-39b0af1e8c4c97f0f9e67e2a6334e177bcfd9421.tar.gz
Fix for Bug#57932 "query with avg returns incorrect results":
when there was one NULL value, AVG(DISTINCT) could forget about other values. See commit comment of item_sum.cc. mysql-test/r/func_group.result: before the code fix, both SELECTs would return NULL sql/item_sum.cc: Assume we are executing "SELECT AVG([DISTINCT] some_field) FROM some_table". and some_field is the single field of some_table for simplicity. Each time a row is processed (evaluate_join_record()-> end_send_group()->update_sum_func()) an aggregator is notified, which itself notifies an Item_sum_avg. Without DISTINCT, this Item_sum_avg immediately increments its internal "sum of values" and "count of values" (the latter being Item_sum_avg::count). The count is incremented only if the row's value is not NULL (in Item_sum_avg::add()), per AVG() semantices. This row's value is available in args[0] of Item_sum_avg ("args[0]" stands for "the first argument of the item": it's an Item_field which automatically receives the row's value when a row is read from the table). bool Item_sum_avg::add() { if (Item_sum_sum::add()) << calculates the sum (ignores NULL) return TRUE; if (!args[0]->null_value)<<if added value is not NULL count++; <<increment "count" return FALSE; } and everything works. With DISTINCT, when a row is processed by evaluate_join_record(), Item_sum_avg does no immediate computation, rather stores the row's value in a tree (to throw the value away if it is a duplicate of previous value, otherwise to remember all distinct values). It's only when it's time to send the average to the user (at end of the query: sub_select(end_of_records=true)->end_send_group()-> select_send->send_data()->Protocol::send_result_set_row()-> Item::send()->Item_sum_avg->val_str()), that we iterate over the tree, compute the sum and count: for this, for each element of the tree, Item_sum_avg::add() is called and has the same two steps as before: * Item_sum_sum::add() updates the sum (finding the tree element's value correctly, and determining correctly its NULLness - look for "arg_is_null" in that function) * the "if (!args[0]->null_value)" test right after, breaks: it uses args[0], which isn't the tree's element but rather the value for the last row processed by evaluate_join_record(). So if that last row was NULL, "count" stays 0 for each row, and AVG() then returns NULL (count==0 => NULL, per AVG() semantics). The fix is to let the aggregator tell whether the value it just saw was NULL. The aggregator knows where to get the info thanks to virtual functions. Item_sum_sum::add() now asks the aggregator. Item_sum_avg() also asks the aggregator and then knows it shouldn't increment "count". sql/item_sum.h: Aggregator can now tell about value/NULLness of just-aggregated value
Diffstat (limited to 'sql/item_sum.h')
-rw-r--r--sql/item_sum.h16
1 files changed, 16 insertions, 0 deletions
diff --git a/sql/item_sum.h b/sql/item_sum.h
index 06a8c2c58a4..c7159568c40 100644
--- a/sql/item_sum.h
+++ b/sql/item_sum.h
@@ -101,6 +101,15 @@ public:
*/
virtual void endup() = 0;
+ /** Decimal value of being-aggregated argument */
+ virtual my_decimal *arg_val_decimal(my_decimal * value) = 0;
+ /** Floating point value of being-aggregated argument */
+ virtual double arg_val_real() = 0;
+ /**
+ NULLness of being-aggregated argument; can be called only after
+ arg_val_decimal() or arg_val_real().
+ */
+ virtual bool arg_is_null() = 0;
};
@@ -304,6 +313,7 @@ class st_select_lex;
class Item_sum :public Item_result_field
{
friend class Aggregator_distinct;
+ friend class Aggregator_simple;
protected:
/**
@@ -600,6 +610,9 @@ public:
void clear();
bool add();
void endup();
+ virtual my_decimal *arg_val_decimal(my_decimal * value);
+ virtual double arg_val_real();
+ virtual bool arg_is_null();
bool unique_walk_function(void *element);
static int composite_key_cmp(void* arg, uchar* key1, uchar* key2);
@@ -623,6 +636,9 @@ public:
void clear() { item_sum->clear(); }
bool add() { return item_sum->add(); }
void endup() {};
+ virtual my_decimal *arg_val_decimal(my_decimal * value);
+ virtual double arg_val_real();
+ virtual bool arg_is_null();
};