MDEV-35422 Fix spider group by handler trying to use fake group by fields

This is a fixup of MDEV-26345 commit
77ed235d50.

In MDEV-26345 the spider group by handler was updated so that it uses
the item_ptr fields of Query::group_by and Query::order_by, instead of
item. This was and is because the call to
join->set_items_ref_array(join->items1) during the execution stage,
just before the execution replaces the order-by / group-by item arrays
with Item_temptable_field.

Spider traverses the item tree during the group by handler (gbh)
creation at the end of the optimization stage, and decides a gbh could
handle the execution of the query. Basically spider gbh can handle the
execution if it can construct a well-formed query, executes on the
data node, and store the results in the correct places. If so, it will
create one, otherwise it will return NULL and the execution will use
the usual handler (ha_spider instead of spider_group_by_handler). To
that end, the general principle is the items checked for creation
should be the same items later used for query construciton. Since in
MDEV-26345 we changed to use the item_ptr field instead of item field
of order-by and group-by in query construction, in this patch we do
the same for the gbh creation.

The item_ptr field could be the uninitialised NULL value during the
gbh creation. This is because the optimizer may replace a DISTINCT
with a GROUP BY, which only happens if the original GROUP BY is empty.
It creates the artificial GROUP BY by calling create_distinct_group(),
which creates the corresponding ORDER object with item field aligning
with somewhere in ref_pointer_array, but leaving item_ptr to be NULL.
When spider finds out that item_ptr is NULL, it knows there's some
optimizer skullduggery and it is passed a query different from the
original. Without a clear contract between the server layer and the
gbh, it is better to be safe than sorry and not create the gbh in this
case.

Also add a check and error reporting for the unlikely case of item_ptr
changing from non-NULL at gbh construction to NULL at execution to
prevent server crash.

Also, we remove a check added in MDEV-29480 of order by items being
aggregate functions. That check was added with the premise that spider
was including auxiliary SELECT items which is referenced by ORDER BY
items. This premise was no longer true since MDEV-26345, and caused
problems such as MDEV-29546, which was fixed by MDEV-26345.

storage/spider/mysql-test/spider/bugfix/r/mdev_34659.result

@@ -16,6 +16,13 @@ SELECT * FROM t2 ORDER BY CAST(c AS INET6);
 c
 456
 123
+SELECT * FROM t2 GROUP BY CAST(c AS char(60));
+c
+123
+456
+SELECT * FROM t2 GROUP BY CAST(c AS INET6);
+c
+456
 DROP TABLE t1,t2;
 drop server srv;
 for master_1

storage/spider/mysql-test/spider/bugfix/r/mdev_35422.result

@@ -0,0 +1,24 @@
+for master_1
+for child2
+for child3
+set spider_same_server_link= 1;
+CREATE SERVER srv FOREIGN DATA WRAPPER mysql
+OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root');
+create table t2 (c varchar(10));
+create table t1 (c varchar(10)) ENGINE=Spider
+COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"';
+insert into t1 values ('abc'), ('abd'), ('abcd'), ('abc');
+SELECT DISTINCT c FROM t1;
+c
+abc
+abd
+abcd
+SELECT DISTINCT c FROM t1 WHERE (c LIKE 'abc%');
+c
+abc
+abcd
+drop table t1, t2;
+drop server srv;
+for master_1
+for child2
+for child3

storage/spider/mysql-test/spider/bugfix/t/mdev_34659.test

@@ -13,6 +13,8 @@ CREATE TABLE t2 (c INT) ENGINE=Spider COMMENT='WRAPPER "mysql",SRV "srv",TABLE "
 insert into t2 values (456), (123);
 SELECT * FROM t2 ORDER BY CAST(c AS char(60));
 SELECT * FROM t2 ORDER BY CAST(c AS INET6);
+SELECT * FROM t2 GROUP BY CAST(c AS char(60));
+SELECT * FROM t2 GROUP BY CAST(c AS INET6);
 # Cleanup
 DROP TABLE t1,t2;
 drop server srv;

storage/spider/mysql-test/spider/bugfix/t/mdev_35422.test

@@ -0,0 +1,21 @@
+--disable_query_log
+--disable_result_log
+--source ../../t/test_init.inc
+--enable_result_log
+--enable_query_log
+set spider_same_server_link= 1;
+evalp CREATE SERVER srv FOREIGN DATA WRAPPER mysql
+OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root');
+create table t2 (c varchar(10));
+create table t1 (c varchar(10)) ENGINE=Spider
+COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"';
+insert into t1 values ('abc'), ('abd'), ('abcd'), ('abc');
+SELECT DISTINCT c FROM t1;
+SELECT DISTINCT c FROM t1 WHERE (c LIKE 'abc%');
+drop table t1, t2;
+drop server srv;
+--disable_query_log
+--disable_result_log
+--source ../../t/test_deinit.inc
+--enable_result_log
+--enable_query_log

storage/spider/spd_db_mysql.cc

@@ -14066,6 +14066,11 @@ int spider_mbase_handler::append_group_by_part(
   DBUG_RETURN(error_num);
 }
 
+/*
+  Append the GROUP BY part.
+
+  Only used by the group by handler for query construction.
+*/
 int spider_mbase_handler::append_group_by(
   ORDER *order,
   spider_string *str,
@@ -14084,6 +14089,13 @@ int spider_mbase_handler::append_group_by(
     str->q_append(SPIDER_SQL_GROUP_STR, SPIDER_SQL_GROUP_LEN);
     for (; order; order = order->next)
     {
+      /*
+        This is not expected to happen, as NULL check was performed
+        at the creation of the group by handler, and any NULL item_ptr
+        would have resulted in the gbh not being created.
+      */
+      if (!order->item_ptr)
+        DBUG_RETURN(ER_INTERNAL_ERROR);
       if ((error_num = spider_db_print_item_type(order->item_ptr, NULL, spider,
         str, alias, alias_length, dbton_id, use_fields, fields)))
       {
@@ -14123,6 +14135,11 @@ int spider_mbase_handler::append_order_by_part(
   DBUG_RETURN(error_num);
 }
 
+/*
+  Append the ORDER BY part.
+
+  Only used by the group by handler for query construction.
+*/
 int spider_mbase_handler::append_order_by(
   ORDER *order,
   spider_string *str,
@@ -14141,6 +14158,13 @@ int spider_mbase_handler::append_order_by(
     str->q_append(SPIDER_SQL_ORDER_STR, SPIDER_SQL_ORDER_LEN);
     for (; order; order = order->next)
     {
+      /*
+        This is not expected to happen, as NULL check was performed
+        at the creation of the group by handler, and any NULL item_ptr
+        would have resulted in the gbh not being created.
+      */
+      if (!order->item_ptr)
+        DBUG_RETURN(ER_INTERNAL_ERROR);
       if ((error_num = spider_db_print_item_type(order->item_ptr, NULL, spider,
         str, alias, alias_length, dbton_id, use_fields, fields)))
       {

storage/spider/spd_group_by_handler.cc

@@ -1622,8 +1622,10 @@ group_by_handler *spider_create_group_by_handler(
         {
           for (order = query->group_by; order; order = order->next)
           {
-            if (spider_db_print_item_type((*order->item), NULL, spider, NULL, NULL, 0,
-              roop_count, TRUE, fields_arg))
+            if (order->item_ptr == NULL ||
+                spider_db_print_item_type(order->item_ptr, NULL, spider,
+                                          NULL, NULL, 0, roop_count, TRUE,
+                                          fields_arg))
             {
               DBUG_PRINT("info",("spider dbton_id=%d can't create group by", roop_count));
               spider_clear_bit(dbton_bitmap, roop_count);
@@ -1640,10 +1642,10 @@ group_by_handler *spider_create_group_by_handler(
         {
           for (order = query->order_by; order; order = order->next)
           {
-            if ((*order->item)->type() == Item::SUM_FUNC_ITEM)
-              continue;
-            if (spider_db_print_item_type((*order->item), NULL, spider, NULL, NULL, 0,
-              roop_count, TRUE, fields_arg))
+            if (order->item_ptr == NULL ||
+                spider_db_print_item_type(order->item_ptr, NULL, spider,
+                                          NULL, NULL, 0, roop_count, TRUE,
+                                          fields_arg))
             {
               DBUG_PRINT("info",("spider dbton_id=%d can't create order by", roop_count));
               spider_clear_bit(dbton_bitmap, roop_count);