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@mariadb.org