Bug#8670

  Rework to resolve ambigious grammer: conflict in join expression
  handling of parentheses for nested joins and derived tables.
  Tests included of failing statements
Optimize item construction for AND/OR logical expressions

mysql-test/r/select.result

@@ -2446,3 +2446,12 @@ cast((a - b) as unsigned)
 1
 18446744073709551615
 drop table t1;
+create table t1 (a int, b int);
+create table t2 like t1;
+select t1.a from (t1 inner join t2 on t1.a=t2.a) where t2.a=1;
+a
+select t1.a from ((t1 inner join t2 on t1.a=t2.a)) where t2.a=1;
+a
+select x.a, y.a, z.a from ( (t1 x inner join t2 y on x.a=y.a) inner join t2 z on y.a=z.a) WHERE x.a=1;
+a	a	a
+drop table t1,t2;

mysql-test/t/select.test

@@ -2015,3 +2015,13 @@ select a-b , (a-b < 0)  from t1 order by 1;
 select a-b as d, (a-b >= 0), b from t1 group by b having d >= 0;
 select cast((a - b) as unsigned) from t1 order by 1;
 drop table t1;
+
+#
+# Bug#8670
+#
+create table t1 (a int, b int);
+create table t2 like t1;
+select t1.a from (t1 inner join t2 on t1.a=t2.a) where t2.a=1;
+select t1.a from ((t1 inner join t2 on t1.a=t2.a)) where t2.a=1;
+select x.a, y.a, z.a from ( (t1 x inner join t2 y on x.a=y.a) inner join t2 z on y.a=z.a) WHERE x.a=1;
+drop table t1,t2;

sql/sql_parse.cc

@@ -5852,6 +5852,7 @@ TABLE_LIST *st_select_lex::end_nested_join(THD *thd)
 {
   TABLE_LIST *ptr;
   DBUG_ENTER("end_nested_join");
+  DBUG_ASSERT(embedding);
   ptr= embedding;
   join_list= ptr->join_list;
   embedding= ptr->embedding;
@@ -5865,6 +5866,12 @@ TABLE_LIST *st_select_lex::end_nested_join(THD *thd)
     join_list->push_front(embedded);
     ptr= embedded;
   }
+  else
+  if (nested_join->join_list.elements == 0)
+  {
+    join_list->pop();
+    DBUG_RETURN(0);
+  }
   DBUG_RETURN(ptr);
 }
 

sql/sql_yacc.yy

@@ -52,6 +52,13 @@ const LEX_STRING null_lex_str={0,0};
 		      ER_WARN_DEPRECATED_SYNTAX, \
 		      ER(ER_WARN_DEPRECATED_SYNTAX), (A), (B));
 
+#define TEST_ASSERT(A) \
+  if (!(A)) \
+  {					\
+    yyerror(ER(ER_SYNTAX_ERROR));	\
+    YYABORT;				\
+  }
+
 /* Helper for parsing "IS [NOT] truth_value" */
 inline Item *is_truth_value(Item *A, bool v1, bool v2)
 {
@@ -692,6 +699,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
         opt_var_ident_type delete_option opt_temporary all_or_any opt_distinct
         opt_ignore_leaves fulltext_options spatial_type union_option
         start_transaction_opts opt_chain opt_release
+        union_opt select_derived_init
 
 %type <ulong_num>
 	ULONG_NUM raid_types merge_insert_types
@@ -737,6 +745,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
 %type <table_list>
 	join_table_list  join_table
         table_factor table_ref
+        select_derived derived_table_list
 
 %type <date_time_type> date_time_type;
 %type <interval> interval
@@ -771,6 +780,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
 %type <variable> internal_variable_name
 
 %type <select_lex> in_subselect in_subselect_init
+	get_select_lex
 
 %type <boolfunc2creator> comp_op
 
@@ -4022,13 +4032,53 @@ optional_braces:
 
 /* all possible expressions */
 expr:	
-	expr or bool_term	{ $$= new Item_cond_or($1,$3); }
-	| expr XOR bool_term	{ $$= new Item_cond_xor($1,$3); }
-	| bool_term ;
+	  bool_term { Select->expr_list.push_front(new List<Item>); }
+          bool_or_expr
+          {
+            List<Item> *list= Select->expr_list.pop();
+            if (list->elements)
+            {
+              list->push_front($1);
+              $$= new Item_cond_or(*list);
+              /* optimize construction of logical OR to reduce
+                 amount of objects for complex expressions */
+            }
+            else
+              $$= $1;
+            delete list;
+          }
+	;
+
+bool_or_expr:
+	/* empty */
+        | bool_or_expr or bool_term
+          { Select->expr_list.head()->push_back($3); }
+        ;
 
 bool_term:
-	bool_term and bool_factor { $$= new Item_cond_and($1,$3); }
-	| bool_factor ;
+	bool_term XOR bool_term { $$= new Item_cond_xor($1,$3); }
+	| bool_factor { Select->expr_list.push_front(new List<Item>); }
+          bool_and_expr
+          {
+            List<Item> *list= Select->expr_list.pop();
+            if (list->elements)
+            {
+              list->push_front($1);
+              $$= new Item_cond_and(*list);
+              /* optimize construction of logical AND to reduce
+                 amount of objects for complex expressions */
+            }
+            else
+              $$= $1;
+            delete list;
+          }
+	;
+
+bool_and_expr:
+	/* empty */
+        | bool_and_expr and bool_factor
+          { Select->expr_list.head()->push_back($3); }
+        ;
 
 bool_factor:
 	NOT_SYM bool_factor	{ $$= negate_expression(YYTHD, $2); }
@@ -4911,6 +4961,7 @@ when_list2:
 	    sel->when_list.head()->push_back($5);
 	  };
 
+/* Warning - may return NULL in case of incomplete SELECT */
 table_ref:
         table_factor            { $$=$1; }
         | join_table            { $$=$1; }
@@ -4922,36 +4973,47 @@ table_ref:
         ;
 
 join_table_list:
+	derived_table_list		{ TEST_ASSERT($$=$1); }
+	;
+        
+/* Warning - may return NULL in case of incomplete SELECT */
+derived_table_list:
         table_ref { $$=$1; }
-        | join_table_list ',' table_ref { $$=$3; }
+        | derived_table_list ',' table_ref
+          {
+            TEST_ASSERT($1 && ($$=$3));
+          }
         ;
 
 join_table:
-        table_ref normal_join table_ref { $$=$3; }
+        table_ref normal_join table_ref { TEST_ASSERT($1 && ($$=$3)); }
 	| table_ref STRAIGHT_JOIN table_factor
-	  { $3->straight=1; $$=$3 ; }
+	  {  TEST_ASSERT($1 && ($$=$3)); $3->straight=1; }
 	| table_ref normal_join table_ref ON expr
-	  { add_join_on($3,$5); $$=$3; }
+	  { TEST_ASSERT($1 && ($$=$3)); add_join_on($3,$5); }
 	| table_ref normal_join table_ref
 	  USING
 	  {
 	    SELECT_LEX *sel= Select;
+            TEST_ASSERT($1 && $3);
             sel->save_names_for_using_list($1, $3);
 	  }
 	  '(' using_list ')'
 	  { add_join_on($3,$7); $$=$3; }
 
 	| table_ref LEFT opt_outer JOIN_SYM table_ref ON expr
-	  { add_join_on($5,$7); $5->outer_join|=JOIN_TYPE_LEFT; $$=$5; }
+	  { TEST_ASSERT($1 && $5); add_join_on($5,$7); $5->outer_join|=JOIN_TYPE_LEFT; $$=$5; }
 	| table_ref LEFT opt_outer JOIN_SYM table_factor
 	  {
 	    SELECT_LEX *sel= Select;
+            TEST_ASSERT($1 && $5);
             sel->save_names_for_using_list($1, $5);
 	  }
 	  USING '(' using_list ')'
 	  { add_join_on($5,$9); $5->outer_join|=JOIN_TYPE_LEFT; $$=$5; }
 	| table_ref NATURAL LEFT opt_outer JOIN_SYM table_factor
 	  {
+            TEST_ASSERT($1 && $6);
 	    add_join_natural($1,$6);
 	    $6->outer_join|=JOIN_TYPE_LEFT;
 	    $$=$6;
@@ -4959,6 +5021,7 @@ join_table:
 	| table_ref RIGHT opt_outer JOIN_SYM table_ref ON expr
           { 
 	    LEX *lex= Lex;
+            TEST_ASSERT($1 && $5);
             if (!($$= lex->current_select->convert_right_join()))
               YYABORT;
             add_join_on($$, $7);
@@ -4966,6 +5029,7 @@ join_table:
 	| table_ref RIGHT opt_outer JOIN_SYM table_factor
 	  {
 	    SELECT_LEX *sel= Select;
+            TEST_ASSERT($1 && $5);
             sel->save_names_for_using_list($1, $5);
 	  }
 	  USING '(' using_list ')'
@@ -4977,13 +5041,14 @@ join_table:
           }
 	| table_ref NATURAL RIGHT opt_outer JOIN_SYM table_factor
 	  {
+            TEST_ASSERT($1 && $6);
 	    add_join_natural($6,$1);
 	    LEX *lex= Lex;
             if (!($$= lex->current_select->convert_right_join()))
               YYABORT;
 	  }
 	| table_ref NATURAL JOIN_SYM table_factor
-	  { add_join_natural($1,$4); $$=$4; };
+	  { TEST_ASSERT($1 && ($$=$4)); add_join_natural($1,$4); };
         
 
 normal_join:
@@ -4992,6 +5057,7 @@ normal_join:
 	| CROSS JOIN_SYM	{}
 	;
 
+/* Warning - may return NULL in case of incomplete SELECT */
 table_factor:
 	{
 	  SELECT_LEX *sel= Select;
@@ -5010,50 +5076,96 @@ table_factor:
 	    YYABORT;
           sel->add_joined_table($$);
 	}
-        | '('
+	| '{' ident table_ref LEFT OUTER JOIN_SYM table_ref ON expr '}'
+	  { TEST_ASSERT($3 && $7); add_join_on($7,$9); $7->outer_join|=JOIN_TYPE_LEFT; $$=$7; }
+	| select_derived_init get_select_lex select_derived2
           {
             LEX *lex= Lex;
-            if (lex->current_select->init_nested_join(lex->thd))
-              YYABORT;
+            SELECT_LEX *sel= lex->current_select;
+            if ($1)
+            {
+	      if (sel->set_braces(1))
+	      {
+	        yyerror(ER(ER_SYNTAX_ERROR));
+	        YYABORT;
+	      }
+              /* select in braces, can't contain global parameters */
+	      if (sel->master_unit()->fake_select_lex)
+                sel->master_unit()->global_parameters=
+                   sel->master_unit()->fake_select_lex;
+            }
+            if ($2->init_nested_join(lex->thd))
+              YYABORT;            
+            $$= 0;
+            /* incomplete derived tables return NULL, we must be 
+               nested in select_derived rule to be here. */
           }
-          join_table_list ')'
+	| '(' get_select_lex select_derived union_opt ')' opt_table_alias
+	{
+          /* Use $2 instead of Lex->current_select as derived table will
+             alter value of Lex->current_select. */
+
+          if (!($3 || $6) && $2->embedding && 
+              !$2->embedding->nested_join->join_list.elements)
           {
-            LEX *lex= Lex;
-            if (!($$= lex->current_select->end_nested_join(lex->thd)))
-              YYABORT;
+            /* we have a derived table ($3 == NULL) but no alias,
+               Since we are nested in further parentheses so we
+               can pass NULL to the outer level parentheses
+               Permits parsing of "((((select ...))) as xyz)" */
+            $$= 0;
           }
-	| '{' ident table_ref LEFT OUTER JOIN_SYM table_ref ON expr '}'
-	  { add_join_on($7,$9); $7->outer_join|=JOIN_TYPE_LEFT; $$=$7; }
-	| '(' select_derived union_opt ')' opt_table_alias
-	{
-	  LEX *lex=Lex;
-	  SELECT_LEX_UNIT *unit= lex->current_select->master_unit();
-	  lex->current_select= unit->outer_select();
-	  if (!($$= lex->current_select->
-                add_table_to_list(lex->thd, new Table_ident(unit), $5, 0,
-				  TL_READ,(List<String> *)0,
-	                          (List<String> *)0)))
+          else
+          if (!$3)
+          {
+            /* Handle case of derived table, alias may be NULL if there
+               are no outer parentheses, add_table_to_list() will throw
+               error in this case */
+	    LEX *lex=Lex;
+            SELECT_LEX *sel= lex->current_select;
+	    SELECT_LEX_UNIT *unit= sel->master_unit();
+	    lex->current_select= sel= unit->outer_select();
+	    if (!($$= sel->
+                  add_table_to_list(lex->thd, new Table_ident(unit), $6, 0,
+				    TL_READ,(List<String> *)0,
+	                            (List<String> *)0)))
 
+	      YYABORT;
+            sel->add_joined_table($$);           
+          }
+	  else
+          if ($4 || $6)
+	  {
+            /* simple nested joins cannot have aliases or unions */
+            yyerror(ER(ER_SYNTAX_ERROR));
 	    YYABORT;
-          lex->current_select->add_joined_table($$);           
-	};
-
+	  }
+          else
+            $$= $3;
+	}
+        ;
 
+/* handle contents of parentheses in join expression */
 select_derived:
-	SELECT_SYM select_derived2
-	| '(' select_derived ')'
+	  get_select_lex
 	  {
-            SELECT_LEX *sel= Select;
-	    if (sel->set_braces(1))
-	    {
+            LEX *lex= Lex;
+            if ($1->init_nested_join(lex->thd))
+              YYABORT;
+          }
+          derived_table_list
+          {
+            LEX *lex= Lex;
+            /* for normal joins, $3 != NULL and end_nested_join() != NULL,
+               for derived tables, both must equal NULL */
+            
+            if (!($$= $1->end_nested_join(lex->thd)) && $3)
+              YYABORT;
+            if (!$3 && $$)
+            {
 	      yyerror(ER(ER_SYNTAX_ERROR));
 	      YYABORT;
-	    }
-            /* select in braces, can't contain global parameters */
-	    if (sel->master_unit()->fake_select_lex)
-              sel->master_unit()->global_parameters=
-                 sel->master_unit()->fake_select_lex;
-	  }
+            }
+          }
  	;
 
 select_derived2:
@@ -5081,6 +5193,29 @@ select_derived2:
 	opt_select_from
         ;
 
+get_select_lex:
+	/* Empty */ { $$= Select; }
+        ;
+
+select_derived_init:
+          SELECT_SYM
+          {
+            LEX *lex= Lex;
+            SELECT_LEX *sel= lex->current_select;        
+            TABLE_LIST *embedding;
+            if (!sel->embedding || sel->end_nested_join(lex->thd))
+	    {
+              /* we are not in parentheses */
+              yyerror(ER(ER_SYNTAX_ERROR));
+	      YYABORT;
+	    }
+            embedding= Select->embedding;
+            $$= embedding &&
+                !embedding->nested_join->join_list.elements;
+            /* return true if we are deeply nested */
+          }
+        ;
+
 opt_outer:
 	/* empty */	{}
 	| OUTER		{};
@@ -8077,13 +8212,12 @@ union_list:
 	;
 
 union_opt:
-	union_list {}
-	| optional_order_or_limit {}
+	/* Empty */ { $$= 0; }
+	| union_list { $$= 1; }
+	| union_order_or_limit { $$= 1; }
 	;
 
-optional_order_or_limit:
-	/* Empty */ {}
-	|
+union_order_or_limit:
 	  {
 	    THD *thd= YYTHD;
 	    LEX *lex= thd->lex;