Skip to content

Commit

Permalink
MDEV-9641 MDEV-9644 NULLIF assertions
Browse files Browse the repository at this point in the history
* only copy args[0] to args[2] after fix_fields (when all item
  substitutions have already happened)

* change QT_ITEM_FUNC_NULLIF_TO_CASE (that allows to print NULLIF
  as CASE) to QT_ITEM_ORIGINAL_FUNC_NULLIF (that prohibits it).
  So that NULLIF-to-CASE is allowed by default and only disabled
  explicitly for SHOW VIEW|FUNCTION|PROCEDURE and mysql_make_view.
  By default it is allowed (in particular in error messages and
  debug output, that can happen anytime before or after optimizer).
  • Loading branch information
vuvova committed Mar 5, 2016
1 parent 5a3a79c commit ff93b77
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 31 deletions.
16 changes: 16 additions & 0 deletions mysql-test/r/null.result
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,22 @@ nullif(count(col1),0)
3
drop view v1;
drop table t1;
select nullif((select 1), (select 2));
nullif((select 1), (select 2))
1
create table t1 (f int);
insert into t1 values (1),(2);
select nullif( not f, 1 ) from t1;
nullif( not f, 1 )
0
0
drop table t1;
set names utf8;
create table t1 (f1 varchar(10));
insert into t1 values ('2015-12-31');
select power( timestamp( nullif( '2002-09-08', f1 ) ), 24 ) from t1;
ERROR 22003: DOUBLE value is out of range in 'pow(cast((case when '2002-09-08' = '2015-12-31' then NULL else '2002-09-08' end) as datetime(6)),24)'
drop table t1;
#
# End of 10.1 tests
#
19 changes: 19 additions & 0 deletions mysql-test/t/null.test
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,25 @@ select nullif(count(col1),0) from t1;
drop view v1;
drop table t1;

#
# MDEV-9644 Assertion `args[0] == args[2] || thd->stmt_arena->is_stmt_execute()' failed in Item_func_nullif::fix_length_and_dec()
#
select nullif((select 1), (select 2));
create table t1 (f int);
insert into t1 values (1),(2);
select nullif( not f, 1 ) from t1;
drop table t1;

#
# MDEV-9641 Assertion `args[0] == args[2] || _current_thd()->lex->context_analysis_only' failed in Item_func_nullif::print(String*, enum_query_type)
#
set names utf8;
create table t1 (f1 varchar(10));
insert into t1 values ('2015-12-31');
--error ER_DATA_OUT_OF_RANGE
select power( timestamp( nullif( '2002-09-08', f1 ) ), 24 ) from t1;
drop table t1;

--echo #
--echo # End of 10.1 tests
--echo #
26 changes: 15 additions & 11 deletions sql/item_cmpfunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2566,8 +2566,15 @@ void Item_func_nullif::update_used_tables()
void
Item_func_nullif::fix_length_and_dec()
{
if (!args[2]) // Only false if EOM
return;
/*
If this is the first invocation of fix_length_and_dec(), create the
third argument as a copy of the first. This cannot be done before
fix_fields(), because fix_fields() might replace items,
for exampe NOT x --> x==0, or (SELECT 1) --> 1.
See also class Item_func_nullif declaration.
*/
if (arg_count == 2)
args[arg_count++]= args[0];

THD *thd= current_thd;
/*
Expand Down Expand Up @@ -2706,7 +2713,7 @@ Item_func_nullif::fix_length_and_dec()
m_cache= args[0]->cmp_type() == STRING_RESULT ?
new (thd->mem_root) Item_cache_str_for_nullif(thd, args[0]) :
Item_cache::get_cache(thd, args[0]);
m_cache->setup(current_thd, args[0]);
m_cache->setup(thd, args[0]);
m_cache->store(args[0]);
m_cache->set_used_tables(args[0]->used_tables());
thd->change_item_tree(&args[0], m_cache);
Expand All @@ -2718,7 +2725,7 @@ Item_func_nullif::fix_length_and_dec()
unsigned_flag= args[2]->unsigned_flag;
fix_char_length(args[2]->max_char_length());
maybe_null=1;
setup_args_and_comparator(current_thd, &cmp);
setup_args_and_comparator(thd, &cmp);
}


Expand All @@ -2737,26 +2744,23 @@ void Item_func_nullif::print(String *str, enum_query_type query_type)
Therefore, after equal field propagation args[0] and args[2] can point
to different items.
*/
if (!(query_type & QT_ITEM_FUNC_NULLIF_TO_CASE) || args[0] == args[2])
if ((query_type & QT_ITEM_ORIGINAL_FUNC_NULLIF) || args[0] == args[2])
{
/*
If no QT_ITEM_FUNC_NULLIF_TO_CASE is requested,
If QT_ITEM_ORIGINAL_FUNC_NULLIF is requested,
that means we want the original NULLIF() representation,
e.g. when we are in:
SHOW CREATE {VIEW|FUNCTION|PROCEDURE}
The original representation is possible only if
args[0] and args[2] still point to the same Item.
The caller must pass call print() with QT_ITEM_FUNC_NULLIF_TO_CASE
The caller must never pass call print() with QT_ITEM_ORIGINAL_FUNC_NULLIF
if an expression has undergone some optimization
(e.g. equal field propagation done in optimize_cond()) already and
NULLIF() potentially has two different representations of "a":
- one "a" for comparison
- another "a" for the returned value!
Note, the EXPLAIN EXTENDED and EXPLAIN FORMAT=JSON routines
do pass QT_ITEM_FUNC_NULLIF_TO_CASE to print().
*/
DBUG_ASSERT(args[0] == args[2] || current_thd->lex->context_analysis_only);
str->append(func_name());
Expand Down Expand Up @@ -5789,7 +5793,7 @@ bool Item_func_not::fix_fields(THD *thd, Item **ref)
args[0]->under_not(this);
if (args[0]->type() == FIELD_ITEM)
{
/* replace "NOT <field>" with "<filed> == 0" */
/* replace "NOT <field>" with "<field> == 0" */
Query_arena backup, *arena;
Item *new_item;
bool rc= TRUE;
Expand Down
11 changes: 9 additions & 2 deletions sql/item_cmpfunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -998,11 +998,18 @@ class Item_func_nullif :public Item_func_hybrid_field_type
Item_cache *m_cache;
int compare();
public:
// Put "a" to args[0] for comparison and to args[2] for the returned value.
/*
Here we pass three arguments to the parent constructor, as NULLIF
is a three-argument function, it needs two copies of the first argument
(see above). But fix_fields() will be confused if we try to prepare the
same Item twice (if args[0]==args[2]), so we hide the third argument
(decrementing arg_count) and copy args[2]=args[0] again after fix_fields().
See also Item_func_nullif::fix_length_and_dec().
*/
Item_func_nullif(THD *thd, Item *a, Item *b):
Item_func_hybrid_field_type(thd, a, b, a),
m_cache(NULL)
{}
{ arg_count--; }
bool date_op(MYSQL_TIME *ltime, uint fuzzydate);
double real_op();
longlong int_op();
Expand Down
15 changes: 8 additions & 7 deletions sql/mysqld.h
Original file line number Diff line number Diff line change
Expand Up @@ -665,15 +665,16 @@ enum enum_query_type
/// If NULLIF(a,b) should print itself as
/// CASE WHEN a_for_comparison=b THEN NULL ELSE a_for_return_value END
/// when "a" was replaced to two different items
/// (e.g. by equal fields propagation in optimize_cond()).
/// The default behaviour is to print as NULLIF(a_for_return, b)
/// which should be Ok for SHOW CREATE {VIEW|PROCEDURE|FUNCTION}
/// as they are not affected by WHERE optimization.
QT_ITEM_FUNC_NULLIF_TO_CASE= (1 <<6),
/// (e.g. by equal fields propagation in optimize_cond())
/// or always as NULLIF(a, b).
/// The default behaviour is to use CASE syntax when
/// a_for_return_value is not the same as a_for_comparison.
/// SHOW CREATE {VIEW|PROCEDURE|FUNCTION} and other cases where the
/// original representation is required, should set this flag.
QT_ITEM_ORIGINAL_FUNC_NULLIF= (1 <<6),

/// This value means focus on readability, not on ability to parse back, etc.
QT_EXPLAIN= QT_TO_SYSTEM_CHARSET |
QT_ITEM_FUNC_NULLIF_TO_CASE |
QT_ITEM_IDENT_SKIP_CURRENT_DATABASE |
QT_ITEM_CACHE_WRAPPER_SKIP_DETAILS |
QT_ITEM_SUBSELECT_ID_ONLY,
Expand All @@ -682,7 +683,7 @@ enum enum_query_type
/// Be more detailed than QT_EXPLAIN.
/// Perhaps we should eventually include QT_ITEM_IDENT_SKIP_CURRENT_DATABASE
/// here, as it would give better readable results
QT_EXPLAIN_EXTENDED= QT_TO_SYSTEM_CHARSET | QT_ITEM_FUNC_NULLIF_TO_CASE
QT_EXPLAIN_EXTENDED= QT_TO_SYSTEM_CHARSET
};


Expand Down
18 changes: 12 additions & 6 deletions sql/sp_head.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3272,7 +3272,8 @@ sp_instr_set::print(String *str)
}
str->qs_append(m_offset);
str->qs_append(' ');
m_value->print(str, QT_ORDINARY);
m_value->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
}


Expand Down Expand Up @@ -3304,9 +3305,11 @@ void
sp_instr_set_trigger_field::print(String *str)
{
str->append(STRING_WITH_LEN("set_trigger_field "));
trigger_field->print(str, QT_ORDINARY);
trigger_field->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
str->append(STRING_WITH_LEN(":="));
value->print(str, QT_ORDINARY);
value->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
}

/*
Expand Down Expand Up @@ -3432,7 +3435,8 @@ sp_instr_jump_if_not::print(String *str)
str->qs_append('(');
str->qs_append(m_cont_dest);
str->qs_append(STRING_WITH_LEN(") "));
m_expr->print(str, QT_ORDINARY);
m_expr->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
}


Expand Down Expand Up @@ -3528,7 +3532,8 @@ sp_instr_freturn::print(String *str)
str->qs_append(STRING_WITH_LEN("freturn "));
str->qs_append((uint)m_type);
str->qs_append(' ');
m_value->print(str, QT_ORDINARY);
m_value->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
}

/*
Expand Down Expand Up @@ -4000,7 +4005,8 @@ sp_instr_set_case_expr::print(String *str)
str->qs_append(STRING_WITH_LEN(") "));
str->qs_append(m_case_expr_id);
str->qs_append(' ');
m_case_expr->print(str, QT_ORDINARY);
m_case_expr->print(str, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
}

uint
Expand Down
2 changes: 1 addition & 1 deletion sql/sql_lex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2689,7 +2689,7 @@ void st_select_lex::print_order(String *str,
{
if (order->counter_used)
{
if (query_type != QT_VIEW_INTERNAL)
if (!(query_type & QT_VIEW_INTERNAL))
{
char buffer[20];
size_t length= my_snprintf(buffer, 20, "%d", order->counter);
Expand Down
3 changes: 2 additions & 1 deletion sql/sql_show.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2352,7 +2352,8 @@ static int show_create_view(THD *thd, TABLE_LIST *table, String *buff)
We can't just use table->query, because our SQL_MODE may trigger
a different syntax, like when ANSI_QUOTES is defined.
*/
table->view->unit.print(buff, QT_ORDINARY);
table->view->unit.print(buff, enum_query_type(QT_ORDINARY |
QT_ITEM_ORIGINAL_FUNC_NULLIF));

if (table->with_check != VIEW_CHECK_NONE)
{
Expand Down
8 changes: 5 additions & 3 deletions sql/sql_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -901,9 +901,11 @@ static int mysql_register_view(THD *thd, TABLE_LIST *view,
ulong sql_mode= thd->variables.sql_mode & MODE_ANSI_QUOTES;
thd->variables.sql_mode&= ~MODE_ANSI_QUOTES;

lex->unit.print(&view_query, QT_VIEW_INTERNAL);
lex->unit.print(&is_query,
enum_query_type(QT_TO_SYSTEM_CHARSET | QT_WITHOUT_INTRODUCERS));
lex->unit.print(&view_query, enum_query_type(QT_VIEW_INTERNAL |
QT_ITEM_ORIGINAL_FUNC_NULLIF));
lex->unit.print(&is_query, enum_query_type(QT_TO_SYSTEM_CHARSET |
QT_WITHOUT_INTRODUCERS |
QT_ITEM_ORIGINAL_FUNC_NULLIF));

thd->variables.sql_mode|= sql_mode;
}
Expand Down

0 comments on commit ff93b77

Please sign in to comment.