mirror/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2026-05-02 13:15:32 +02:00
Code Issues Projects Releases Packages Wiki Activity

Implement some code review fixes for the fix for Bug#27430

Browse source 
"Crash in subquery code when in PS and table DDL changed after PREPARE"


include/my_sys.h:
  Add two new flags for my_error(). These flags help parameterize
  behavoiur of my_message_sql()
sql/item.cc:
  Update comments.
  Fix a typo in Item_param::set_param_type_and_swap_value()
sql/mysqld.cc:
  Implement two additional flags for my_error():
   - if ME_NO_SP_HANDLER is specified, ignore stored procedure continue/
      exit handlers
   - if ME_NO_WARNING_FOR_ERROR is specified, do not push warning
sql/sql_base.cc:
  Update comments.
  Rename a few methods.
sql/sql_class.h:
  Update and improve comments.
sql/sql_prepare.cc:
  Update comments.
  Style changes.
sql/table.h:
  Update comments.
  Style changes.
  Rename a few methods.
tests/mysql_client_test.c:
  Zero the bind array, to follow C API requirements.
This commit is contained in:
unknown 2008-05-18 01:51:18 +04:00
commit 1cf0b5cb89
8 changed files with 92 additions and 71 deletions
Download patch file Download diff file Expand all files Collapse all files

3
include/my_sys.h
View file

@ -90,6 +90,9 @@ extern int NEAR my_errno; /* Last error in mysys */
#define ME_COLOUR1 ((1 << ME_HIGHBYTE)) /* Possibly error-colours */
#define ME_COLOUR2 ((2 << ME_HIGHBYTE))
#define ME_COLOUR3 ((3 << ME_HIGHBYTE))
#define ME_FATALERROR 1024 /* Fatal statement error */
#define ME_NO_WARNING_FOR_ERROR 2048 /* Don't push a warning for error */
#define ME_NO_SP_HANDLER 4096 /* Don't call stored routine error handlers */
/* Bits in last argument to fn_format */
#define MY_REPLACE_DIR 1 /* replace dir in name with 'dir' */

12
sql/item.cc
View file

@ -3165,15 +3165,19 @@ void Item_param::print(String *str, enum_query_type query_type)
Preserve the original parameter types and values
when re-preparing a prepared statement.
Copy parameter type information and conversion function
pointers from a parameter of the old statement to the
corresponding parameter of the new one.
@details Copy parameter type information and conversion
function pointers from a parameter of the old statement
to the corresponding parameter of the new one.
Move parameter values from the old parameters to the new
one. We simply "exchange" the values, which allows
to save on allocation and character set conversion in
case a parameter is a string or a blob/clob.
The old parameter gets the value of this one, which
ensures that all memory of this parameter is freed
correctly.
@param[in] src parameter item of the original
prepared statement
*/
@ -3187,7 +3191,7 @@ Item_param::set_param_type_and_swap_value(Item_param *src)
item_type= src->item_type;
item_result_type= src->item_result_type;
collation.set(src->collation.collation);
collation.set(src->collation);
maybe_null= src->maybe_null;
null_value= src->null_value;
max_length= src->max_length;

4
sql/mysqld.cc
View file

@ -2843,6 +2843,7 @@ int my_message_sql(uint error, const char *str, myf MyFlags)
by the stored procedures code.
*/
if (thd->spcont &&
! (MyFlags & ME_NO_SP_HANDLER) &&
thd->spcont->handle_error(error, MYSQL_ERROR::WARN_LEVEL_ERROR, thd))
{
/*
@ -2852,7 +2853,8 @@ int my_message_sql(uint error, const char *str, myf MyFlags)
DBUG_RETURN(0);
}
if (!thd->no_warnings_for_error)
if (!thd->no_warnings_for_error &&
!(MyFlags & ME_NO_WARNING_FOR_ERROR))
{
/*
Suppress infinite recursion if there a memory allocation error

23
sql/sql_base.cc
View file

@ -3722,9 +3722,8 @@ void assign_new_table_id(TABLE_SHARE *share)
Compare metadata versions of an element obtained from the table
definition cache and its corresponding node in the parse tree.
If the new and the old values mismatch, invoke
@details If the new and the old values mismatch, invoke
Metadata_version_observer.
At prepared statement prepare, all TABLE_LIST version values are
NULL and we always have a mismatch. But there is no observer set
in THD, and therefore no error is reported. Instead, we update
@ -3738,8 +3737,8 @@ void assign_new_table_id(TABLE_SHARE *share)
@sa Execute_observer
@sa check_prepared_statement() to see cases when an observer is installed
@sa TABLE_LIST::is_metadata_version_equal()
@sa TABLE_SHARE::get_metadata_version()
@sa TABLE_LIST::is_metadata_id_equal()
@sa TABLE_SHARE::get_metadata_id()
@param[in] thd used to report errors
@param[in,out] tables TABLE_LIST instance created by the parser
@ -3755,27 +3754,28 @@ bool
check_and_update_table_version(THD *thd,
TABLE_LIST *tables, TABLE_SHARE *table_share)
{
if (! tables->is_metadata_version_equal(table_share))
if (! tables->is_metadata_id_equal(table_share))
{
if (thd->m_metadata_observer &&
thd->m_metadata_observer->check_metadata_change(thd))
thd->m_metadata_observer->report_error(thd))
{
/*
Version of the table share is different from the
previous execution of the prepared statement, and it is
unacceptable for this SQLCOM. Error has been reported.
*/
DBUG_ASSERT(thd->is_error());
return TRUE;
}
/* Always maintain the latest version */
tables->set_metadata_version(table_share);
/* Always maintain the latest version and type */
tables->set_metadata_id(table_share);
}
#if 0
#ifndef DBUG_OFF
/* Spuriously reprepare each statement. */
if (thd->m_metadata_observer && thd->stmt_arena->is_reprepared == FALSE)
{
thd->m_metadata_observer->check_metadata_change(thd);
thd->m_metadata_observer->report_error(thd);
return TRUE;
}
#endif
@ -3828,6 +3828,10 @@ retry:
if (share->is_view)
{
/*
This table is a view. Validate its metadata version: in particular,
that it was a view when the statement was prepared.
*/
if (check_and_update_table_version(thd, table_list, share))
goto err;
if (table_list->i_s_requested_object & OPEN_TABLE_ONLY)
@ -4622,6 +4626,7 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags)
}
tables->table->grant= tables->grant;
/* Check and update metadata version of a base table. */
if (check_and_update_table_version(thd, tables, tables->table->s))
{
result= -1;

18
sql/sql_class.h
View file

@ -57,16 +57,15 @@
class Metadata_version_observer
{
protected:
virtual ~Metadata_version_observer();
public:
virtual ~Metadata_version_observer();
/**
Check if a change of metadata is OK. In future
the signature of this method may be extended to accept the old
and the new versions, but since currently the check is very
simple, we only need the THD to report an error.
*/
virtual bool check_metadata_change(THD *thd)= 0;
virtual bool report_error(THD *thd)= 0;
};
@ -2842,6 +2841,19 @@ public:
#define CF_STATUS_COMMAND 4
#define CF_SHOW_TABLE_COMMAND 8
#define CF_WRITE_LOGS_COMMAND 16
/**
Must be set for SQL statements that may contain
Item expressions and/or use joins and tables.
Indicates that the parse tree of such statement may
contain rule-based optimizations that depend on metadata
(i.e. number of columns in a table), and consequently
that the statement must be re-prepared whenever
referenced metadata changes. Must not be set for
statements that themselves change metadata, e.g. RENAME,
ALTER and other DDL, since otherwise will trigger constant
reprepare. Consequently, complex item expressions and
joins are currently prohibited in these statements.
*/
#define CF_REEXECUTION_FRAGILE 32
/* Functions in sql_class.cc */

86
sql/sql_prepare.cc
View file

@ -124,7 +124,7 @@ public:
class Execute_observer: public Metadata_version_observer
{
public:
virtual bool check_metadata_change(THD *thd);
virtual bool report_error(THD *thd);
/** Set to TRUE if metadata of some used table has changed since prepare */
bool m_invalidated;
};
@ -137,14 +137,12 @@ public:
*/
bool
Execute_observer::check_metadata_change(THD *thd)
Execute_observer::report_error(THD *thd)
{
bool save_thd_no_warnings_for_error= thd->no_warnings_for_error;
DBUG_ENTER("Execute_observer::notify_about_metadata_change");
DBUG_ENTER("Execute_observer::report_error");
my_error(ER_NEED_REPREPARE, MYF(ME_NO_WARNING_FOR_ERROR|ME_NO_SP_HANDLER));
thd->no_warnings_for_error= TRUE;
my_error(ER_NEED_REPREPARE, MYF(0));
thd->no_warnings_for_error= save_thd_no_warnings_for_error;
m_invalidated= TRUE;
DBUG_RETURN(TRUE);
}
@ -984,7 +982,7 @@ static bool emb_insert_params_with_log(Prepared_statement *stmt,
/**
Setup data conversion routines using an array of parameter
markers from the original prepared statement.
Move the parameter data of the original prepared
Swap the parameter data of the original prepared
statement to the new one.
Used only when we re-prepare a prepared statement.
@ -2844,7 +2842,7 @@ Select_fetch_protocol_binary::send_data(List<Item> &fields)
****************************************************************************/
Prepared_statement::Prepared_statement(THD *thd_arg, Protocol *protocol_arg)
:Statement(0, &main_mem_root,
:Statement(NULL, &main_mem_root,
INITIALIZED, ++thd_arg->statement_id_counter),
thd(thd_arg),
result(thd_arg),
@ -3153,38 +3151,36 @@ Prepared_statement::set_parameters(String *expanded_query,
uchar *packet, uchar *packet_end)
{
bool is_sql_ps= packet == NULL;
bool res;
if (is_sql_ps)
{
/* SQL prepared statement */
if (set_params_from_vars(this, thd->lex->prepared_stmt_params,
expanded_query))
goto set_params_data_err;
res= set_params_from_vars(this, thd->lex->prepared_stmt_params,
expanded_query);
}
else if (param_count)
{
#ifndef EMBEDDED_LIBRARY
uchar *null_array= packet;
if (setup_conversion_functions(this, &packet, packet_end) ||
set_params(this, null_array, packet, packet_end,
expanded_query))
goto set_params_data_err;
res= (setup_conversion_functions(this, &packet, packet_end) ||
set_params(this, null_array, packet, packet_end, expanded_query));
#else
/*
In embedded library we re-install conversion routines each time
we set params, and also we don't need to parse packet.
So we do it in one function.
*/
if (set_params_data(this, expanded_query))
goto set_params_data_err;
/*
In embedded library we re-install conversion routines each time
we set parameters, and also we don't need to parse packet.
So we do it in one function.
*/
res= set_params_data(this, expanded_query);
#endif
}
return FALSE;
set_params_data_err:
my_error(ER_WRONG_ARGUMENTS, MYF(0),
is_sql_ps ? "EXECUTE" : "mysql_stmt_execute");
reset_stmt_params(this);
return TRUE;
if (res)
{
my_error(ER_WRONG_ARGUMENTS, MYF(0),
is_sql_ps ? "EXECUTE" : "mysql_stmt_execute");
reset_stmt_params(this);
}
return res;
}
@ -3249,6 +3245,7 @@ reexecute:
if (sql_command_flags[lex->sql_command] &
CF_REEXECUTION_FRAGILE)
{
DBUG_ASSERT(thd->m_metadata_observer == NULL);
thd->m_metadata_observer= &execute_observer;
}
@ -3260,12 +3257,13 @@ reexecute:
if (!(specialflag & SPECIAL_NO_PRIOR))
my_pthread_setprio(pthread_self(), WAIT_PRIOR);
thd->m_metadata_observer= 0;
thd->m_metadata_observer= NULL;
if (error && !thd->is_fatal_error && !thd->killed &&
execute_observer.m_invalidated &&
reprepare_attempt++ < MAX_REPREPARE_ATTEMPTS)
{
DBUG_ASSERT(thd->main_da.sql_errno() == ER_NEED_REPREPARE);
thd->clear_error();
error= reprepare();
@ -3299,44 +3297,40 @@ Prepared_statement::reprepare()
LEX_STRING saved_cur_db_name=
{ saved_cur_db_name_buf, sizeof(saved_cur_db_name_buf) };
LEX_STRING stmt_db_name= { db, db_length };
Prepared_statement *copy;
bool cur_db_changed;
bool error= TRUE;
bool error;
Prepared_statement copy(thd, &thd->protocol_text);
status_var_increment(thd->status_var.com_stmt_reprepare);
copy= new Prepared_statement(thd, &thd->protocol_text);
if (! copy)
return TRUE;
if (mysql_opt_change_db(thd, &stmt_db_name, &saved_cur_db_name, TRUE,
&cur_db_changed))
goto end;
return TRUE;
error= (name.str && copy->set_name(&name) ||
copy->prepare(query, query_length) ||
validate_metadata(copy));
error= (name.str && copy.set_name(&name) ||
copy.prepare(query, query_length) ||
validate_metadata(&copy));
if (cur_db_changed)
mysql_change_db(thd, &saved_cur_db_name, TRUE);
if (! error)
{
swap_prepared_statement(copy);
swap_parameter_array(param_array, copy->param_array, param_count);
swap_prepared_statement(&copy);
swap_parameter_array(param_array, copy.param_array, param_count);
#ifndef DBUG_OFF
is_reprepared= TRUE;
#endif
/*
Clear possible warnigns during re-prepare, it has to be completely
Clear possible warnings during reprepare, it has to be completely
transparent to the user. We use mysql_reset_errors() since
there were no separate query id issued for re-prepare.
Sic: we can't simply silence warnings during reprepare, because if
it's failed, we need to return all the warnings to the user.
*/
mysql_reset_errors(thd, TRUE);
}
end:
delete copy;
return error;
}

16
sql/table.h
View file

@ -478,10 +478,10 @@ typedef struct st_table_share
Let's try to explain why and how this limited solution allows
to validate prepared statements.
Firstly, spaces (in mathematical sense) of version numbers
Firstly, sets (in mathematical sense) of version numbers
never intersect for different metadata types. Therefore,
version id of a temporary table is never compared with
a version id of a view or a temporary table, and vice versa.
a version id of a view, and vice versa.
Secondly, for base tables, we know that each DDL flushes the
respective share from the TDC. This ensures that whenever
@ -530,11 +530,11 @@ typedef struct st_table_share
with a base table, a base table is replaced with a temporary
table and so on.
@sa TABLE_LIST::is_metadata_version_equal()
@sa TABLE_LIST::is_metadata_id_equal()
*/
ulong get_metadata_version() const
{
return tmp_table == SYSTEM_TMP_TABLE || is_view ? 0 : table_map_id;
return (tmp_table == SYSTEM_TMP_TABLE || is_view) ? 0 : table_map_id;
}
} TABLE_SHARE;
@ -1340,7 +1340,7 @@ struct TABLE_LIST
@sa check_and_update_table_version()
*/
inline
bool is_metadata_version_equal(TABLE_SHARE *s) const
bool is_metadata_id_equal(TABLE_SHARE *s) const
{
return (m_metadata_type == s->get_metadata_type() &&
m_metadata_version == s->get_metadata_version());
@ -1353,7 +1353,7 @@ struct TABLE_LIST
@sa check_and_update_table_version()
*/
inline
void set_metadata_version(TABLE_SHARE *s)
void set_metadata_id(TABLE_SHARE *s)
{
m_metadata_type= s->get_metadata_type();
m_metadata_version= s->get_metadata_version();
@ -1369,9 +1369,9 @@ private:
/* Remembered MERGE child def version. See top comment in ha_myisammrg.cc */
ulong child_def_version;
/** See comments for set_metadata_version() */
/** See comments for set_metadata_id() */
enum enum_metadata_type m_metadata_type;
/** See comments for set_metadata_version() */
/** See comments for set_metadata_id() */
ulong m_metadata_version;
};

1
tests/mysql_client_test.c
View file

@ -17418,6 +17418,7 @@ static void test_wl4166()
verify_param_count(stmt, 7);
bzero(my_bind, sizeof(my_bind));
/* tinyint */
my_bind[0].buffer_type= MYSQL_TYPE_TINY;
my_bind[0].buffer= (void *)&tiny_data;