fix for table/field caching mechanism

save moving ON/USING tables conditions to WHERE clause (BUG#2794)


sql/sql_base.cc:
  fix for table/field caching mechanism (global lock of it and right table passed as parameter)
  save moving ON/USING tables conditions to WHERE clause (BUG#2794)
sql/sql_class.cc:
  lock for using field/table cache in Item name resolution
sql/sql_class.h:
  lock for using field/table cache in Item name resolution
sql/sql_insert.cc:
  lock for using field/table cache in Item name resolution
tests/client_test.c:
  Test of PS queries with ON condition

sql/sql_base.cc

@@ -1884,7 +1884,7 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
   char name_buff[NAME_LEN+1];
 
 
-  if (item->cached_table)
+  if (!thd->no_table_fix_fields_cache && item->cached_table)
   {
     /*
       This shortcut is used by prepared statements. We assuming that 
@@ -1895,8 +1895,9 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables,
       field makes some prepared query ambiguous and so erronous, but we 
       accept this trade off.
     */
-    found= find_field_in_table(thd,tables->table,name,length,
-                               test(tables->table->grant.want_privilege),
+    found= find_field_in_table(thd, item->cached_table->table, name, length,
+                               test(item->cached_table->
+				    table->grant.want_privilege),
                                1, &(item->cached_field_index));
 
     if (found)
@@ -2381,6 +2382,8 @@ insert_fields(THD *thd,TABLE_LIST *tables, const char *db_name,
 int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
 {
   table_map not_null_tables= 0;
+  Statement *stmt= thd->current_statement, backup;
+
   DBUG_ENTER("setup_conds");
   thd->set_query_id=1;
   
@@ -2394,18 +2397,21 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
     not_null_tables= (*conds)->not_null_tables();
   }
 
+
   /* Check if we are using outer joins */
   for (TABLE_LIST *table=tables ; table ; table=table->next)
   {
     if (table->on_expr)
     {
+      if (stmt)
+	thd->set_n_backup_item_arena(stmt, &backup);
       /* Make a join an a expression */
       thd->where="on clause";
       
       if (!table->on_expr->fixed &&
 	  table->on_expr->fix_fields(thd, tables, &table->on_expr) ||
 	  table->on_expr->check_cols(1))
-	DBUG_RETURN(1);
+	goto err;
       thd->lex->current_select->cond_count++;
 
       /*
@@ -2418,18 +2424,22 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
       {
 	table->outer_join= 0;
 	if (!(*conds= and_conds(thd, *conds, table->on_expr, tables)))
-	  DBUG_RETURN(1);
+	  goto err;
 	table->on_expr=0;
       }
+      if (stmt)
+	thd->restore_backup_item_arena(stmt, &backup);
     }
     if (table->natural_join)
     {
+      if (stmt)
+	thd->set_n_backup_item_arena(stmt, &backup);
       /* Make a join of all fields with have the same name */
       TABLE *t1= table->table;
       TABLE *t2= table->natural_join->table;
       Item_cond_and *cond_and= new Item_cond_and();
       if (!cond_and)				// If not out of memory
-	DBUG_RETURN(1);
+	goto err;
       cond_and->top_level_item();
 
       Field **t1_field, *t2_field;
@@ -2445,7 +2455,7 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
           Item_func_eq *tmp=new Item_func_eq(new Item_field(*t1_field),
                                              new Item_field(t2_field));
           if (!tmp)
-            DBUG_RETURN(1);
+            goto err;
           /* Mark field used for table cache */
           (*t1_field)->query_id= t2_field->query_id= thd->query_id;
           cond_and->list.push_back(tmp);
@@ -2460,18 +2470,36 @@ int setup_conds(THD *thd,TABLE_LIST *tables,COND **conds)
 	if (!(*conds= and_conds(thd, *conds, cond_and, tables)) ||
 	    (*conds && !(*conds)->fixed &&
 	     (*conds)->fix_fields(thd, tables, conds)))
-	  DBUG_RETURN(1);
+	  goto err;
       }
       else
       {
 	table->on_expr= and_conds(thd, table->on_expr, cond_and, tables);
 	if (table->on_expr && !table->on_expr->fixed &&
 	    table->on_expr->fix_fields(thd, tables, &table->on_expr))
-	  DBUG_RETURN(1);
+	  goto err;
       }
+      if (stmt)
+	thd->restore_backup_item_arena(stmt, &backup);
     }
   }
+
+  if (stmt)
+  {
+    /*
+      We are in prepared statement preparation code => we should store
+      WHERE clause changing for next executions.
+
+      We do this ON -> WHERE transformation only once per PS statement.
+    */
+    thd->lex->current_select->where= *conds;
+  }
   DBUG_RETURN(test(thd->net.report_error));
+
+err:
+  if (stmt)
+      thd->restore_backup_item_arena(stmt, &backup);
+  DBUG_RETURN(1);
 }
 
 

sql/sql_class.cc

@@ -84,6 +84,7 @@ extern "C" void free_user_var(user_var_entry *entry)
 ****************************************************************************/
 
 THD::THD():user_time(0), current_statement(0), is_fatal_error(0),
+	   no_table_fix_fields_cache(0),
 	   last_insert_id_used(0),
 	   insert_id_used(0), rand_used(0), in_lock_tables(0),
 	   global_read_lock(0), bootstrap(0)

sql/sql_class.h

@@ -787,6 +787,12 @@ public:
   bool	     charset_is_system_charset, charset_is_collation_connection;
   bool       slow_command;
 
+  /*
+    Used in prepared statement to prevent using table/field cache in
+    Item_idend, bacuse it can point on removed table.
+  */
+  bool	     no_table_fix_fields_cache;
+
   /*
     If we do a purge of binary logs, log index info of the threads
     that are currently reading it needs to be adjusted. To do that
@@ -1044,13 +1050,15 @@ public:
 
 class select_insert :public select_result {
  public:
+  TABLE_LIST *table_list;
   TABLE *table;
   List<Item> *fields;
   ulonglong last_insert_id;
   COPY_INFO info;
 
-  select_insert(TABLE *table_par,List<Item> *fields_par,enum_duplicates duplic)
-    :table(table_par),fields(fields_par), last_insert_id(0)
+  select_insert(TABLE *table_par, List<Item> *fields_par,
+		enum_duplicates duplic)
+    :table(table_par), fields(fields_par), last_insert_id(0)
   {
     bzero((char*) &info,sizeof(info));
     info.handle_duplicates=duplic;

sql/sql_insert.cc

@@ -84,9 +84,14 @@ check_insert_fields(THD *thd,TABLE *table,List<Item> &fields,
     table_list.grant=table->grant;
 
     thd->dupp_field=0;
+    thd->no_table_fix_fields_cache= 1;
     if (setup_tables(&table_list) ||
 	setup_fields(thd, 0, &table_list,fields,1,0,0))
+    {
+      thd->no_table_fix_fields_cache= 0;
       return -1;
+    }
+    thd->no_table_fix_fields_cache= 0;
     if (thd->dupp_field)
     {
       my_error(ER_FIELD_SPECIFIED_TWICE,MYF(0), thd->dupp_field->field_name);

tests/client_test.c

@@ -8494,6 +8494,43 @@ static void test_bug3117()
   myquery(rc);
 }
 
+
+static void test_on()
+{
+  MYSQL_STMT *stmt;
+  int rc, i;
+  const char *query= "SELECT * FROM t2 join t1 on (t1.a=t2.a)";
+
+  myheader("test_on");
+  
+  rc = mysql_query(mysql, "DROP TABLE IF EXISTS t1,t2");
+  myquery(rc);
+  
+  rc= mysql_query(mysql,"CREATE TABLE t1 (a int , b int);");
+  myquery(rc);
+
+  rc= mysql_query(mysql,
+		  "insert into t1 values (1,1), (2, 2), (3,3), (4,4), (5,5);");
+  myquery(rc);
+
+  rc= mysql_query(mysql,"create table t2 select * from t1;");
+  myquery(rc);
+
+  stmt= mysql_prepare(mysql, query, strlen(query));
+  mystmt_init(stmt);
+  for (i= 0; i < 3; i++)
+  {
+    rc= mysql_execute(stmt);
+    mystmt(stmt, rc);
+    assert(5 == my_process_stmt_result(stmt));
+  }
+  mysql_stmt_close(stmt);
+
+  rc= mysql_query(mysql, "DROP TABLE t1,t2");
+  myquery(rc);
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -8753,6 +8790,8 @@ int main(int argc, char **argv)
 			       Item_field -> Item_ref */
     test_union();	    /* test union with prepared statements */
     test_bug3117();	    /* BUG#3117: LAST_INSERT_ID() */
+    test_on();		    /* ... join ... on(), BUG#2794 */
+
 
     end_time= time((time_t *)0);
     total_time+= difftime(end_time, start_time);