Hello Sergei,
On 01/23/2017 08:12 PM, Sergei Golubchik wrote: > Hi, Alexander! > > On Jan 20, Alexander Barkov wrote: >> Hello Sergei, >> >> Please review a patch for MDEV-11780. >> >> Thanks! > >> commit 1e6544808b1da0ca76d6c2e72318f41eeeb037a7 >> Author: Alexander Barkov <[email protected]> >> Date: Fri Jan 20 14:20:52 2017 +0400 >> >> MDEV-11780 Crash with PREPARE + SP out parameter + literal > > A bit more of a comment would've been nice here. > > I don't understand what's going on, why the crash. So, for > > EXECUTE stmt USING 10; > > it calls set_from_item. Literal or not, It doesn't change the fact > that Item_param itself is a Settable_routine_parameter. > > So, why does it crash in Protocol_text::send_out_parameters? Sending an updated patch, with comments. Thanks! > > Regards, > Sergei > Chief Architect MariaDB > and [email protected] >
commit e7f7de29e9a3ec676ba0b131d454bf3556fd992e Author: Alexander Barkov <[email protected]> Date: Mon Jan 23 22:29:48 2017 +0400 MDEV-11780 Crash with PREPARE + SP out parameter + literal Before "MDEV-10709 Expressions as parameters to Dynamic SQL" only user variables were syntactically allowed as EXECUTE parameters. User variables were OK as both IN and OUT parameters. When Item_param was bound to an actual parameter (a user variable), it automatically meant that the bound Item was settable. The DBUG_ASSERT() in Protocol_text::send_out_parameters() guarded that the actual parameter is really settable. After MDEV-10709, any kind of expressions are allowed as EXECUTE IN parameters. But the patch for MDEV-10709 forgot to check that only descendants of Settable_routine_parameter should be allowed as OUT parameters. So an attempt to pass a non-settable parameter as an OUT parameter made server crash on the above mentioned DBUG_ASSERT. This patch changes Item_param::get_settable_routine_parameter(), which previously always returned "this". Now, when Item_param is bound to some Item, it caches if the bound Item is settable. Item_param::get_settable_routine_parameter() now returns "this" only if the bound actual parameter is settable, and returns NULL otherwise. diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 5b7d4b6..6c0750e 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -4747,3 +4747,26 @@ INSERT INTO t1 VALUES (1),(2),(3); EXECUTE IMMEDIATE 'EXPLAIN EXTENDED SELECT * FROM t1 WHERE ?+a<=>?+a' USING DEFAULT,DEFAULT; ERROR HY000: Default/ignore value is not supported for such parameter usage DROP TABLE t1; +# +# MDEV-11780 Crash with PREPARE + SP out parameter + literal +# +CREATE OR REPLACE PROCEDURE p1(OUT a INT) +BEGIN +SET a=10; +END; +$$ +PREPARE stmt FROM 'CALL p1(?)'; +EXECUTE stmt USING 10; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +EXECUTE stmt USING DEFAULT; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +EXECUTE stmt USING IGNORE; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +DEALLOCATE PREPARE stmt; +EXECUTE IMMEDIATE 'CALL p1(?)' USING 10; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +EXECUTE IMMEDIATE 'CALL p1(?)' USING DEFAULT; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +EXECUTE IMMEDIATE 'CALL p1(?)' USING IGNORE; +ERROR 42000: OUT or INOUT argument 1 for routine test.p1 is not a variable or NEW pseudo-variable in BEFORE trigger +DROP PROCEDURE p1; diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index 00e0c40..74ab765 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -4287,3 +4287,32 @@ INSERT INTO t1 VALUES (1),(2),(3); --error ER_INVALID_DEFAULT_PARAM EXECUTE IMMEDIATE 'EXPLAIN EXTENDED SELECT * FROM t1 WHERE ?+a<=>?+a' USING DEFAULT,DEFAULT; DROP TABLE t1; + +--echo # +--echo # MDEV-11780 Crash with PREPARE + SP out parameter + literal +--echo # + +DELIMITER $$; +CREATE OR REPLACE PROCEDURE p1(OUT a INT) +BEGIN + SET a=10; +END; +$$ +DELIMITER ;$$ + +PREPARE stmt FROM 'CALL p1(?)'; +--error ER_SP_NOT_VAR_ARG +EXECUTE stmt USING 10; +--error ER_SP_NOT_VAR_ARG +EXECUTE stmt USING DEFAULT; +--error ER_SP_NOT_VAR_ARG +EXECUTE stmt USING IGNORE; +DEALLOCATE PREPARE stmt; + +--error ER_SP_NOT_VAR_ARG +EXECUTE IMMEDIATE 'CALL p1(?)' USING 10; +--error ER_SP_NOT_VAR_ARG +EXECUTE IMMEDIATE 'CALL p1(?)' USING DEFAULT; +--error ER_SP_NOT_VAR_ARG +EXECUTE IMMEDIATE 'CALL p1(?)' USING IGNORE; +DROP PROCEDURE p1; diff --git a/sql/item.cc b/sql/item.cc index 8698b0f..b060074 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3313,7 +3313,14 @@ Item_param::Item_param(THD *thd, uint pos_in_query_arg): /* Don't pretend to be a literal unless value for this item is set. */ item_type(PARAM_ITEM), set_param_func(default_set_param_func), - m_out_param_info(NULL) + m_out_param_info(NULL), + /* + Set m_is_settable to "true". This is needed for client-server protocol, + whose parameters are always settable. + For dynamic SQL, settability depends on the type of Item passed + as an actual parameter. See Item_param::set_from_item(). + */ + m_is_settable_routine_parameter(true) { name= (char*) "?"; /* @@ -3535,6 +3542,7 @@ bool Item_param::CONVERSION_INFO::convert(THD *thd, String *str) bool Item_param::set_from_item(THD *thd, Item *item) { DBUG_ENTER("Item_param::set_from_item"); + m_is_settable_routine_parameter= item->get_settable_routine_parameter(); if (limit_clause_param) { longlong val= item->val_int(); @@ -4122,6 +4130,7 @@ Item_param::set_param_type_and_swap_value(Item_param *src) void Item_param::set_default() { + m_is_settable_routine_parameter= false; state= DEFAULT_VALUE; /* When Item_param is set to DEFAULT_VALUE: @@ -4136,6 +4145,7 @@ void Item_param::set_default() void Item_param::set_ignore() { + m_is_settable_routine_parameter= false; state= IGNORE_VALUE; null_value= true; } diff --git a/sql/item.h b/sql/item.h index 1f3e0f0..4dbaf8d 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2916,7 +2916,7 @@ class Item_param :public Item_basic_value, Rewritable_query_parameter *get_rewritable_query_parameter() { return this; } Settable_routine_parameter *get_settable_routine_parameter() - { return this; } + { return m_is_settable_routine_parameter ? this : NULL; } bool append_for_log(THD *thd, String *str); bool check_vcol_func_processor(void *int_arg) {return FALSE;} @@ -2936,6 +2936,7 @@ class Item_param :public Item_basic_value, private: Send_field *m_out_param_info; + bool m_is_settable_routine_parameter; };
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
