Hi, Alexander,
A couple of comments:
On Aug 03, Alexander Barkov wrote:
> revision-id: f3fc58149dd (mariadb-10.7.4-39-gf3fc58149dd)
> parent(s): 97d16c7544c
> author: Alexander Barkov
> committer: Alexander Barkov
> timestamp: 2022-08-03 11:28:31 +0400
> message:
>
> MDEV-29159 Patch for MDEV-28918 introduces more inconsistency than it solves,
> breaks usability
>
> Store assignment failures on incompatible data types now cause errors if:
> - STRICT_ALL_TABLES or STRICT_TRANS_TABLES sql_mode is used, and
> - IGNORE is not used
>
> Otherwise, only a warning is raised and the statement continues.
>
> TODO: add MTR tests for "loose" modes.
>
> diff --git a/sql/field.cc b/sql/field.cc
> index 249269a6b1d..e997e2cbd69 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -964,7 +964,7 @@ Type_handler::aggregate_for_result_traditional(const
> Type_handler *a,
> }
>
>
> -bool Field::check_assignability_from(const Type_handler *from) const
> +bool Field::check_assignability_from(const Type_handler *from, bool error)
> const
"error" is very confusing here, I had to read the whole patch to
understand what it means. Better names:
* only_warning
* ignore
* strict
* issue_error
> {
> /*
> Using type_handler_for_item_field() here to get the data type handler
> @@ -982,6 +982,15 @@ bool Field::check_assignability_from(const Type_handler
> *from) const
> type_handler_for_item_field());
> if (th.aggregate_for_result(from->type_handler_for_item_field()))
> {
> + if (!error)
> + {
> + THD *thd= get_thd();
> + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> + ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION,
> + ER_THD(thd,
> ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION),
> + type_handler()->name().ptr(), from->name().ptr(), "SET");
> + return false;
> + }
> my_error(ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION, MYF(0),
> type_handler()->name().ptr(), from->name().ptr(), "SET");
> return true;
if you wouldn't want to change the error message in a followup commit,
I'd suggested something like
my_error(ER_ILLEGAL_PARAMETER_DATA_TYPES2_FOR_OPERATION,
MYF(error ? 0 : ME_WARNING),
type_handler()->name().ptr(), from->name().ptr(), "SET");
return error;
Regards,
Sergei
VP of MariaDB Server Engineering
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
