Fix for BUG#15229.

The cause of this bug was a design flaw due to which the list of natural
join columns was incorrectly computed and stored for nested joins that
are not natural joins, but are operands (possibly indirect) of nested joins.

The patch corrects the flaw in a such a way, that the result columns of a
table reference are materialized only if it is a leaf table (that is, only
if it is a view, stored table, or natural/using join).


mysql-test/r/join.result:
  Added test for BUG#15229 and uncommented failing test cases of
  BUG#15357 (now fixed by this patch).
mysql-test/t/join.test:
  Added test for BUG#15229 and uncommented failing test cases of
  BUG#15357 (now fixed by this patch).
sql/sql_base.cc:
  - Do not materialize the result columns of regular nested joins
    (that are not natural/using joins).
  - Moved most of the code that creates/adds new natural join column
    references to the method 'get_or_create_column_ref', and simplified
    'mark_common_columns'.
  - Replaced a call to 'get_or_create_column_ref' with 'get_natural_column_ref'
    where it is for sure all columns are alredy created.
sql/table.cc:
  - Modified the method 'get_or_create_column_ref' so that it adds itself
    the newly created natural join columns to the respective table reference.
sql/table.h:
  - Modified the method 'get_or_create_column_ref' so that it adds itself
    the newly created natural join columns to the respective table reference.

mysql-test/r/join.result

@@ -475,6 +475,22 @@ b	c	a	c	b	y
 1	10	2	3	1	2
 1	3	2	3	1	11
 1	3	2	3	1	2
+select * from t5 natural join ((t1 natural join t2), (t3 natural join t4));
+y	z	b	c	a	c	b
+11	4	1	10	2	3	1
+11	4	1	3	2	3	1
+select * from  ((t1 natural join t2),  (t3 natural join t4)) natural join t5;
+y	b	c	a	c	b	z
+11	1	10	2	3	1	4
+11	1	3	2	3	1	4
+select * from t5 natural join ((t1 natural join t2) cross join (t3 natural join t4));
+y	z	b	c	a	c	b
+11	4	1	10	2	3	1
+11	4	1	3	2	3	1
+select * from  ((t1 natural join t2) cross join (t3 natural join t4)) natural join t5;
+y	b	c	a	c	b	z
+11	1	10	2	3	1	4
+11	1	3	2	3	1	4
 select * from (t1 join t2 using (b)) join (t3 join t4 using (c)) using (c);
 c	b	a	b	y
 3	1	2	1	11
@@ -665,6 +681,8 @@ select * from ((t1 natural join t2), (t3 natural join t4)) natural join t6;
 ERROR 23000: Column 'c' in from clause is ambiguous
 select * from ((t1 natural join t2), (t3 natural join t4)) natural join t6;
 ERROR 23000: Column 'c' in from clause is ambiguous
+select * from t6 natural join ((t1 natural join t2),  (t3 natural join t4));
+ERROR 23000: Column 'c' in from clause is ambiguous
 select * from (t1 join t2 on t1.b=t2.b) natural join (t3 natural join t4);
 ERROR 23000: Column 'b' in from clause is ambiguous
 select * from  (t3 natural join t4) natural join (t1 join t2 on t1.b=t2.b);
@@ -673,6 +691,8 @@ select * from (t3 join (t4 natural join t5) on (b < z))
 natural join
 (t1 natural join t2);
 ERROR 23000: Column 'c' in from clause is ambiguous
+select * from (t1 natural join t2) natural join (t3 join (t4 natural join t5) on (b < z));
+ERROR 23000: Column 'c' in from clause is ambiguous
 select t1.b from v1a;
 ERROR 42S22: Unknown column 't1.b' in 'field list'
 select * from v1a join v1b on t1.b = t2.b;
@@ -692,3 +712,30 @@ drop view v2b;
 drop view v3a;
 drop view v3b;
 drop view v4;
+create table t1 (a1 int, a2 int);
+create table t2 (a1 int, b int);
+create table t3 (c1 int, c2 int);
+create table t4 (c2 int);
+insert into t1 values (1,1);
+insert into t2 values (1,1);
+insert into t3 values (1,1);
+insert into t4 values (1);
+select * from t1 join t2 using (a1) join t3 on b=c1 join t4 using (c2);
+c2	a1	a2	b	c1
+1	1	1	1	1
+select * from t3 join (t1 join t2 using (a1)) on b=c1 join t4 using (c2);
+c2	c1	a1	a2	b
+1	1	1	1	1
+select a2 from t1 join t2 using (a1) join t3 on b=c1 join t4 using (c2);
+a2
+1
+select a2 from t3 join (t1 join t2 using (a1)) on b=c1 join t4 using (c2);
+a2
+1
+select a2 from ((t1 join t2 using (a1)) join t3 on b=c1) join t4 using (c2);
+a2
+1
+select a2 from ((t1 natural join t2) join t3 on b=c1) natural join t4;
+a2
+1
+drop table t1,t2,t3,t4;

mysql-test/t/join.test

@@ -408,11 +408,10 @@ select * from t1 natural join (t2 natural join (t3 natural join t4));
 select * from t5 natural right join (t4 natural right join ((t2 natural right join t1) natural right join t3));
 select * from (t1 natural join t2), (t3 natural join t4);
 -- MySQL extension - nested comma ',' operator instead of cross join.
--- BUG#15357 - natural join with nested cross-join results in incorrect columns
--- select * from t5 natural join ((t1 natural join t2), (t3 natural join t4));
--- select * from  ((t1 natural join t2),  (t3 natural join t4)) natural join t5;
--- select * from t5 natural join ((t1 natural join t2) cross join (t3 natural join t4));
--- select * from  ((t1 natural join t2) cross join (t3 natural join t4)) natural join t5;
+select * from t5 natural join ((t1 natural join t2), (t3 natural join t4));
+select * from  ((t1 natural join t2),  (t3 natural join t4)) natural join t5;
+select * from t5 natural join ((t1 natural join t2) cross join (t3 natural join t4));
+select * from  ((t1 natural join t2) cross join (t3 natural join t4)) natural join t5;
 
 select * from (t1 join t2 using (b)) join (t3 join t4 using (c)) using (c);
 select * from (t1 join t2 using (b)) natural join (t3 join t4 using (c));
@@ -500,8 +499,7 @@ select * from ((t1 natural join t2), (t3 natural join t4)) natural join t6;
 -- error 1052
 select * from ((t1 natural join t2), (t3 natural join t4)) natural join t6;
 -- error 1052
--- BUG#15357: doesn't detect non-unique column 'c', as in the above query.
--- select * from t6 natural join ((t1 natural join t2),  (t3 natural join t4));
+select * from t6 natural join ((t1 natural join t2),  (t3 natural join t4));
 -- error 1052
 select * from (t1 join t2 on t1.b=t2.b) natural join (t3 natural join t4);
 -- error 1052
@@ -512,11 +510,7 @@ select * from (t3 join (t4 natural join t5) on (b < z))
               natural join
               (t1 natural join t2);
 -- error 1052
--- BUG#15357: this query should return an ambiguous column error
--- Expected result: the query must return error with duplicate column 'c'
---select * from (t1 natural join t2)
---              natural join
---              (t3 join (t4 natural join t5) on (b < z));
+select * from (t1 natural join t2) natural join (t3 join (t4 natural join t5) on (b < z));
 
 -- error 1054
 select t1.b from v1a;
@@ -540,4 +534,27 @@ drop view v3a;
 drop view v3b;
 drop view v4;
 
+#
+# BUG#15229 - columns of nested joins that are not natural joins incorrectly
+# materialized
+#
+create table t1 (a1 int, a2 int);
+create table t2 (a1 int, b int);
+create table t3 (c1 int, c2 int);
+create table t4 (c2 int);
+
+insert into t1 values (1,1);
+insert into t2 values (1,1);
+insert into t3 values (1,1);
+insert into t4 values (1);
+
+select * from t1 join t2 using (a1) join t3 on b=c1 join t4 using (c2);
+select * from t3 join (t1 join t2 using (a1)) on b=c1 join t4 using (c2);
+select a2 from t1 join t2 using (a1) join t3 on b=c1 join t4 using (c2);
+select a2 from t3 join (t1 join t2 using (a1)) on b=c1 join t4 using (c2);
+select a2 from ((t1 join t2 using (a1)) join t3 on b=c1) join t4 using (c2);
+select a2 from ((t1 natural join t2) join t3 on b=c1) natural join t4;
+
+drop table t1,t2,t3,t4;
+
 # End of tests for WL#2486 - natural/using join

sql/sql_base.cc

@@ -3608,8 +3608,18 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
   Natural_join_column *nj_col_1, *nj_col_2;
   const char *field_name_1;
   Query_arena *arena, backup;
-  bool add_columns= TRUE;
   bool result= TRUE;
+  bool first_outer_loop= TRUE;
+  /*
+    Leaf table references to which new natural join columns are added
+    if the leaves are != NULL.
+  */
+  TABLE_LIST *leaf_1= (table_ref_1->nested_join &&
+                       !table_ref_1->is_natural_join) ?
+                      NULL : table_ref_1;
+  TABLE_LIST *leaf_2= (table_ref_2->nested_join &&
+                       !table_ref_2->is_natural_join) ?
+                      NULL : table_ref_2;
 
   DBUG_ENTER("mark_common_columns");
   DBUG_PRINT("info", ("operand_1: %s  operand_2: %s",
@@ -3618,35 +3628,13 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
   *found_using_fields= 0;
   arena= thd->activate_stmt_arena_if_needed(&backup);
 
-  /*
-    TABLE_LIST::join_columns could be allocated by the previous call to
-    store_natural_using_join_columns() for the lower level of nested tables.
-  */
-  if (!table_ref_1->join_columns)
-  {
-    if (!(table_ref_1->join_columns= new List<Natural_join_column>))
-      goto err;
-    table_ref_1->is_join_columns_complete= FALSE;
-  }
-  if (!table_ref_2->join_columns)
-  {
-    if (!(table_ref_2->join_columns= new List<Natural_join_column>))
-      goto err;
-    table_ref_2->is_join_columns_complete= FALSE;
-  }
-
   for (it_1.set(table_ref_1); !it_1.end_of_fields(); it_1.next())
   {
-    bool is_created_1;
     bool found= FALSE;
-    if (!(nj_col_1= it_1.get_or_create_column_ref(&is_created_1)))
+    if (!(nj_col_1= it_1.get_or_create_column_ref(leaf_1)))
       goto err;
     field_name_1= nj_col_1->name();
 
-    /* If nj_col_1 was just created add it to the list of join columns. */
-    if (is_created_1)
-      table_ref_1->join_columns->push_back(nj_col_1);
-
     /*
       Find a field with the same name in table_ref_2.
 
@@ -3657,17 +3645,12 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
     nj_col_2= NULL;
     for (it_2.set(table_ref_2); !it_2.end_of_fields(); it_2.next())
     {
-      bool is_created_2;
       Natural_join_column *cur_nj_col_2;
       const char *cur_field_name_2;
-      if (!(cur_nj_col_2= it_2.get_or_create_column_ref(&is_created_2)))
+      if (!(cur_nj_col_2= it_2.get_or_create_column_ref(leaf_2)))
         goto err;
       cur_field_name_2= cur_nj_col_2->name();
 
-      /* If nj_col_1 was just created add it to the list of join columns. */
-      if (add_columns && is_created_2)
-        table_ref_2->join_columns->push_back(cur_nj_col_2);
-
       /*
         Compare the two columns and check for duplicate common fields.
         A common field is duplicate either if it was already found in
@@ -3686,9 +3669,15 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
         found= TRUE;
       }
     }
-    /* Force it_2.set() to use table_ref_2->join_columns. */
-    table_ref_2->is_join_columns_complete= TRUE;
-    add_columns= FALSE;
+    if (first_outer_loop && leaf_2)
+    {
+      /*
+        Make sure that the next inner loop "knows" that all columns
+        are materialized already.
+      */
+      leaf_2->is_join_columns_complete= TRUE;
+      first_outer_loop= FALSE;
+    }
     if (!found)
       continue;                                 // No matching field
 
@@ -3772,7 +3761,8 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
         ++(*found_using_fields);
     }
   }
-  table_ref_1->is_join_columns_complete= TRUE;
+  if (leaf_1)
+    leaf_1->is_join_columns_complete= TRUE;
 
   /*
     Everything is OK.
@@ -4625,16 +4615,15 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
 
         if (tables->is_natural_join)
         {
-          bool is_created;
           TABLE *field_table;
           /*
             In this case we are sure that the column ref will not be created
             because it was already created and stored with the natural join.
           */
           Natural_join_column *nj_col;
-          if (!(nj_col= field_iterator.get_or_create_column_ref(&is_created)))
+          if (!(nj_col= field_iterator.get_natural_column_ref()))
             DBUG_RETURN(TRUE);
-          DBUG_ASSERT(nj_col->table_field && !is_created);
+          DBUG_ASSERT(nj_col->table_field);
           field_table= nj_col->table_ref->table;
           if (field_table)
           {

sql/table.cc

@@ -2816,11 +2816,31 @@ GRANT_INFO *Field_iterator_table_ref::grant()
 
   SYNOPSIS
     Field_iterator_table_ref::get_or_create_column_ref()
-    is_created  [out] set to TRUE if the column was created,
-                      FALSE if we return an already created colum
+    parent_table_ref  the parent table reference over which the
+                      iterator is iterating
 
   DESCRIPTION
-    TODO
+    Create a new natural join column for the current field of the
+    iterator if no such column was created, or return an already
+    created natural join column. The former happens for base tables or
+    views, and the latter for natural/using joins. If a new field is
+    created, then the field is added to 'parent_table_ref' if it is
+    given, or to the original table referene of the field if
+    parent_table_ref == NULL.
+
+  NOTES
+    This method is designed so that when a Field_iterator_table_ref
+    walks through the fields of a table reference, all its fields
+    are created and stored as follows:
+    - If the table reference being iterated is a stored table, view or
+      natural/using join, store all natural join columns in a list
+      attached to that table reference.
+    - If the table reference being iterated is a nested join that is
+      not natural/using join, then do not materialize its result
+      fields. This is OK because for such table references
+      Field_iterator_table_ref iterates over the fields of the nested
+      table references (recursively). In this way we avoid the storage
+      of unnecessay copies of result columns of nested joins.
 
   RETURN
     #     Pointer to a column of a natural join (or its operand)
@@ -2828,22 +2848,28 @@ GRANT_INFO *Field_iterator_table_ref::grant()
 */
 
 Natural_join_column *
-Field_iterator_table_ref::get_or_create_column_ref(bool *is_created)
+Field_iterator_table_ref::get_or_create_column_ref(TABLE_LIST *parent_table_ref)
 {
   Natural_join_column *nj_col;
+  bool is_created= TRUE;
+  uint field_count;
+  TABLE_LIST *add_table_ref= parent_table_ref ?
+                             parent_table_ref : table_ref;
 
-  *is_created= TRUE;
   if (field_it == &table_field_it)
   {
     /* The field belongs to a stored table. */
     Field *field= table_field_it.field();
     nj_col= new Natural_join_column(field, table_ref);
+    field_count= table_ref->table->s->fields;
   }
   else if (field_it == &view_field_it)
   {
     /* The field belongs to a merge view or information schema table. */
     Field_translator *translated_field= view_field_it.field_translator();
     nj_col= new Natural_join_column(translated_field, table_ref);
+    field_count= table_ref->field_translation_end -
+                 table_ref->field_translation;
   }
   else
   {
@@ -2852,12 +2878,43 @@ Field_iterator_table_ref::get_or_create_column_ref(bool *is_created)
       already created via one of the two constructor calls above. In this case
       we just return the already created column reference.
     */
-    *is_created= FALSE;
+    DBUG_ASSERT(table_ref->is_join_columns_complete);
+    is_created= FALSE;
     nj_col= natural_join_it.column_ref();
     DBUG_ASSERT(nj_col);
   }
   DBUG_ASSERT(!nj_col->table_field ||
               nj_col->table_ref->table == nj_col->table_field->table);
+
+  /*
+    If the natural join column was just created add it to the list of
+    natural join columns of either 'parent_table_ref' or to the table
+    reference that directly contains the original field.
+  */
+  if (is_created)
+  {
+    /* Make sure not all columns were materialized. */
+    DBUG_ASSERT(!add_table_ref->is_join_columns_complete);
+    if (!add_table_ref->join_columns)
+    {
+      /* Create a list of natural join columns on demand. */
+      if (!(add_table_ref->join_columns= new List<Natural_join_column>))
+        return NULL;
+      add_table_ref->is_join_columns_complete= FALSE;
+    }
+    add_table_ref->join_columns->push_back(nj_col);
+    /*
+      If new fields are added to their original table reference, mark if
+      all fields were added. We do it here as the caller has no easy way
+      of knowing when to do it.
+      If the fields are being added to parent_table_ref, then the caller
+      must take care to mark when all fields are created/added.
+    */
+    if (!parent_table_ref &&
+        add_table_ref->join_columns->elements == field_count)
+      add_table_ref->is_join_columns_complete= TRUE;
+  }
+
   return nj_col;
 }
 

sql/table.h

@@ -782,7 +782,7 @@ public:
   GRANT_INFO *grant();
   Item *create_item(THD *thd) { return field_it->create_item(thd); }
   Field *field() { return field_it->field(); }
-  Natural_join_column *get_or_create_column_ref(bool *is_created);
+  Natural_join_column *get_or_create_column_ref(TABLE_LIST *parent_table_ref);
   Natural_join_column *get_natural_column_ref();
 };