MySQL Bugs: #26208: a typo (wrong variable name) in mysql_prepare

Bug #26208	a typo (wrong variable name) in mysql_prepare_update function's source code?
Submitted:	9 Feb 2007 6:38	Modified:	20 May 2008 23:20
Reporter:	Shan Lu	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Prepared statements	Severity:	S3 (Non-critical)
Version:	5.1.16-BK, mysql-5.2.0-falcon-alpha	OS:	Linux (Linux)
Assigned to:	Kristofer Pettersson	CPU Architecture:	Any

Description:
In mysql-5.2.0-falcon-alpha, file sql_update.cc, function:
bool mysql_prepare_update(THD *thd, TABLE_LIST *table_list,
                         Item **conds, uint order_num, ORDER *order)
includes following lines of code:

line 743  bzero((char*) &tables,sizeof(tables));        // For ORDER BY
line 744  tables.table= table;
line 745  tables.alias= table_list->alias;
line 746  thd->lex->allow_sum_func= 0;

line 748  if (setup_tables_and_check_access(thd, &select_lex->context,
line 749                            &select_lex->top_join_list,
line 750                            table_list,
line 751                            &select_lex->leaf_tables,
line 752                            FALSE, UPDATE_ACL, SELECT_ACL) ||
line 753  setup_conds(thd, table_list, select_lex->leaf_tables, conds) ||
line 754      select_lex->setup_ref_array(thd, order_num) ||
line 755      setup_order(thd, select_lex->ref_pointer_array,
line 756                  table_list, all_fields, all_fields, order) ||
                        ~~~~~~~~~~~~~
                           <--should this be 'tables' instead of table_list? 

Actually, the varaible tables initialized on line 743--745 are not used anymore in the following code. Based on the comment 'For order by', I guess it should be used in setup_order function call. 

Or maybe the variable `tables' is completely redundant and should not appear in this function?

I have found similar code in sql_table.cc line 6752 -- line 6759; sql_delete.cc line 164 -- line 169; and mysql_update function in old version msyql. In all other places, `tables' are used in setup_order. Therefore, I am thinking this might be a typo.

A related suspect is: one line 744 and 745, why only tables.alias is written? Is it all right to leave tables.table_name and tables.db 0? It seems that in sql_table.cc line 6752, in similar situation, tables.db, tables.table_name and tables.alias are all assigned proper value.

How to repeat:
no test case; got by code analysis tool

Suggested fix:
mysql_prepare_update@sql_update.cc
755      setup_order(thd, select_lex->ref_pointer_array,
- 756                  table_list, all_fields, all_fields, order) ||
+ 756                  &tables, all_fields, all_fiels, order)||

Thank you for a problem report. Verified just as described on latest 5.1.16-BK also. mysql_prepare_update implemented as follows:

bool mysql_prepare_update(THD *thd, TABLE_LIST *table_list,
                         Item **conds, uint order_num, ORDER *order)
{
  Item *fake_conds= 0;
  TABLE *table= table_list->table;
  TABLE_LIST tables;
  List<Item> all_fields;
  SELECT_LEX *select_lex= &thd->lex->select_lex;
  DBUG_ENTER("mysql_prepare_update");

#ifndef NO_EMBEDDED_ACCESS_CHECKS
  table_list->grant.want_privilege= table->grant.want_privilege=
    (SELECT_ACL & ~table->grant.privilege);
  table_list->register_want_access(SELECT_ACL);
#endif

  bzero((char*) &tables,sizeof(tables));        // For ORDER BY
  tables.table= table;
  tables.alias= table_list->alias;
  thd->lex->allow_sum_func= 0;

  if (setup_tables_and_check_access(thd, &select_lex->context,
                                    &select_lex->top_join_list,
                                    table_list,
                                    &select_lex->leaf_tables,
                                    FALSE, UPDATE_ACL, SELECT_ACL) ||
      setup_conds(thd, table_list, select_lex->leaf_tables, conds) ||
      select_lex->setup_ref_array(thd, order_num) ||
      setup_order(thd, select_lex->ref_pointer_array,
                  table_list, all_fields, all_fields, order) ||
      setup_ftfuncs(select_lex))
    DBUG_RETURN(TRUE);

  /* Check that we are not using table that we are updating in a sub select */
  {
    TABLE_LIST *duplicate;
    if ((duplicate= unique_table(thd, table_list, table_list->next_global)))
    {
      update_non_unique_table_error(table_list, "UPDATE", duplicate);
      my_error(ER_UPDATE_TABLE_USED, MYF(0), table_list->table_name);
      DBUG_RETURN(TRUE);
    }
  }
  select_lex->fix_prepare_information(thd, conds, &fake_conds);
  DBUG_RETURN(FALSE);
}

So, tables variable is really not used after initialization.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/44727

ChangeSet@1.2583, 2008-04-01 14:41:13+02:00, thek@adventure.(none) +1 -0
  Bug#26208 a typo (wrong variable name) in mysql_prepare_update function's source code?
  
  This is a code clean up.
  Removed redundant (and unused) TABLE_LIST variable intended as an IN-
  parameter for setup_order.

Pushed into 6.0.6-alpha

Pushed into 5.1.25-rc

No changelog entry needed.

Pushed into 5.1.25-rc  (revid:sp1r-kostja@bodhi.(none)-20080520073817-17550) (version source revid:sp1r-kostja@bodhi.(none)-20080520073817-17550) (pib:3)