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

[9 Feb 2007 6:38] Shan Lu
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)||
[9 Feb 2007 11:21] Valeriy Kravchuk
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.
[1 Apr 2008 12:32] Bugs System
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.
[20 Apr 2008 13:00] Bugs System
Pushed into 6.0.6-alpha
[20 May 2008 21:22] Bugs System
Pushed into 5.1.25-rc
[20 May 2008 23:20] Paul DuBois
No changelog entry needed.
[28 Jul 2008 16:53] Bugs System
Pushed into 5.1.25-rc  (revid:sp1r-kostja@bodhi.(none)-20080520073817-17550) (version source revid:sp1r-kostja@bodhi.(none)-20080520073817-17550) (pib:3)

Portions of this website are copyright © 2001, 2002 The PHP Group

Page generated in 0.042 sec. using MySQL 8.4.7-u6-cloud

Timestamp references displayed by the system are UTC.
Content reproduced on this site is the property of the respective copyright holders.
It is not reviewed in advance by Oracle and does not necessarily represent the opinion of Oracle or any other party.