Date: Mon, 12 Oct 2020 15:12:36 +0200
From: Sergei Golubchik <serg@mariadb.org>
To: Alexander Barkov <bar@mariadb.com>
Cc: maria-developers@lists.launchpad.net
Subject: Re: d19f380d62c: MDEV-21445 Strange/inconsistent behavior of IN
 condition when mixing numbers and strings
Message-ID: <20201012131236.GA8414@pweza.fritz.box>
References: <XO4FM06Z00D.A.KDC.MTFhfB@pweza>

Hi, Alexander!

On Oct 12, Alexander Barkov wrote:
> revision-id: d19f380d62c (mariadb-10.3.21-213-gd19f380d62c)
> parent(s): 90c8d773ed0
> author: Alexander Barkov <bar@mariadb.com>
> committer: Alexander Barkov <bar@mariadb.com>
> timestamp: 2020-08-18 13:19:21 +0400
> message:
> 
> MDEV-21445 Strange/inconsistent behavior of IN condition when mixing numbers and strings
> 
> Comparison is now performed as DECIMAL for pairs (int,string) and (string,int).

Are there tests with strings like '1e100', outside of bigint range ?

> Before the patch the comparison was performed in DOUBLE format,
> so precision loss happened on huge values, e.g.:
> 
>   '97716021308405775' was converted to
>    97716021308405770 (notice, the last digit was lost)
> 
> diff --git a/sql/field.h b/sql/field.h
> index be3a648617b..c9f5a34a3ae 100644
> --- a/sql/field.h
> +++ b/sql/field.h
> @@ -311,7 +311,7 @@ class Value_source
>                                               const char *end)
>    {
>      Converter_str2my_decimal_with_warn(NULL, Warn_filter_all(),
> -                                       E_DEC_FATAL_ERROR & ~E_DEC_BAD_NUM,
> +                                       0,

why did you remove warnings (here and elsewhere)?

would be good to have it in the commit comment too, not just in email.

>                                         cs, cptr, end - cptr, decimal_value);
>      return decimal_value;
>    }
> diff --git a/sql/sql_type.cc b/sql/sql_type.cc
> index 9d3a47adfa5..7a847c90b8f 100644
> --- a/sql/sql_type.cc
> +++ b/sql/sql_type.cc
> @@ -764,6 +764,11 @@ Type_handler_hybrid_field_type::aggregate_for_comparison(const Type_handler *h)
>          m_type_handler= &type_handler_datetime;
>      }
>    }
> +  else if ((a == INT_RESULT && b == STRING_RESULT) ||
> +           (b == INT_RESULT && a == INT_RESULT))

looks like a typo, the second line should be

               b == INT_RESULT && a == STRING_RESULT

please add some tests to cover it.

> +  {
> +    m_type_handler= &type_handler_newdecimal;
> +  }
>    else if ((a == INT_RESULT || a == DECIMAL_RESULT) &&
>             (b == INT_RESULT || b == DECIMAL_RESULT))
>    {
Regards,
Sergei
VP of MariaDB Server Engineering
and security@mariadb.org