Addressed the feedback from the review of Monty on the cumulative patch for

mwl#21.

include/my_tree.h

@@ -30,6 +30,15 @@ extern "C" {
 
 #define tree_set_pointer(element,ptr) *((uchar **) (element+1))=((uchar*) (ptr))
 
+/*
+  A tree with its flag set to TREE_ONLY_DUPS behaves differently on inserting 
+  an element that is not in the tree:
+  the element is not added at all, but instead tree_insert() returns a special
+  address TREE_ELEMENT_UNIQUE as an indication that the function has not failed
+  due to lack of memory. 
+*/
+
+#define TREE_ELEMENT_UNIQUE ((TREE_ELEMENT *) 1)
 #define TREE_NO_DUPS 1
 #define TREE_ONLY_DUPS 2
 

sql/filesort.cc

@@ -141,9 +141,6 @@ ha_rows filesort(THD *thd, TABLE *table, SORT_FIELD *sortorder, uint s_length,
   /* filesort cannot handle zero-length records. */
   DBUG_ASSERT(param.sort_length);
   param.ref_length= table->file->ref_length;
-  param.min_dupl_count= 0;
-  param.addon_field= 0;
-  param.addon_length= 0;
   if (!(table->file->ha_table_flags() & HA_FAST_KEY_READ) &&
       !table->fulltext_searched && !sort_positions)
   {
@@ -1197,8 +1194,11 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
   QUEUE queue;
   qsort2_cmp cmp;
   void *first_cmp_arg;
-  volatile THD::killed_state *killed= &current_thd->killed;
+  element_count dupl_count;
+  uchar *src;
   THD::killed_state not_killable;
+  uchar *unique_buff= param->unique_buff;
+  volatile THD::killed_state *killed= &current_thd->killed;
   DBUG_ENTER("merge_buffers");
 
   status_var_increment(current_thd->status_var.filesort_merge_passes);
@@ -1213,13 +1213,13 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
   rec_length= param->rec_length;
   res_length= param->res_length;
   sort_length= param->sort_length;
-  element_count dupl_count;
-  uchar *src;
   uint dupl_count_ofs= rec_length-sizeof(element_count);
   uint min_dupl_count= param->min_dupl_count;
-  offset= rec_length-
-          (flag && min_dupl_count ? sizeof(dupl_count) : 0)-res_length;
+  bool check_dupl_count= flag && min_dupl_count;
+  offset= (rec_length-
+           (flag && min_dupl_count ? sizeof(dupl_count) : 0)-res_length);
   uint wr_len= flag ? res_length : rec_length;
+  uint wr_offset= flag ? offset : 0;
   maxcount= (ulong) (param->keys/((uint) (Tb-Fb) +1));
   to_start_filepos= my_b_tell(to_file);
   strpos= sort_buffer;
@@ -1228,7 +1228,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
   /* The following will fire if there is not enough space in sort_buffer */
   DBUG_ASSERT(maxcount!=0);
   
-  if (param->unique_buff)
+  if (unique_buff)
   {
     cmp= param->compare;
     first_cmp_arg= (void *) &param->cmp_context;
@@ -1253,25 +1253,22 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
     queue_insert(&queue, (uchar*) buffpek);
   }
 
-  if (param->unique_buff)
+  if (unique_buff)
   {
     /* 
        Called by Unique::get()
-       Copy the first argument to param->unique_buff for unique removal.
+       Copy the first argument to unique_buff for unique removal.
        Store it also in 'to_file'.
-
-       This is safe as we know that there is always more than one element
-       in each block to merge (This is guaranteed by the Unique:: algorithm
     */
     buffpek= (BUFFPEK*) queue_top(&queue);
-    memcpy(param->unique_buff, buffpek->key, rec_length);
+    memcpy(unique_buff, buffpek->key, rec_length);
     if (min_dupl_count)
-      memcpy(&dupl_count, param->unique_buff+dupl_count_ofs, 
+      memcpy(&dupl_count, unique_buff+dupl_count_ofs, 
              sizeof(dupl_count));
     buffpek->key+= rec_length;
     if (! --buffpek->mem_count)
     {
-      if (!(error= (int) read_to_buffer(from_file,buffpek,
+      if (!(error= (int) read_to_buffer(from_file, buffpek,
                                         rec_length)))
       {
         VOID(queue_remove(&queue,0));
@@ -1297,7 +1294,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
       src= buffpek->key;
       if (cmp)                                        // Remove duplicates
       {
-        if (!(*cmp)(first_cmp_arg, &(param->unique_buff),
+        if (!(*cmp)(first_cmp_arg, &unique_buff,
                     (uchar**) &buffpek->key))
 	{
           if (min_dupl_count)
@@ -1310,24 +1307,32 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
         }
         if (min_dupl_count)
 	{
-          memcpy(param->unique_buff+dupl_count_ofs, &dupl_count,
+          memcpy(unique_buff+dupl_count_ofs, &dupl_count,
                  sizeof(dupl_count));
         }
-	src= param->unique_buff;
+	src= unique_buff;
       }
         
-      if (!flag || !min_dupl_count || dupl_count >= min_dupl_count)
+      /* 
+        Do not write into the output file if this is the final merge called
+        for a Unique object used for intersection and dupl_count is less
+        than min_dupl_count.
+        If the Unique object is used to intersect N sets of unique elements
+        then for any element:
+        dupl_count >= N <=> the element is occurred in each of these N sets.
+      */          
+      if (!check_dupl_count || dupl_count >= min_dupl_count)
       {
-        if (my_b_write(to_file, src+(flag ? offset : 0), wr_len))
+        if (my_b_write(to_file, src+wr_offset, wr_len))
         {
           error=1; goto err;                        /* purecov: inspected */
         }
       }
       if (cmp)
       {   
-        memcpy(param->unique_buff, (uchar*) buffpek->key, rec_length);
+        memcpy(unique_buff, (uchar*) buffpek->key, rec_length);
         if (min_dupl_count)
-          memcpy(&dupl_count, param->unique_buff+dupl_count_ofs, 
+          memcpy(&dupl_count, unique_buff+dupl_count_ofs, 
                  sizeof(dupl_count));
       }
       if (!--max_rows)
@@ -1340,7 +1345,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
       buffpek->key+= rec_length;
       if (! --buffpek->mem_count)
       {
-        if (!(error= (int) read_to_buffer(from_file,buffpek,
+        if (!(error= (int) read_to_buffer(from_file, buffpek,
                                           rec_length)))
         {
           VOID(queue_remove(&queue,0));
@@ -1363,7 +1368,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
   */
   if (cmp)
   {
-    if (!(*cmp)(first_cmp_arg, &(param->unique_buff), (uchar**) &buffpek->key))
+    if (!(*cmp)(first_cmp_arg, &unique_buff, (uchar**) &buffpek->key))
     {
       if (min_dupl_count)
       {
@@ -1376,13 +1381,13 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
     }
 
     if (min_dupl_count)
-      memcpy(param->unique_buff+dupl_count_ofs, &dupl_count,
+      memcpy(unique_buff+dupl_count_ofs, &dupl_count,
              sizeof(dupl_count));
 
-    if (!flag || !min_dupl_count || dupl_count >= min_dupl_count)
+    if (!check_dupl_count || dupl_count >= min_dupl_count)
     {
-      src= param->unique_buff;
-      if (my_b_write(to_file, src+(flag ? offset : 0), wr_len))
+      src= unique_buff;
+      if (my_b_write(to_file, src+wr_offset, wr_len))
       {
         error=1; goto err;                        /* purecov: inspected */
       }
@@ -1404,7 +1409,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
     max_rows-= buffpek->mem_count;
     if (flag == 0)
     {
-      if (my_b_write(to_file,(uchar*) buffpek->key,
+      if (my_b_write(to_file, (uchar*) buffpek->key,
                      (rec_length*buffpek->mem_count)))
       {
         error= 1; goto err;                        /* purecov: inspected */
@@ -1418,11 +1423,12 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
            src != end ;
            src+= rec_length)
       {
-        if (flag && min_dupl_count &&
-            memcmp(&min_dupl_count, src+dupl_count_ofs, 
-                   sizeof(dupl_count_ofs))<0)
+        if (check_dupl_count)
+        {
+          memcpy((uchar *) &dupl_count, src+dupl_count_ofs, sizeof(dupl_count)); 
+          if (dupl_count < min_dupl_count)
 	    continue;
-        
+        }
         if (my_b_write(to_file, src, wr_len))
         {
           error=1; goto err;                        
@@ -1430,7 +1436,7 @@ int merge_buffers(SORTPARAM *param, IO_CACHE *from_file,
       }
     }
   }
-  while ((error=(int) read_to_buffer(from_file,buffpek, rec_length))
+  while ((error=(int) read_to_buffer(from_file, buffpek, rec_length))
          != -1 && error != 0);
 
 end:

sql/mysql_priv.h

@@ -340,6 +340,9 @@ class Default_object_creation_ctx : public Object_creation_ctx
 */
 #define TIME_FOR_COMPARE_ROWID  (TIME_FOR_COMPARE*100)
 
+/* cost1 is better that cost2 only if cost1 + COST_EPS < cost2 */
+#define COST_EPS  0.001
+
 /*
   For sequential disk seeks the cost formula is:
     DISK_SEEK_BASE_COST + DISK_SEEK_PROP_COST * #blocks_to_skip  

sql/opt_range.h

@@ -328,6 +328,7 @@ class QUICK_SELECT_I
     selects output and/or can produce output suitable for merging.
   */
   virtual void add_info_string(String *str) {}
+
   /*
     Return 1 if any index used by this quick select
     uses field which is marked in passed bitmap.
@@ -406,9 +407,11 @@ class QUICK_RANGE_SELECT : public QUICK_SELECT_I
                                 QUICK_RANGE_SELECT *pk_quick_select,
                                 READ_RECORD *read_record,
                                 bool intersection,
+                                key_map *filtered_scans,
                                 Unique **unique_ptr);
 
   friend class QUICK_SELECT_DESC;
+  friend class QUICK_INDEX_SORT_SELECT;
   friend class QUICK_INDEX_MERGE_SELECT;
   friend class QUICK_INDEX_INTERSECT_SELECT;
   friend class QUICK_ROR_INTERSECT_SELECT;
@@ -464,40 +467,44 @@ class QUICK_RANGE_SELECT_GEOM: public QUICK_RANGE_SELECT
 
 
 /*
-  QUICK_INDEX_MERGE_SELECT - index_merge access method quick select.
+  QUICK_INDEX_SORT_SELECT is the base class for the common functionality of:
+  - QUICK_INDEX_MERGE_SELECT, access based on multi-index merge/union 
+  - QUICK_INDEX_INTERSECT_SELECT, access based on  multi-index intersection 
+    
 
-    QUICK_INDEX_MERGE_SELECT uses
+    QUICK_INDEX_SORT_SELECT uses
      * QUICK_RANGE_SELECTs to get rows
-     * Unique class to remove duplicate rows
+     * Unique class
+       - to remove duplicate rows for QUICK_INDEX_MERGE_SELECT
+       - to intersect rows for QUICK_INDEX_INTERSECT_SELECT
 
   INDEX MERGE OPTIMIZER
-    Current implementation doesn't detect all cases where index_merge could
+    Current implementation doesn't detect all cases where index merge could
     be used, in particular:
-     * index_merge will never be used if range scan is possible (even if
-       range scan is more expensive)
 
-     * index_merge+'using index' is not supported (this the consequence of
+     * index merge+'using index' is not supported (this the consequence of
        the above restriction)
 
      * If WHERE part contains complex nested AND and OR conditions, some ways
-       to retrieve rows using index_merge will not be considered. The choice
+       to retrieve rows using index merge will not be considered. The choice
        of read plan may depend on the order of conjuncts/disjuncts in WHERE
        part of the query, see comments near imerge_list_or_list and
        SEL_IMERGE::or_sel_tree_with_checks functions for details.
 
-     * There is no "index_merge_ref" method (but index_merge on non-first
+     * There is no "index_merge_ref" method (but index merge on non-first
        table in join is possible with 'range checked for each record').
 
-    See comments around SEL_IMERGE class and test_quick_select for more
-    details.
 
   ROW RETRIEVAL ALGORITHM
 
-    index_merge uses Unique class for duplicates removal.  index_merge takes
-    advantage of Clustered Primary Key (CPK) if the table has one.
-    The index_merge algorithm consists of two phases:
+    index merge/intersection uses Unique class for duplicates removal. 
+    index merge/intersection takes advantage of Clustered Primary Key (CPK)
+    if the table has one.
+    The index merge/intersection algorithm consists of two phases:
+
+    Phase 1 
+    (implemented by a QUICK_INDEX_MERGE_SELECT::read_keys_and_merge call):
 
-    Phase 1 (implemented in QUICK_INDEX_MERGE_SELECT::prepare_unique):
     prepare()
     {
       activate 'index only';
@@ -511,32 +518,31 @@ class QUICK_RANGE_SELECT_GEOM: public QUICK_RANGE_SELECT
       deactivate 'index only';
     }
 
-    Phase 2 (implemented as sequence of QUICK_INDEX_MERGE_SELECT::get_next
-    calls):
+    Phase 2 
+    (implemented as sequence of QUICK_INDEX_MERGE_SELECT::get_next calls):
 
     fetch()
     {
-      retrieve all rows from row pointers stored in Unique;
+      retrieve all rows from row pointers stored in Unique
+      (merging/intersecting them);
       free Unique;
+      if (! intersection) 
         retrieve all rows for CPK scan;
     }
 */
 
-class QUICK_INDEX_MERGE_SELECT : public QUICK_SELECT_I
+class QUICK_INDEX_SORT_SELECT : public QUICK_SELECT_I
 {
+protected:
   Unique *unique;
 public:
-  QUICK_INDEX_MERGE_SELECT(THD *thd, TABLE *table);
-  ~QUICK_INDEX_MERGE_SELECT();
+  QUICK_INDEX_SORT_SELECT(THD *thd, TABLE *table);
+  ~QUICK_INDEX_SORT_SELECT();
 
   int  init();
   int  reset(void);
-  int  get_next();
   bool reverse_sorted() { return false; }
   bool unique_key_range() { return false; }
-  int get_type() { return QS_TYPE_INDEX_MERGE; }
-  void add_keys_and_lengths(String *key_names, String *used_lengths);
-  void add_info_string(String *str);
   bool is_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
@@ -544,60 +550,54 @@ class QUICK_INDEX_MERGE_SELECT : public QUICK_SELECT_I
 
   bool push_quick_back(QUICK_RANGE_SELECT *quick_sel_range);
 
-  /* range quick selects this index_merge read consists of */
+  /* range quick selects this index merge/intersect consists of */
   List<QUICK_RANGE_SELECT> quick_selects;
 
   /* quick select that uses clustered primary key (NULL if none) */
   QUICK_RANGE_SELECT* pk_quick_select;
 
-  /* true if this select is currently doing a clustered PK scan */
-  bool  doing_pk_scan;
-
   MEM_ROOT alloc;
   THD *thd;
-  int read_keys_and_merge();
+  virtual int read_keys_and_merge()= 0;
 
   /* used to get rows collected in Unique */
   READ_RECORD read_record;
 };
 
-class QUICK_INDEX_INTERSECT_SELECT : public QUICK_SELECT_I
+
+
+class QUICK_INDEX_MERGE_SELECT : public QUICK_INDEX_SORT_SELECT
 {
-  Unique *unique;
+private:
+  /* true if this select is currently doing a clustered PK scan */
+  bool  doing_pk_scan;
+protected:
+  int read_keys_and_merge();
+
 public:
-  QUICK_INDEX_INTERSECT_SELECT(THD *thd, TABLE *table);
-  ~QUICK_INDEX_INTERSECT_SELECT();
+  QUICK_INDEX_MERGE_SELECT(THD *thd, TABLE *table)
+    :QUICK_INDEX_SORT_SELECT(thd, table) {}
 
-  int  init();
-  int  reset(void);
   int get_next();
-  bool reverse_sorted() { return false; }
-  bool unique_key_range() { return false; }
-  int get_type() { return QS_TYPE_INDEX_INTERSECT; }
+  int get_type() { return QS_TYPE_INDEX_MERGE; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
   void add_info_string(String *str);
-  bool is_keys_used(const MY_BITMAP *fields);
-#ifndef DBUG_OFF
-  void dbug_dump(int indent, bool verbose);
-#endif
-
-  bool push_quick_back(QUICK_RANGE_SELECT *quick_sel_range);
-
-  /* range quick selects this index_merge read consists of */
-  List<QUICK_RANGE_SELECT> quick_selects;
-
-  /* quick select that uses clustered primary key (NULL if none) */
-  QUICK_RANGE_SELECT* pk_quick_select;
-
-  /* true if this select is currently doing a clustered PK scan */
-  bool  doing_pk_scan;
+};
 
-  MEM_ROOT alloc;
-  THD *thd;
+class QUICK_INDEX_INTERSECT_SELECT : public QUICK_INDEX_SORT_SELECT
+{
+protected:
   int read_keys_and_merge();
 
-  /* used to get rows collected in Unique */
-  READ_RECORD read_record;
+public:
+  QUICK_INDEX_INTERSECT_SELECT(THD *thd, TABLE *table)
+    :QUICK_INDEX_SORT_SELECT(thd, table) {}
+
+  key_map filtered_scans;
+  int get_next();
+  int get_type() { return QS_TYPE_INDEX_INTERSECT; }
+  void add_keys_and_lengths(String *key_names, String *used_lengths);
+  void add_info_string(String *str);
 };
 
 

sql/sql_class.h

@@ -2998,7 +2998,7 @@ class Unique :public Sql_alloc
   bool flush();
   uint size;
   uint full_size;
-  uint min_dupl_count;
+  uint min_dupl_count;   /* always 0 for unions, > 0 for intersections */
 
 public:
   ulong elements;
@@ -3022,6 +3022,7 @@ class Unique :public Sql_alloc
 
   bool get(TABLE *table);
   
+  /* Cost of searching for an element in the tree */
   inline static double get_search_cost(uint tree_elems, uint compare_factor)
   {
     return log((double) tree_elems) / (compare_factor * M_LN2);