[Maria-developers] MDEV-10596
Hi Alexander, In sql_mode=oracle, when a stored procedure varchar parameter is defined without size, you have chosen to translate it by VARCHAR(4000) (or CHAR(2000) for CHAR parameter). Oracle does not work like this. Size is inherited from the size of argument at runtime. Example: CREATE OR REPLACE PROCEDURE p1(p OUT VARCHAR) AS BEGIN p:='0123456789'; END; / declare w varchar(10); begin p1(w); end; / --> work fine declare w varchar(8); begin p1(w); end; / --> failed with : ERROR at line 1: ORA-06502: PL/SQL: numeric or value error: character string buffer too small ORA-06512: at "OPS$DBO.P1", line 4 ORA-06512: at line 3 Furthermore, since Oracle 9, VARCHAR datatype in PL/SQL is not limited to 4000 char but to 32k. It's the size of varchar column in a table that is limited to 4000 (until Oracle 12C which allow 32k when MAX_STRING_SIZE=EXTENDED). I've done the attached patch to resize these parameters at runtime. What do you think about it ? Another question : Oracle does not allow to change the value of an IN parameter. The following procedure cannot be compile : CREATE OR REPLACE PROCEDURE p1(p IN VARCHAR) AS BEGIN p:='0123456789'; END; / Warning: Procedure created with compilation errors. //sun10:1521/CS> select * from user_errors; NAME TYPE SEQUENCE LINE POSITION ------------------------------ ------------ ---------- ---------- ---------- TEXT ------------------------------------------------------------------------------------------------------------------------------------ ATTRIBUTE MESSAGE_NUMBER --------- -------------- P1 PROCEDURE 1 4 3 PLS-00363: expression 'P' cannot be used as an assignment target ERROR 363 Mariadb allow this. It's fine for us but do you have plan to change this behavior ? (or add a new STRICT_xxx mode ?) Regard, Jérôme.
Hi Jerome, Thank you very much for your contribution! On 09/12/2017 04:42 PM, jerome brauge wrote:
Hi Alexander, In sql_mode=oracle, when a stored procedure varchar parameter is defined without size, you have chosen to translate it by VARCHAR(4000) (or CHAR(2000) for CHAR parameter). Oracle does not work like this. Size is inherited from the size of argument at runtime. <cut>
Your patch looks very good. I have minor improvement suggestions, to make the code more future-proof. Please find my comments below: <cut>
diff --git a/mysql-test/suite/compat/oracle/t/sp-param.test b/mysql-test/suite/compat/oracle/t/sp-param.test index 2320331..be929cb 100644 --- a/mysql-test/suite/compat/oracle/t/sp-param.test +++ b/mysql-test/suite/compat/oracle/t/sp-param.test <cut> +--error ER_DATA_TOO_LONG +declare str varchar(6000); + pout varchar(4000); +begin + str:=lpad('x',6000,'y'); + call p1(pout,str); + select length(pout); +end; +/ +drop procedure p1 +/ \ No newline at end of file
Please add a new line. <cut>
diff --git a/sql/sp_pcontext.cc b/sql/sp_pcontext.cc index d98f800..6531046 100644 --- a/sql/sp_pcontext.cc +++ b/sql/sp_pcontext.cc @@ -95,7 +95,8 @@ sp_pcontext::sp_pcontext() : Sql_alloc(), m_max_var_index(0), m_max_cursor_index(0), m_parent(NULL), m_pboundary(0), - m_scope(REGULAR_SCOPE) + m_scope(REGULAR_SCOPE), + m_dyn_varchar_param(false) { init(0, 0, 0); } diff --git a/sql/sp_pcontext.h b/sql/sp_pcontext.h index 215ebbe..776bd10 100644 --- a/sql/sp_pcontext.h +++ b/sql/sp_pcontext.h @@ -692,6 +692,12 @@ class sp_pcontext : public Sql_alloc return m_for_loop; }
+ void set_dyn_char_param() + { m_dyn_varchar_param= true; } + + bool use_dyn_char_param() const + { return m_dyn_varchar_param; } + private: /// Constructor for a tree node. /// @param prev the parent parsing context @@ -786,6 +792,8 @@ class sp_pcontext : public Sql_alloc
/// FOR LOOP characteristics Lex_for_loop m_for_loop; + + bool m_dyn_varchar_param; }; // class sp_pcontext : public Sql_alloc
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc index 8c598e8..3e449cd 100644 --- a/sql/sp_rcontext.cc +++ b/sql/sp_rcontext.cc @@ -63,7 +63,8 @@ sp_rcontext::~sp_rcontext() sp_rcontext *sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, - bool resolve_type_refs) + bool resolve_type_refs, + List<Item> *args) { sp_rcontext *ctx= new (thd->mem_root) sp_rcontext(root_parsing_ctx, return_value_fld, @@ -75,6 +76,34 @@ sp_rcontext *sp_rcontext::create(THD *thd, List<Spvar_definition> field_def_lst; ctx->m_root_parsing_ctx->retrieve_field_definitions(&field_def_lst);
+ if (ctx->m_root_parsing_ctx->use_dyn_char_param() && args) + { + List_iterator<Spvar_definition> it(field_def_lst); + List_iterator<Item> it_args(*args); + DBUG_ASSERT(field_def_lst.elements >= args->elements ); + Spvar_definition *def; + Item *arg; + uint arg_max_length; + /* + If length is not specified for varchar arguments, set length to the + maximum length of the real argument. Goals are: + -- avoid to allocate too much inused memory for m_var_table + -- allow length check inside the callee rather than during copy of + returned values in output variables. + -- allow varchar parameter size greater than 4000 + Default length has been stored in "decimal" member during parse. + */ + while ((def= it++) && (arg= it_args++)) + { + if (def->type_handler() == &type_handler_varchar && def->decimals) + { + arg_max_length= arg->max_char_length(); + def->length= arg_max_length > 0 ? arg_max_length : def->decimals; + def->create_length_to_internal_length_string(); + } + } + }
I'd like to make the code more symmetric across data types. It's very likely that we'll want some more formal-param-to-actual-param-adjustment in the future. Can you please move this code into a new virtual method in Type_handler? Something like this: virtual bool adjust_spparam_type(Spvar_definition *def, Item *from) const { return false; } and override it for VARCHAR: /* If length is not specified for a varchar parameter, set length to the maximum length of the actual argument. Goals are: - avoid to allocate too much unused memory for m_var_table - allow length check inside the callee rather than during copy of returned values in output variables. - allow varchar parameter size greater than 4000 Default length has been stored in "decimal" member during parse. */ bool Type_handler_varchar::adjust_spparam_type(Spvar_definition *def, Item *param) const { if (def->decimals) { arg_max_length= param->max_char_length(); def->length= arg_max_length > 0 ? arg_max_length : def->decimals; def->create_length_to_internal_length_string(); } return false; } Also, move the loop code into a new methods in sp_rcontext, e.g. like this: bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, List<Item> *args) { List_iterator<Spvar_definition> it(field_def_lst); List_iterator<Item> it_args(*args); DBUG_ASSERT(field_def_lst.elements >= args->elements ); Spvar_definition *def; Item *arg; uint arg_max_length; while ((def= it++) && (arg= it_args++)) { if (def->type_handler()->adjust_spparam_type(def, arg)) true; } return false; } and call it from sp_rcontext::create() as follows: if (adjust_formal_params_to_actual_params(thd, args)) return NULL. No needs to test for ctx->m_root_parsing_ctx->use_dyn_char_param(). Just do it always unconditionally. Please also remove: sp_rcontext::set_dyn_char_param() sp_rcontext::use_dyn_char_param() sp_rcontext::m_dyn_varchar_param; Btw, why did you add it? Is it to safe on populating List<Item> ? If so, then we can just pass arguments as follows; sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, bool resolve_type_refs) bool resolve_type_refs, Item **args, uint arg_count); and: bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, Item **args, uint arg_count); <cut>
diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index 97a55df..8e8a016 100644 --- a/sql/sql_yacc_ora.yy +++ b/sql/sql_yacc_ora.yy <cut> @@ -6565,17 +6567,20 @@ opt_field_length_default_1: the maximum possible length in characters in case of mbmaxlen=4 (e.g. utf32, utf16, utf8mb4). However, we'll have character sets with mbmaxlen=5 soon (e.g. gb18030). - - Let's translate VARCHAR to VARCHAR(4000), which covert all possible Oracle - values. */ opt_field_length_default_sp_param_varchar: - /* empty */ { $$= (char*) "4000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("4000", "4000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Please follow our coding style for *.yy. A multi-line code block corresponding to the "empty" rule should go under the comment "/* empty */", like this: opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); Lex->spcont->set_dyn_char_param(); } | field_length { $$.set($1, NULL); } But if we don't need set_dyn_char_param(), the above will be simplified to: opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); } | field_length { $$.set($1, NULL); } It's ok to put single-line code blocks to the right.
opt_field_length_default_sp_param_char: - /* empty */ { $$= (char*) "2000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("2000", "2000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Same here.
opt_precision: /* empty */ { $$.set(0, 0); }
Thanks!
Hi Alexander, I just need a confirmation on remove test on use_dyn_char_param(). This test avoid unnecessary loop on parameters at runtime in sql_mode=default or when all varchar parameter size are specified in sql_mode=oracle (I agree, it's not really Oracle compliant, but it's a useful feature). Do you really want I remove it ? Thank you very much.
-----Message d'origine----- De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mercredi 27 septembre 2017 09:38 À : jerome brauge Cc : MariaDB Developers (maria-developers@lists.launchpad.net) Objet : Re: MDEV-10596
Hi Jerome,
Thank you very much for your contribution!
On 09/12/2017 04:42 PM, jerome brauge wrote:
Hi Alexander, In sql_mode=oracle, when a stored procedure varchar parameter is defined without size, you have chosen to translate it by VARCHAR(4000) (or CHAR(2000) for CHAR parameter). Oracle does not work like this. Size is inherited from the size of argument at runtime. <cut>
Your patch looks very good. I have minor improvement suggestions, to make the code more future- proof.
Please find my comments below:
<cut>
diff --git a/mysql-test/suite/compat/oracle/t/sp-param.test b/mysql-test/suite/compat/oracle/t/sp-param.test index 2320331..be929cb 100644 --- a/mysql-test/suite/compat/oracle/t/sp-param.test +++ b/mysql-test/suite/compat/oracle/t/sp-param.test <cut> +--error ER_DATA_TOO_LONG +declare str varchar(6000); + pout varchar(4000); +begin + str:=lpad('x',6000,'y'); + call p1(pout,str); + select length(pout); +end; +/ +drop procedure p1 +/ \ No newline at end of file
Please add a new line.
<cut>
diff --git a/sql/sp_pcontext.cc b/sql/sp_pcontext.cc index d98f800..6531046 100644 --- a/sql/sp_pcontext.cc +++ b/sql/sp_pcontext.cc @@ -95,7 +95,8 @@ sp_pcontext::sp_pcontext() : Sql_alloc(), m_max_var_index(0), m_max_cursor_index(0), m_parent(NULL), m_pboundary(0), - m_scope(REGULAR_SCOPE) + m_scope(REGULAR_SCOPE), + m_dyn_varchar_param(false) { init(0, 0, 0); } diff --git a/sql/sp_pcontext.h b/sql/sp_pcontext.h index 215ebbe..776bd10 100644 --- a/sql/sp_pcontext.h +++ b/sql/sp_pcontext.h @@ -692,6 +692,12 @@ class sp_pcontext : public Sql_alloc return m_for_loop; }
+ void set_dyn_char_param() + { m_dyn_varchar_param= true; } + + bool use_dyn_char_param() const + { return m_dyn_varchar_param; } + private: /// Constructor for a tree node. /// @param prev the parent parsing context @@ -786,6 +792,8 @@ class sp_pcontext : public Sql_alloc
/// FOR LOOP characteristics Lex_for_loop m_for_loop; + + bool m_dyn_varchar_param; }; // class sp_pcontext : public Sql_alloc
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc index 8c598e8..3e449cd 100644 --- a/sql/sp_rcontext.cc +++ b/sql/sp_rcontext.cc @@ -63,7 +63,8 @@ sp_rcontext::~sp_rcontext() sp_rcontext *sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, - bool resolve_type_refs) + bool resolve_type_refs, + List<Item> *args) { sp_rcontext *ctx= new (thd->mem_root) sp_rcontext(root_parsing_ctx, return_value_fld, @@ -75,6 +76,34 @@ sp_rcontext *sp_rcontext::create(THD *thd, List<Spvar_definition> field_def_lst;
ctx->m_root_parsing_ctx->retrieve_field_definitions(&field_def_lst);
+ if (ctx->m_root_parsing_ctx->use_dyn_char_param() && args) { + List_iterator<Spvar_definition> it(field_def_lst); + List_iterator<Item> it_args(*args); + DBUG_ASSERT(field_def_lst.elements >= args->elements ); + Spvar_definition *def; + Item *arg; + uint arg_max_length; + /* + If length is not specified for varchar arguments, set length to the + maximum length of the real argument. Goals are: + -- avoid to allocate too much inused memory for m_var_table + -- allow length check inside the callee rather than during copy of + returned values in output variables. + -- allow varchar parameter size greater than 4000 + Default length has been stored in "decimal" member during parse. + */ + while ((def= it++) && (arg= it_args++)) + { + if (def->type_handler() == &type_handler_varchar && def->decimals) + { + arg_max_length= arg->max_char_length(); + def->length= arg_max_length > 0 ? arg_max_length : def->decimals; + def->create_length_to_internal_length_string(); + } + } + }
I'd like to make the code more symmetric across data types. It's very likely that we'll want some more formal-param-to-actual-param- adjustment in the future.
Can you please move this code into a new virtual method in Type_handler?
Something like this:
virtual bool adjust_spparam_type(Spvar_definition *def, Item *from) const { return false; }
and override it for VARCHAR:
/* If length is not specified for a varchar parameter, set length to the maximum length of the actual argument. Goals are: - avoid to allocate too much unused memory for m_var_table - allow length check inside the callee rather than during copy of returned values in output variables. - allow varchar parameter size greater than 4000 Default length has been stored in "decimal" member during parse. */ bool Type_handler_varchar::adjust_spparam_type(Spvar_definition *def, Item *param) const { if (def->decimals) { arg_max_length= param->max_char_length(); def->length= arg_max_length > 0 ? arg_max_length : def->decimals; def->create_length_to_internal_length_string(); } return false; }
Also, move the loop code into a new methods in sp_rcontext, e.g. like this:
bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, List<Item> *args) { List_iterator<Spvar_definition> it(field_def_lst); List_iterator<Item> it_args(*args); DBUG_ASSERT(field_def_lst.elements >= args->elements ); Spvar_definition *def; Item *arg; uint arg_max_length; while ((def= it++) && (arg= it_args++)) { if (def->type_handler()->adjust_spparam_type(def, arg)) true; } return false; }
and call it from sp_rcontext::create() as follows:
if (adjust_formal_params_to_actual_params(thd, args)) return NULL.
No needs to test for ctx->m_root_parsing_ctx->use_dyn_char_param(). Just do it always unconditionally.
Please also remove:
sp_rcontext::set_dyn_char_param() sp_rcontext::use_dyn_char_param() sp_rcontext::m_dyn_varchar_param;
Btw, why did you add it? Is it to safe on populating List<Item> ?
If so, then we can just pass arguments as follows;
sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, bool resolve_type_refs) bool resolve_type_refs, Item **args, uint arg_count);
and:
bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, Item **args, uint arg_count);
<cut>
diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index 97a55df..8e8a016 100644 --- a/sql/sql_yacc_ora.yy +++ b/sql/sql_yacc_ora.yy <cut> @@ -6565,17 +6567,20 @@ opt_field_length_default_1: the maximum possible length in characters in case of mbmaxlen=4 (e.g. utf32, utf16, utf8mb4). However, we'll have character sets with mbmaxlen=5 soon (e.g. gb18030). - - Let's translate VARCHAR to VARCHAR(4000), which covert all possible Oracle - values. */ opt_field_length_default_sp_param_varchar: - /* empty */ { $$= (char*) "4000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("4000", "4000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Please follow our coding style for *.yy. A multi-line code block corresponding to the "empty" rule should go under the comment "/* empty */", like this:
opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); Lex->spcont->set_dyn_char_param(); } | field_length { $$.set($1, NULL); }
But if we don't need set_dyn_char_param(), the above will be simplified to:
opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); } | field_length { $$.set($1, NULL); }
It's ok to put single-line code blocks to the right.
opt_field_length_default_sp_param_char: - /* empty */ { $$= (char*) "2000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("2000", "2000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Same here.
opt_precision: /* empty */ { $$.set(0, 0); }
Thanks!
Hi Jerome, On 09/27/2017 01:36 PM, jerome brauge wrote:
Hi Alexander, I just need a confirmation on remove test on use_dyn_char_param().
This test avoid unnecessary loop on parameters at runtime in sql_mode=default or when all varchar parameter size are specified in sql_mode=oracle (I agree, it's not really Oracle compliant, but it's a useful feature). Do you really want I remove it ?
Yes please. I know that for now only VARCHAR will do adjustment, and the loop will be useless if there are no any VARCHAR parameter with no length were given. But we'll have pluggable data types soon and new data type plugins will want to do parameter adjustments as well. So let's make the code ready for this right now. The amount of SP parameters is usually small. So the loop calling the new Type_handler virtual method for all parameters will be cheap comparing to the entire sp_rcontext::create(). Thanks!
Thank you very much.
-----Message d'origine----- De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mercredi 27 septembre 2017 09:38 À : jerome brauge Cc : MariaDB Developers (maria-developers@lists.launchpad.net) Objet : Re: MDEV-10596
Hi Jerome,
Thank you very much for your contribution!
On 09/12/2017 04:42 PM, jerome brauge wrote:
Hi Alexander, In sql_mode=oracle, when a stored procedure varchar parameter is defined without size, you have chosen to translate it by VARCHAR(4000) (or CHAR(2000) for CHAR parameter). Oracle does not work like this. Size is inherited from the size of argument at runtime. <cut>
Your patch looks very good. I have minor improvement suggestions, to make the code more future- proof.
Please find my comments below:
<cut>
diff --git a/mysql-test/suite/compat/oracle/t/sp-param.test b/mysql-test/suite/compat/oracle/t/sp-param.test index 2320331..be929cb 100644 --- a/mysql-test/suite/compat/oracle/t/sp-param.test +++ b/mysql-test/suite/compat/oracle/t/sp-param.test <cut> +--error ER_DATA_TOO_LONG +declare str varchar(6000); + pout varchar(4000); +begin + str:=lpad('x',6000,'y'); + call p1(pout,str); + select length(pout); +end; +/ +drop procedure p1 +/ \ No newline at end of file
Please add a new line.
<cut>
diff --git a/sql/sp_pcontext.cc b/sql/sp_pcontext.cc index d98f800..6531046 100644 --- a/sql/sp_pcontext.cc +++ b/sql/sp_pcontext.cc @@ -95,7 +95,8 @@ sp_pcontext::sp_pcontext() : Sql_alloc(), m_max_var_index(0), m_max_cursor_index(0), m_parent(NULL), m_pboundary(0), - m_scope(REGULAR_SCOPE) + m_scope(REGULAR_SCOPE), + m_dyn_varchar_param(false) { init(0, 0, 0); } diff --git a/sql/sp_pcontext.h b/sql/sp_pcontext.h index 215ebbe..776bd10 100644 --- a/sql/sp_pcontext.h +++ b/sql/sp_pcontext.h @@ -692,6 +692,12 @@ class sp_pcontext : public Sql_alloc return m_for_loop; }
+ void set_dyn_char_param() + { m_dyn_varchar_param= true; } + + bool use_dyn_char_param() const + { return m_dyn_varchar_param; } + private: /// Constructor for a tree node. /// @param prev the parent parsing context @@ -786,6 +792,8 @@ class sp_pcontext : public Sql_alloc
/// FOR LOOP characteristics Lex_for_loop m_for_loop; + + bool m_dyn_varchar_param; }; // class sp_pcontext : public Sql_alloc
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc index 8c598e8..3e449cd 100644 --- a/sql/sp_rcontext.cc +++ b/sql/sp_rcontext.cc @@ -63,7 +63,8 @@ sp_rcontext::~sp_rcontext() sp_rcontext *sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, - bool resolve_type_refs) + bool resolve_type_refs, + List<Item> *args) { sp_rcontext *ctx= new (thd->mem_root) sp_rcontext(root_parsing_ctx, return_value_fld, @@ -75,6 +76,34 @@ sp_rcontext *sp_rcontext::create(THD *thd, List<Spvar_definition> field_def_lst;
ctx->m_root_parsing_ctx->retrieve_field_definitions(&field_def_lst);
+ if (ctx->m_root_parsing_ctx->use_dyn_char_param() && args) { + List_iterator<Spvar_definition> it(field_def_lst); + List_iterator<Item> it_args(*args); + DBUG_ASSERT(field_def_lst.elements >= args->elements ); + Spvar_definition *def; + Item *arg; + uint arg_max_length; + /* + If length is not specified for varchar arguments, set length to the + maximum length of the real argument. Goals are: + -- avoid to allocate too much inused memory for m_var_table + -- allow length check inside the callee rather than during copy of + returned values in output variables. + -- allow varchar parameter size greater than 4000 + Default length has been stored in "decimal" member during parse. + */ + while ((def= it++) && (arg= it_args++)) + { + if (def->type_handler() == &type_handler_varchar && def->decimals) + { + arg_max_length= arg->max_char_length(); + def->length= arg_max_length > 0 ? arg_max_length : def->decimals; + def->create_length_to_internal_length_string(); + } + } + }
I'd like to make the code more symmetric across data types. It's very likely that we'll want some more formal-param-to-actual-param- adjustment in the future.
Can you please move this code into a new virtual method in Type_handler?
Something like this:
virtual bool adjust_spparam_type(Spvar_definition *def, Item *from) const { return false; }
and override it for VARCHAR:
/* If length is not specified for a varchar parameter, set length to the maximum length of the actual argument. Goals are: - avoid to allocate too much unused memory for m_var_table - allow length check inside the callee rather than during copy of returned values in output variables. - allow varchar parameter size greater than 4000 Default length has been stored in "decimal" member during parse. */ bool Type_handler_varchar::adjust_spparam_type(Spvar_definition *def, Item *param) const { if (def->decimals) { arg_max_length= param->max_char_length(); def->length= arg_max_length > 0 ? arg_max_length : def->decimals; def->create_length_to_internal_length_string(); } return false; }
Also, move the loop code into a new methods in sp_rcontext, e.g. like this:
bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, List<Item> *args) { List_iterator<Spvar_definition> it(field_def_lst); List_iterator<Item> it_args(*args); DBUG_ASSERT(field_def_lst.elements >= args->elements ); Spvar_definition *def; Item *arg; uint arg_max_length; while ((def= it++) && (arg= it_args++)) { if (def->type_handler()->adjust_spparam_type(def, arg)) true; } return false; }
and call it from sp_rcontext::create() as follows:
if (adjust_formal_params_to_actual_params(thd, args)) return NULL.
No needs to test for ctx->m_root_parsing_ctx->use_dyn_char_param(). Just do it always unconditionally.
Please also remove:
sp_rcontext::set_dyn_char_param() sp_rcontext::use_dyn_char_param() sp_rcontext::m_dyn_varchar_param;
Btw, why did you add it? Is it to safe on populating List<Item> ?
If so, then we can just pass arguments as follows;
sp_rcontext::create(THD *thd, const sp_pcontext *root_parsing_ctx, Field *return_value_fld, bool resolve_type_refs) bool resolve_type_refs, Item **args, uint arg_count);
and:
bool sp_rcontext::adjust_formal_params_to_actual_params(THD *thd, List<Spvar_definition> &field_def_lst, Item **args, uint arg_count);
<cut>
diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy index 97a55df..8e8a016 100644 --- a/sql/sql_yacc_ora.yy +++ b/sql/sql_yacc_ora.yy <cut> @@ -6565,17 +6567,20 @@ opt_field_length_default_1: the maximum possible length in characters in case of mbmaxlen=4 (e.g. utf32, utf16, utf8mb4). However, we'll have character sets with mbmaxlen=5 soon (e.g. gb18030). - - Let's translate VARCHAR to VARCHAR(4000), which covert all possible Oracle - values. */ opt_field_length_default_sp_param_varchar: - /* empty */ { $$= (char*) "4000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("4000", "4000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Please follow our coding style for *.yy. A multi-line code block corresponding to the "empty" rule should go under the comment "/* empty */", like this:
opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); Lex->spcont->set_dyn_char_param(); } | field_length { $$.set($1, NULL); }
But if we don't need set_dyn_char_param(), the above will be simplified to:
opt_field_length_default_sp_param_varchar: /* empty */ { $$.set("4000", "4000"); } | field_length { $$.set($1, NULL); }
It's ok to put single-line code blocks to the right.
opt_field_length_default_sp_param_char: - /* empty */ { $$= (char*) "2000"; } - | field_length { $$= $1; } + /* empty */ { + $$.set("2000", "2000"); + Lex->spcont->set_dyn_char_param(); + } + | field_length { $$.set($1, NULL); }
Same here.
opt_precision: /* empty */ { $$.set(0, 0); }
Thanks!
participants (2)
-
Alexander Barkov -
jerome brauge