Hi, Alexey!
On Mar 13, Alexey Botchkov wrote:
> revision-id: 398ea6ab012938d08126c8d0e0291000d06f6cd6
> (mariadb-10.2.13-43-g398ea6a)
> parent(s): 9d95b8665a3d87ab857e77e220effc64454eb881
> committer: Alexey Botchkov
> timestamp: 2018-03-13 17:13:27 +0400
> message:
>
> MDEV-11917 enum/set command-line options aren't respecting max-* settings.
>
> Code added to make maximum- working for Sys_var_enum
> and Sys_var_set.
>
> diff --git a/sql/set_var.cc b/sql/set_var.cc
> index e96e636..32e52a9 100644
> --- a/sql/set_var.cc
> +++ b/sql/set_var.cc
> @@ -452,6 +452,22 @@ void sys_var::do_deprecated_warning(THD *thd)
>
> @retval true on error, false otherwise (warning or ok)
> */
> +
> +
> +bool throw_bounds_warning(THD *thd, const char *name,const char *v)
> +{
> + if (thd->variables.sql_mode & MODE_STRICT_ALL_TABLES)
> + {
> + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, v);
This is rather illogical, setting a variable doesn't touch any tables,
so MODE_STRICT_ALL_TABLES should not affect it.
Also, the error message could be improved.
But it's how it was, and it's not subject of this bugfix, I agree.
> + return true;
> + }
> + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> + ER_TRUNCATED_WRONG_VALUE,
> + ER_THD(thd, ER_TRUNCATED_WRONG_VALUE), name, v);
> + return false;
> +}
> +
> +
> bool throw_bounds_warning(THD *thd, const char *name,
> bool fixed, bool is_unsigned, longlong v)
> {
> @@ -464,14 +480,7 @@ bool throw_bounds_warning(THD *thd, const char *name,
> else
> llstr(v, buf);
>
> - if (thd->variables.sql_mode & MODE_STRICT_ALL_TABLES)
> - {
> - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, buf);
> - return true;
> - }
> - push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> - ER_TRUNCATED_WRONG_VALUE,
> - ER_THD(thd, ER_TRUNCATED_WRONG_VALUE), name, buf);
ah, good. only one place to change when we fix the above.
thanks.
> + return throw_bounds_warning(thd, name, buf);
> }
> return false;
> }
> diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic
> index bc913b1..adcda5d 100644
> --- a/sql/sys_vars.ic
> +++ b/sql/sys_vars.ic
all ok, looks good. Just one comment. Please make max_var_ptr() logic to
be in one place only. If C++ cannot do it, then, in the worst case, make
it a macro, like
#define define_max_var_ptr(TYPE) \
TYPE *max_var_ptr() { return scope() == SESSION ?
(TYPE*)(((uchar*)&max_system_variables) + offset) : 0; }
after that ok to push, thanks!
Regards,
Sergei
Chief Architect MariaDB
and secur...@mariadb.org
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
