maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Varun,

Part#1 of the followup to our discussion about removing from the TREE object.

So I tried to get tree_remove() to work.

- I enabled deletion from the tree based on your analysis.
- then, I got the tree_remove code to compile by making these changes:


diff --git a/sql/item_sum.cc b/sql/item_sum.cc
index e137720..8bcd7c6 100644
--- a/sql/item_sum.cc
+++ b/sql/item_sum.cc
@@ -3835,10 +3835,13 @@ bool Item_func_group_concat::add()
       return 1;
     if(limit_clause && (tree->elements_in_tree > row_limit+offset_limit))
     {
-      //tree_search_edge(tree,tree->parents,&tree->parents,
-      //offsetof(TREE_ELEMENT, left));
-      //tree_delete(tree, table->record[0] + table->s->null_bytes, 0,
-      //              tree->custom_arg);
+      TREE_ELEMENT *parents[MAX_TREE_HEIGHT+1];
+      TREE_ELEMENT **pos;
+      void *key;
+      key= tree_search_edge(tree, parents, &pos,
+                            offsetof(TREE_ELEMENT, left));
+      uint tree_key_length= table->s->reclength - table->s->null_bytes;
+      tree_delete(tree, key, tree_key_length, tree->custom_arg);
     }
   }
   /*
@@ -4036,7 +4039,10 @@ bool Item_func_group_concat::setup(THD *thd)
                                thd->variables.sortbuff_size/16), 0,
               tree_key_length, 
               group_concat_key_cmp_with_order, NULL, (void*) this,
-              MYF(MY_THREAD_SPECIFIC));
+              //MYF(MY_THREAD_SPECIFIC));
+              limit_clause? MYF(MY_TREE_WITH_DELETE) : MYF(0));
+    // psergey-note: it passes MY_THREAD_SPECIFIC, while init_tree
+    // actually checks for MY_TREE_WITH_DELETE!
   }
 
   if (distinct)

It still didn't remove the element!

I started debugging, ended up here:
  #0  group_concat_key_cmp_with_order (arg=0x7fffc78604d0, key1=0x7fffc78595a8, key2=0x7fffc78595a8) at /home/psergey/dev-git/10.2-varun/sql/item_sum.cc:3464
  #1  0x00005555563d54e3 in tree_delete (tree=0x7fffc7860620, key=0x7fffc78595a8, key_size=11, custom_arg=0x7fffc78604d0) at /home/psergey/dev-git/10.2-varun/mysys/tree.c:293
  #2  0x0000555555e14546 in Item_func_group_concat::add (this=0x7fffc78604d0) at /home/psergey/dev-git/10.2-varun/sql/item_sum.cc:3844
  #3  0x0000555555e15b8b in Aggregator_simple::add (this=0x7fffc78632a8) at /home/psergey/dev-git/10.2-varun/sql/item_sum.h:679
  #4  0x0000555555b52e89 in Item_sum::aggregator_add (this=0x7fffc78604d0) at /home/psergey/dev-git/10.2-varun/sql/item_sum.h:527
  #5  0x0000555555b48a98 in update_sum_func (func_ptr=0x7fffc78619b0) at /home/psergey/dev-git/10.2-varun/sql/sql_select.cc:23240


and the code at the end of group_concat_key_cmp_with_order says:

  /*
    We can't return 0 because in that case the tree class would remove this
    item as double value. This would cause problems for case-changes and
    if the returned values are not the same we do the sort on.
  */
  return 1;

So, group_concat_key_cmp_with_order() never returns 0, which means
tree_delete() can't find the element it wants to delete, and that's why no
element is ever deleted.

(The rest of discussion is coming in separate emails).

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog