maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha!

First, note that it's now a 10.9 task, so don't push it into 10.8,
please.

As for the patch - code-wise it's all good.
New mode names are confusing and should be clarified, see the comment
below.

On Jan 10, Rucha Deodhar wrote:
> revision-id: ce69e0c8e78 (mariadb-10.6.1-88-gce69e0c8e78)
> parent(s): 22556499397
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-10-06 13:25:29 +0530
> message:
> 
> MDEV-24920: Merge "old" SQL variable to "old_mode" sql variable
> 
> Analysis: There are 2 server variables- "old_mode" and "old". "old" is
> no longer needed as "old_mode" has replaced it (however still used in
> some places in the code). "old_mode" and "old" has same purpose-
> emulate behavior from previous MariaDB versions. So they can be merged
> to avoid confusion. Fix: Deprecate "old" variable and create another
> mode for @@old_mode to mimic behavior of previous "old" variable.
> Create specific modes for specifix task that --old sql variable was
> doing earlier and use the new modes instead.
> 
> diff --git a/mysql-test/suite/sys_vars/r/old_mode_basic.result b/mysql-test/suite/sys_vars/r/old_mode_basic.result
> index a6b95f1c60c..2f3742160eb 100644
> --- a/mysql-test/suite/sys_vars/r/old_mode_basic.result
> +++ b/mysql-test/suite/sys_vars/r/old_mode_basic.result
> @@ -264,3 +264,42 @@ SET @@collation_database = @save_collation_database;
>  #
>  # End of 10.6 test
>  #
> +#
> +# Beginning of 10.7 test
> +#

10.9 now

> diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
> index c2a219ee015..d1eaacd777c 100644
> --- a/sql/sys_vars.cc
> +++ b/sql/sys_vars.cc
> @@ -3755,6 +3774,8 @@ static const char *old_mode_names[]=
>    "NO_PROGRESS_INFO",
>    "ZERO_DATE_TIME_CAST",
>    "UTF8_IS_UTF8MB3",
> +  "INDEX_MASKING",
> +  "CHECKSUM_FORMATTING",

that's confusing. I don't understsand from the name of the option what
"CHECKSUM_FORMATTING" or "INDEX_MASKING" changes.

"NO_PROGRESS_INFO" is clear, there won't be progress info

"UTF8_IS_UTF8MB3" is clear

"ZERO_DATE_TIME_CAST" is kind of understandable too,
CAST(<time> AS DATETIME) will be "0000-00-00 <time>", that is zero date

CHECKSUM_FORMATTING and INDEX_MASKING is unclear.

For CHECKSUM_FORMATTING I'd suggest CHECKSUM_SLOW_NULLS
(see the commit that introduced this behavior: 496741d5761f)

"INDEX_MASKING" was introduced in the commit 79542930ea1c, see the
comment there, it explains what was done. This should help you to
suggest a better name for this mode.

>    0
>  };
>  
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx