Merge pull request #519 from drrtuy/MCOL-1510

MCOL-1510 GROUP BY handler doesn't support operations on aggregate functions when projecting columns.

dbcon/mysql/ha_calpont.cpp

@@ -1170,6 +1170,46 @@ create_calpont_group_by_handler(THD* thd, Query* query)
     return handler;
 }
 
+/***********************************************************
+ * DESCRIPTION:
+ * GROUP BY handler constructor
+ * PARAMETERS:
+ *    thd - THD pointer.
+ *    query - Query describing structure
+ ***********************************************************/
+ha_calpont_group_by_handler::ha_calpont_group_by_handler(THD* thd_arg, Query* query)
+        : group_by_handler(thd_arg, calpont_hton),
+          select(query->select),
+          table_list(query->from),
+          distinct(query->distinct),
+          where(query->where),
+          group_by(query->group_by),
+          order_by(query->order_by),
+          having(query->having)
+{
+    List_iterator_fast<Item> item_iter(*select);
+    Item* item;
+    char* str = NULL;
+    while((item = item_iter++))
+    {
+        String descr;
+        item->print(&descr, QT_ORDINARY);
+        str = new char[descr.length()+1];
+        strncpy(str, descr.ptr(), descr.length());
+        str[descr.length()] = '\0';
+        select_list_descr.push_back(str);
+    }
+}
+
+/***********************************************************
+ * DESCRIPTION:
+ * GROUP BY destructor
+ ***********************************************************/
+ha_calpont_group_by_handler::~ha_calpont_group_by_handler()
+{
+    select_list_descr.delete_elements();
+}
+
 /***********************************************************
  * DESCRIPTION:
  * Makes the plan and prepares the data

dbcon/mysql/ha_calpont.h

@@ -255,12 +255,16 @@ public:
  * One should read comments in server/sql/group_by_handler.h
  * Attributes:
  * select - attribute contains all GROUP BY, HAVING, ORDER items and calls it
- * an extended SELECT list accordin to comments in
+ *              an extended SELECT list according to comments in
- * server/sql/group_handler.cc.
+ *              server/sql/group_handler.cc.
- * So the temporary table for
+ *              So the temporary table for
- * select count(*) from b group by a having a > 3 order by a
+ *              select count(*) from b group by a having a > 3 order by a
- * will have 4 columns not 1.
+ *              will have 4 columns not 1.
- * However server ignores all NULLs used in GROUP BY, HAVING, ORDER.
+ *              However server ignores all NULLs used in
+ *              GROUP BY, HAVING, ORDER.
+ * select_list_descr - contains Item description returned by Item->print()
+ *              that is used in lookup for corresponding columns in
+ *              extended SELECT list.
  * table_list - contains all tables involved. Must be CS tables only.
  * distinct - looks like a useless thing for now. Couldn't get it set by server.
  * where - where items.
@@ -275,22 +279,14 @@ public:
 class ha_calpont_group_by_handler: public group_by_handler
 {
 public:
-    ha_calpont_group_by_handler(THD* thd_arg, Query* query)
+    ha_calpont_group_by_handler(THD* thd_arg, Query* query);
-        : group_by_handler(thd_arg, calpont_hton),
+    ~ha_calpont_group_by_handler();
-          select(query->select),
-          table_list(query->from),
-          distinct(query->distinct),
-          where(query->where),
-          group_by(query->group_by),
-          order_by(query->order_by),
-          having(query->having)
-    { }
-    ~ha_calpont_group_by_handler() { }
     int init_scan();
     int next_row();
     int end_scan();
 
     List<Item>* select;
+    List<char> select_list_descr;
     TABLE_LIST* table_list;
     bool        distinct;
     Item*       where;

dbcon/mysql/ha_calpont_execplan.cpp

@@ -42,6 +42,9 @@
 #include <vector>
 #include <map>
 #include <limits>
+
+#include <string.h>
+
 using namespace std;
 
 #include <boost/shared_ptr.hpp>
@@ -188,6 +191,27 @@ bool nonConstFunc(Item_func* ifp)
     return false;
 }
 
+ReturnedColumn* findCorrespTempField(Item_ref* item, gp_walk_info& gwi, bool clone = true)
+{
+    ReturnedColumn* result = NULL;
+    uint32_t i;
+    for (i = 0; i < gwi.returnedCols.size(); i++)
+    {
+        if (item->ref[0] && item->ref[0]->name &&
+        gwi.returnedCols[i]->alias().c_str() &&
+        !strcasecmp(item->ref[0]->name, gwi.returnedCols[i]->alias().c_str()))
+        {
+            if (clone)
+                result = gwi.returnedCols[i]->clone();
+            else
+                result = gwi.returnedCols[i].get();
+            break;
+        }
+    }
+
+    return result;
+}
+
 string getViewName(TABLE_LIST* table_ptr)
 {
     string viewName = "";
@@ -2739,7 +2763,7 @@ CalpontSystemCatalog::ColType colType_MysqlToIDB (const Item* item)
     return ct;
 }
 
-ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport)
+ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport, bool pushdownHand)
 {
     ReturnedColumn* rc = NULL;
 
@@ -2864,9 +2888,9 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp
             }
 
             if (func_name == "+" || func_name == "-" || func_name == "*" || func_name == "/" )
-                return buildArithmeticColumn(ifp, gwi, nonSupport);
+                return buildArithmeticColumn(ifp, gwi, nonSupport, pushdownHand);
             else
-                return buildFunctionColumn(ifp, gwi, nonSupport);
+                return buildFunctionColumn(ifp, gwi, nonSupport, pushdownHand);
         }
 
         case Item::SUM_FUNC_ITEM:
@@ -2998,7 +3022,11 @@ ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupp
     return rc;
 }
 
-ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool& nonSupport)
+ArithmeticColumn* buildArithmeticColumn(
+    Item_func* item,
+    gp_walk_info& gwi,
+    bool& nonSupport,
+    bool pushdownHand)
 {
     if (!(gwi.thd->infinidb_vtable.cal_conn_info))
         gwi.thd->infinidb_vtable.cal_conn_info = (void*)(new cal_connection_info());
@@ -3021,7 +3049,7 @@ ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool
     {
         if (gwi.clauseType == SELECT || /*gwi.clauseType == HAVING || */gwi.clauseType == GROUP_BY || gwi.clauseType == FROM) // select list
         {
-            lhs = new ParseTree(buildReturnedColumn(sfitempp[0], gwi, nonSupport));
+            lhs = new ParseTree(buildReturnedColumn(sfitempp[0], gwi, nonSupport, pushdownHand));
 
             if (!lhs->data() && (sfitempp[0]->type() == Item::FUNC_ITEM))
             {
@@ -3029,8 +3057,15 @@ ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool
                 Item_func* ifp = (Item_func*)sfitempp[0];
                 lhs = buildParseTree(ifp, gwi, nonSupport);
             }
-
+            else if(pushdownHand && !lhs->data() && (sfitempp[0]->type() == Item::REF_ITEM))
-            rhs = new ParseTree(buildReturnedColumn(sfitempp[1], gwi, nonSupport));
+            {
+                // There must be an aggregation column in extended SELECT
+                // list so find the corresponding column.
+                ReturnedColumn* rc = findCorrespTempField(static_cast<Item_ref*>(sfitempp[0]), gwi);
+                if(rc)
+                    lhs = new ParseTree(rc);
+            }
+            rhs = new ParseTree(buildReturnedColumn(sfitempp[1], gwi, nonSupport, pushdownHand));
 
             if (!rhs->data() && (sfitempp[1]->type() == Item::FUNC_ITEM))
             {
@@ -3038,6 +3073,14 @@ ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool
                 Item_func* ifp = (Item_func*)sfitempp[1];
                 rhs = buildParseTree(ifp, gwi, nonSupport);
             }
+            else if(pushdownHand && !rhs->data() && (sfitempp[1]->type() == Item::REF_ITEM))
+            {
+                // There must be an aggregation column in extended SELECT
+                // list so find the corresponding column.
+                ReturnedColumn* rc = findCorrespTempField(static_cast<Item_ref*>(sfitempp[1]), gwi);
+                if(rc)
+                    rhs = new ParseTree(rc);
+            }
         }
         else // where clause
         {
@@ -3198,7 +3241,11 @@ ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool
     return ac;
 }
 
-ReturnedColumn* buildFunctionColumn(Item_func* ifp, gp_walk_info& gwi, bool& nonSupport)
+ReturnedColumn* buildFunctionColumn(
+    Item_func* ifp,
+    gp_walk_info& gwi,
+    bool& nonSupport,
+    bool pushdownHand)
 {
     if (!(gwi.thd->infinidb_vtable.cal_conn_info))
         gwi.thd->infinidb_vtable.cal_conn_info = (void*)(new cal_connection_info());
@@ -3239,7 +3286,7 @@ ReturnedColumn* buildFunctionColumn(Item_func* ifp, gp_walk_info& gwi, bool& non
     // Arithmetic exp
     if (funcName == "+" || funcName == "-" || funcName == "*" || funcName == "/" )
     {
-        ArithmeticColumn* ac = buildArithmeticColumn(ifp, gwi, nonSupport);
+        ArithmeticColumn* ac = buildArithmeticColumn(ifp, gwi, nonSupport, pushdownHand);
         return ac;
     }
 
@@ -3366,7 +3413,14 @@ ReturnedColumn* buildFunctionColumn(Item_func* ifp, gp_walk_info& gwi, bool& non
                     return NULL;
                 }
 
-                ReturnedColumn* rc = buildReturnedColumn(ifp->arguments()[i], gwi, nonSupport);
+                ReturnedColumn* rc = buildReturnedColumn(ifp->arguments()[i], gwi, nonSupport, pushdownHand);
+
+                // MCOL-1510 It must be a temp table field, so find the corresponding column.
+                if (pushdownHand 
+                    && ifp->arguments()[i]->type() == Item::REF_ITEM)
+                {
+                    rc = findCorrespTempField(static_cast<Item_ref*>(ifp->arguments()[i]), gwi);
+                }
 
                 if (!rc || nonSupport)
                 {
@@ -5432,7 +5486,10 @@ void gp_walk(const Item* item, void* arg)
  *  the involved item_fields to the passed in vector. It's used in parsing
  *  functions or arithmetic expressions for vtable post process.
  */
-void parse_item (Item* item, vector<Item_field*>& field_vec, bool& hasNonSupportItem, uint16_t& parseInfo)
+void parse_item (Item* item, vector<Item_field*>& field_vec,
+    bool& hasNonSupportItem, 
+    uint16_t& parseInfo, 
+    gp_walk_info* gwi)
 {
     Item::Type itype = item->type();
 
@@ -5470,7 +5527,7 @@ void parse_item (Item* item, vector<Item_field*>& field_vec, bool& hasNonSupport
             }
 
             for (uint32_t i = 0; i < isp->argument_count(); i++)
-                parse_item(isp->arguments()[i], field_vec, hasNonSupportItem, parseInfo);
+                parse_item(isp->arguments()[i], field_vec, hasNonSupportItem, parseInfo, gwi);
 
 //				parse_item(sfitempp[i], field_vec, hasNonSupportItem, parseInfo);
             break;
@@ -5515,8 +5572,20 @@ void parse_item (Item* item, vector<Item_field*>& field_vec, bool& hasNonSupport
                 }
                 else if ((*(ref->ref))->type() == Item::FIELD_ITEM)
                 {
-                    Item_field* ifp = reinterpret_cast<Item_field*>(*(ref->ref));
+                    // MCOL-1510. This could be a non-supported function
-                    field_vec.push_back(ifp);
+                    // argument in form of a temp_table_field, so check
+                    // and set hasNonSupportItem if it is so.
+                    ReturnedColumn* rc = NULL;
+                    if (gwi)
+                        rc = findCorrespTempField(ref, *gwi, false);
+                    
+                    if (!rc)
+                    {
+                        Item_field* ifp = reinterpret_cast<Item_field*>(*(ref->ref));
+                        field_vec.push_back(ifp);
+                    }
+                    else
+                        hasNonSupportItem = true;
                     break;
                 }
                 else if ((*(ref->ref))->type() == Item::FUNC_ITEM)
@@ -8116,6 +8185,7 @@ int cp_get_group_plan(THD* thd, SCSEP& csep, cal_impl_if::cal_group_info& gi)
     SELECT_LEX select_lex = lex->select_lex;
     gp_walk_info gwi;
     gwi.thd = thd;
+    gwi.groupByAuxDescr = gi.groupByAuxDescr;
     int status = getGroupPlan(gwi, select_lex, csep, gi);
 
     cerr << "---------------- cp_get_group_plan EXECUTION PLAN ----------------" << endl;
@@ -8497,6 +8567,8 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
     string sel_cols_in_create;
     string sel_cols_in_select;
     bool redo = false;
+    List_iterator_fast<char> itDescr(*gi.groupByAuxDescr);
+    char* fieldDescr;
 
     // empty rcWorkStack and ptWorkStack. They should all be empty by now.
     clearStacks(gwi);
@@ -8515,7 +8587,15 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
 
     while ((item = it++))
     {
-        string itemAlias = (item->name ? item->name : "<NULL>");
+        // Given the size of gi.groupByAuxDescr is equal to gi.groupByFields
+        fieldDescr = itDescr++;
+        string itemAlias;
+        if(item->name)
+            itemAlias = (item->name);
+        else
+        {
+            itemAlias = (fieldDescr ? fieldDescr: "<NULL>");
+        }
 
         // @bug 5916. Need to keep checking until getting concret item in case
         // of nested view.
@@ -8621,6 +8701,8 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
                     return ER_CHECK_NOT_IMPLEMENTED;
                 }
 
+                if(!ac->alias().length())
+                    ac->alias(fieldDescr);
                 // add this agg col to returnedColumnList
                 boost::shared_ptr<ReturnedColumn> spac(ac);
                 gwi.returnedCols.push_back(spac);
@@ -8680,7 +8762,7 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
                     return ER_CHECK_NOT_IMPLEMENTED;
                 }
 
-                ReturnedColumn* rc = buildFunctionColumn(ifp, gwi, hasNonSupportItem);
+                ReturnedColumn* rc = buildFunctionColumn(ifp, gwi, hasNonSupportItem, true);
                 SRCP srcp(rc);
 
                 if (rc)
@@ -8736,7 +8818,10 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
                 {
                     hasNonSupportItem = false;
                     uint32_t before_size = funcFieldVec.size();
-                    parse_item(ifp, funcFieldVec, hasNonSupportItem, parseInfo);
+                    // MCOL-1510 Use gwi pointer here to catch funcs with
+                    // not supported aggregate args in projections,
+                    // e.g. NOT(SUM(i)).
+                    parse_item(ifp, funcFieldVec, hasNonSupportItem, parseInfo, &gwi);
                     uint32_t after_size = funcFieldVec.size();
 
                     // group by func and func in subquery can not be post processed
@@ -9865,7 +9950,7 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, SCSEP& csep, cal_gro
                         sel_query += ", ";
                 }
 
-                select_query.replace(lower_select_query.find("select *"), string("select *").length(), sel_query);
+                //select_query.replace(lower_select_query.find("select *"), string("select *").length(), sel_query);
             }
             else
             {

dbcon/mysql/ha_calpont_impl.cpp

@@ -5265,6 +5265,7 @@ int ha_calpont_impl_group_by_init(ha_calpont_group_by_handler* group_hand, TABLE
         // MCOL-1052 Send Items lists down to the optimizer.
         gi.groupByTables = group_hand->table_list;
         gi.groupByFields = group_hand->select;
+        gi.groupByAuxDescr = &group_hand->select_list_descr;
         gi.groupByWhere = group_hand->where;
         gi.groupByGroup = group_hand->group_by;
         gi.groupByOrder = group_hand->order_by;
@@ -5832,7 +5833,8 @@ int ha_calpont_impl_group_by_end(ha_calpont_group_by_handler* group_hand, TABLE*
 
         try
         {
-            sm::tpl_close(ti.tpl_ctx, &hndl, ci->stats);
+            if(hndl)
+                sm::tpl_close(ti.tpl_ctx, &hndl, ci->stats);
 
             ci->cal_conn_hndl = hndl;
 

dbcon/mysql/ha_calpont_impl_if.h

@@ -142,6 +142,7 @@ struct gp_walk_info
     std::map<std::string, execplan::ParseTree*> derivedTbFilterMap;
     uint32_t derivedTbCnt;
     std::vector<execplan::SCSEP> subselectList;
+    List<char>* groupByAuxDescr;
 
     // Kludge for Bug 750
     int32_t recursionLevel;
@@ -195,6 +196,7 @@ struct cal_table_info
 struct cal_group_info
 {
     cal_group_info() : groupByFields(0),
+        groupByAuxDescr(0),
         groupByTables(0),
         groupByWhere(0),
         groupByGroup(0),
@@ -205,6 +207,7 @@ struct cal_group_info
     ~cal_group_info() { }
 
     List<Item>* groupByFields; // MCOL-1052 SELECT
+    List<char>* groupByAuxDescr; //MCOL-1052 Auxilary column descriptions
     TABLE_LIST* groupByTables; // MCOL-1052 FROM
     Item*       groupByWhere; // MCOL-1052 WHERE
     ORDER*      groupByGroup; // MCOL-1052 GROUP BY
@@ -326,15 +329,14 @@ int getGroupPlan(gp_walk_info& gwi, SELECT_LEX& select_lex, execplan::SCSEP& cse
 void setError(THD* thd, uint32_t errcode, const std::string errmsg, gp_walk_info* gwi);
 void setError(THD* thd, uint32_t errcode, const std::string errmsg);
 void gp_walk(const Item* item, void* arg);
-void parse_item (Item* item, std::vector<Item_field*>& field_vec, bool& hasNonSupportItem, uint16& parseInfo);
+void parse_item (Item* item, std::vector<Item_field*>& field_vec, bool& hasNonSupportItem, uint16& parseInfo, gp_walk_info* gwip = NULL);
-execplan::ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport);
 const std::string bestTableName(const Item_field* ifp);
 bool isInfiniDB(TABLE* table_ptr);
 
 // execution plan util functions prototypes
-execplan::ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport);
+execplan::ReturnedColumn* buildReturnedColumn(Item* item, gp_walk_info& gwi, bool& nonSupport, bool pushdownHand = false);
-execplan::ReturnedColumn* buildFunctionColumn(Item_func* item, gp_walk_info& gwi, bool& nonSupport);
+execplan::ReturnedColumn* buildFunctionColumn(Item_func* item, gp_walk_info& gwi, bool& nonSupport, bool pushdownHand = false);
-execplan::ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool& nonSupport);
+execplan::ArithmeticColumn* buildArithmeticColumn(Item_func* item, gp_walk_info& gwi, bool& nonSupport, bool pushdownHand = false);
 execplan::ConstantColumn* buildDecimalColumn(Item* item, gp_walk_info& gwi);
 execplan::SimpleColumn* buildSimpleColumn(Item_field* item, gp_walk_info& gwi);
 execplan::FunctionColumn* buildCaseFunction(Item_func* item, gp_walk_info& gwi, bool& nonSupport);
@@ -346,7 +348,7 @@ void addIntervalArgs(Item_func* ifp, funcexp::FunctionParm& functionParms);
 void castCharArgs(Item_func* ifp, funcexp::FunctionParm& functionParms);
 void castDecimalArgs(Item_func* ifp, funcexp::FunctionParm& functionParms);
 void castTypeArgs(Item_func* ifp, funcexp::FunctionParm& functionParms);
-void parse_item (Item* item, std::vector<Item_field*>& field_vec, bool& hasNonSupportItem, uint16& parseInfo);
+//void parse_item (Item* item, std::vector<Item_field*>& field_vec, bool& hasNonSupportItem, uint16& parseInfo);
 bool isPredicateFunction(Item* item, gp_walk_info* gwip);
 execplan::ParseTree* buildRowPredicate(execplan::RowColumn* lhs, execplan::RowColumn* rhs, std::string predicateOp);
 bool buildRowColumnFilter(gp_walk_info* gwip, execplan::RowColumn* rhs, execplan::RowColumn* lhs, Item_func* ifp);