Hi, Oleksandr, On Jan 06, Oleksandr Byelkin wrote:

revision-id: 92ff948d021 (mariadb-10.3.37-53-g92ff948d021) parent(s): 180b2bcd538 author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2022-12-05 20:42:03 +0100 message:

MDEV-29231 View returns wrong value with SQL_MODE 'NO_BACKSLASH_ESCAPES'

1. make view printing sql_mode independent 1.1. Print always current escape character 1.2. Allow escape character usage independent of sql_mode 2. Add workaround for parsing "like 3 in (0,1) escape 3" problem (alwaysuse parances in case of printing ESCAPE)

diff --git a/mysql-test/main/ctype_cp1250_ch.result b/mysql-test/main/ctype_cp1250_ch.result index b0aa4cff382..88175ae5013 100644 --- a/mysql-test/main/ctype_cp1250_ch.result +++ b/mysql-test/main/ctype_cp1250_ch.result @@ -129,7 +129,7 @@ EXPLAIN EXTENDED SELECT * FROM t1 WHERE CONCAT(c1)='a' AND CONCAT(c1) LIKE 'a '; id select_type table type possible_keys key key_len ref rows filtered Extra 1 SIMPLE t1 ALL NULL NULL NULL NULL 2 100.00 Using where Warnings: -Note 1003 select `test`.`t1`.`c1` AS `c1` from `test`.`t1` where concat(`test`.`t1`.`c1`) = 'a' and concat(`test`.`t1`.`c1`) like 'a ' +Note 1003 select `test`.`t1`.`c1` AS `c1` from `test`.`t1` where concat(`test`.`t1`.`c1`) = 'a' and concat(`test`.`t1`.`c1`) like ('a ') escape '\\'

I don't think there's a need to print the escape clause here, it only makes the output less readable.

diff --git a/mysql-test/main/default.result b/mysql-test/main/default.result index c9ac96367aa..5c72b894bf6 100644 --- a/mysql-test/main/default.result +++ b/mysql-test/main/default.result @@ -1936,7 +1936,7 @@ CREATE OR REPLACE TABLE t1 ( col INT DEFAULT ( 1 LIKE ( NOW() BETWEEN '2000-01-0 SHOW CREATE TABLE t1; Table Create Table t1 CREATE TABLE `t1` ( - `col` int(11) DEFAULT (1 like (current_timestamp() between '2000-01-01' and '2012-12-12')) + `col` int(11) DEFAULT (1 like (current_timestamp() between '2000-01-01' and '2012-12-12') escape '\\')

So, there're basically two approaches to a fix and we're inconsistently using both for different items. 1. one can always print expressions in the environment-independent way - create longer less readable text - not all items support it, new syntax might be needed (cannot emulate NO_BACKSLASH_ESCAPES with the ESCAPE clause) 2. one can set the environment (sql_mode) to the same hard-coded value before parsing views and vcols - one cannot dump the view/table definition in one sql_mode and load in another + this isn't normally a problem, as mysqldump also uses a fixed sql_mode - some item properties (what are they?) might be only set at fix_fields, fixing sql_mode during parsing won't help there Could we try to agree to one approach and use it consistently for all items from now on?

diff --git a/mysql-test/main/parser.result b/mysql-test/main/parser.result index 0bb4e82c8b8..79ec9f539eb 100644 --- a/mysql-test/main/parser.result +++ b/mysql-test/main/parser.result @@ -1325,11 +1325,11 @@ select 1 between 2 in (3,4) and 5 AS `1 between (2 in (3,4)) and 5` create or replace view v1 as select 1 between (2 like 3) and 4; Select view_definition from information_schema.views where table_schema='test' and table_name='v1'; view_definition -select 1 between 2 like 3 and 4 AS `1 between (2 like 3) and 4` +select 1 between 2 like (3) escape '\\' and 4 AS `1 between (2 like 3) and 4` create or replace view v1 as select 1 not between (2 like 3) and 4; Select view_definition from information_schema.views where table_schema='test' and table_name='v1'; view_definition -select 1 not between 2 like 3 and 4 AS `1 not between (2 like 3) and 4` +select 1 not between 2 like (3) escape '\\' and 4 AS `1 not between (2 like 3) and 4` drop view v1; # # Start of 10.2 tests diff --git a/mysql-test/main/precedence.result b/mysql-test/main/precedence.result index fc6579651b4..ce26ca2acf2 100644 --- a/mysql-test/main/precedence.result +++ b/mysql-test/main/precedence.result @@ -113,7 +113,7 @@ NOT 2 != 3 NOT (2 != 3) (NOT 2) != 3 create or replace view v1 as select NOT 2 LIKE 3, NOT (2 LIKE 3), (NOT 2) LIKE 3; Select view_definition from information_schema.views where table_schema='test' and table_name='v1'; view_definition -select 2 not like 3 AS `NOT 2 LIKE 3`,2 not like 3 AS `NOT (2 LIKE 3)`,!2 like 3 AS `(NOT 2) LIKE 3` +select 2 not like (3) escape '\\' AS `NOT 2 LIKE 3`,2 not like (3) escape '\\' AS `NOT (2 LIKE 3)`,!2 like (3) escape '\\' AS `(NOT 2) LIKE 3` select NOT 2 LIKE 3, NOT (2 LIKE 3), (NOT 2) LIKE 3 union select * from v1; NOT 2 LIKE 3 NOT (2 LIKE 3) (NOT 2) LIKE 3 1 1 0 diff --git a/mysql-test/main/precedence_bugs.result b/mysql-test/main/precedence_bugs.result index 723ab823b48..9654e3c31db 100644 --- a/mysql-test/main/precedence_bugs.result +++ b/mysql-test/main/precedence_bugs.result @@ -54,7 +54,7 @@ drop table t1; create view v1 as select 1 like (now() between '2000-01-01' and '2012-12-12' ); show create view v1; View v1 -Create View CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select 1 like (current_timestamp() between '2000-01-01' and '2012-12-12') AS `1 like (now() between '2000-01-01' and '2012-12-12' )` +Create View CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select 1 like (current_timestamp() between '2000-01-01' and '2012-12-12') escape '\\' AS `1 like (now() between '2000-01-01' and '2012-12-12' )` character_set_client latin1 collation_connection latin1_swedish_ci drop view v1;

if you'll always print ESCAPE clause, it'll make many tests in parser.test, precedence.test and precedence_bugs.test meaningless. Don't just update result files, please, remove tests that no longer make sense. Or rewrite them, when possible (but in precedence.test it's not).

diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index b7b0c981c2d..702689a7496 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -5262,10 +5262,15 @@ void Item_func_like::print(String *str, enum_query_type query_type) str->append(STRING_WITH_LEN(" not ")); str->append(func_name()); str->append(' '); - if (escape_used_in_parsing) + if (escape_used_in_parsing || (query_type | QT_VIEW_INTERNAL)) { - args[1]->print_parenthesised(str, query_type, precedence()); - str->append(STRING_WITH_LEN(" escape ")); + /* + Explicit parencies is workaround against parser bug which üprevent + patsing "like 3 in (0,1) escape 3" correctly + */ + str->append('('); + args[1]->print(str, query_type); + str->append(STRING_WITH_LEN(") escape "));

no, this is wrong. Don't just force parentheses there. Instead, fix the precedence, so that the parentheses would be printed automatically when needed.

escape_item->print_parenthesised(str, query_type, higher_precedence()); } else @@ -5390,10 +5395,7 @@ bool fix_escape_item(THD *thd, Item *escape_item, String *tmp_str, if (escape_str) { const char *escape_str_ptr= escape_str->ptr(); - if (escape_used_in_parsing && ( - (((thd->variables.sql_mode & MODE_NO_BACKSLASH_ESCAPES) && - escape_str->numchars() != 1) || - escape_str->numchars() > 1))) + if (escape_used_in_parsing && escape_str->numchars() > 1)

I still think that ESCAPE '' doesn't mean "no escape character". I tried and in my tests it didn't.

{ my_error(ER_WRONG_ARGUMENTS,MYF(0),"ESCAPE"); return TRUE;

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org