Hi, Alexander,
I had only a couple of questions/comments, see below:
On Mar 11, Alexander Barkov wrote:
> revision-id: 2467eb2d314 (mariadb-10.6.1-330-g2467eb2d314)
> parent(s): 61b72ed3dde
> author: Alexander Barkov
> committer: Alexander Barkov
> timestamp: 2022-02-10 10:27:55 +0400
> message:
>
> MDEV-27743 Remove Lex::charset
>
> This patch also fixes:
> MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column
> definition
how do you plan to fix it in 10.2-10.8 ?
>
> diff --git a/mysql-test/main/ctype_collate_column.result
> b/mysql-test/main/ctype_collate_column.result
> new file mode 100644
> index 000..b20be5d3a48
> --- /dev/null
> +++ b/mysql-test/main/ctype_collate_column.result
> @@ -0,0 +1,450 @@
> +#
> +# Start of 10.9 tests
> +#
> +#
> +# MDEV-27743 Remove Lex::charset
1. Jira doesn't explain what user-visible changes one can expect.
If there are none, it's not clear why you needed this big test case
2. the test is totally unreadable. Could you do a normal test
instead? Like, put `select` instead of `execute immediate`
and convert the output to a .test file?
> +#
> +CREATE TABLE cs (cs VARCHAR(64) NOT NULL);
> +INSERT INTO cs (cs) VALUES
> +(''),
> +('CHARACTER SET latin1'),
> +('CHARACTER SET utf8mb4');
> +CREATE TABLE cl0 (cl0 VARCHAR(64) NOT NULL);
> +INSERT INTO cl0 (cl0) VALUES
> +(''),
> +('BINARY'),
> +('COLLATE DEFAULT'),
> +('COLLATE latin1_bin'),
> +('COLLATE latin1_swedish_ci'),
> +('COLLATE utf8mb4_bin'),
> +('COLLATE utf8mb4_general_ci');
> +CREATE TABLE cl1 (cl1 VARCHAR(64) NOT NULL);
> +INSERT INTO cl1 (cl1) VALUES
> +(''),
> +('COLLATE DEFAULT'),
> +('COLLATE latin1_bin'),
> +('COLLATE latin1_swedish_ci'),
> +('COLLATE utf8mb4_bin'),
> +('COLLATE utf8mb4_general_ci');
...
> diff --git a/mysql-test/main/ctype_latin1.result
> b/mysql-test/main/ctype_latin1.result
> index 0b16d1cf6f2..70577f2c22c 100644
> --- a/mysql-test/main/ctype_latin1.result
> +++ b/mysql-test/main/ctype_latin1.result
> @@ -8889,6 +8889,38 @@ a b
> 111 111
> DROP TABLE t1;
> #
> +# MDEV-27690 Crash on `CHARACTER SET csname COLLATE DEFAULT` in column
> definition
> +#
> +CREATE TABLE t1 (a CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT);
> +SHOW CREATE TABLE t1;
> +TableCreate Table
> +t1 CREATE TABLE `t1` (
> + `a` char(10) DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +DROP TABLE t1;
> +SELECT CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT);
> +CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT)
> +a
> +CREATE TABLE t1 AS
> +SELECT CAST('a' AS CHAR(10) CHARACTER SET latin1 COLLATE DEFAULT) AS c1;
> +SHOW CREATE TABLE t1;
> +TableCreate Table
> +t1 CREATE TABLE `t1` (
> + `c1` varchar(10) DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +DROP TABLE t1;
> +SELECT COLUMN_GET(COLUMN_CREATE(0, 'string'),0 AS CHAR CHARACTER SET latin1
> COLLATE DEFAULT) AS c1;
> +c1
> +string
> +CREATE TABLE t1 AS
> +SELECT COLUMN_GET(COLUMN_CREATE(0, 'string'),0 AS CHAR CHARACTER SET latin1
> COLLATE DEFAULT) AS c1;
> +SHOW CREATE TABLE t1;
> +TableCreate Table
> +t1 CREATE TABLE `t1` (
> + `c1` longtext DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +DROP TABLE t1;
1. why is it in "10.2 tests" ?
2. shouldn't here be at least one test that COLLATE DEFAULT is not a no-op?
Like CREATE TABLE (... COLLATE DEFAULT ...) COLLATE _non_default ?
Or is it in your generated ctype_collate_column.test?
> +#
> # End of 10.2 tests
> #
> #
> diff --git
> a/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test
> b/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test
> index 9cf0d7b4aff..dac6eab21e9 100644
> --- a/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test
> +++ b/mysql-test/suite/roles/create_and_drop_role_invalid_user_table.test
> @@ -36,7 +36,7 @@ set default role test_role for root@localhost;
> drop role test_role;
> drop user test_user@localhost;
>
> -alter table user add column default_role char(80) binary default '' not null
> +alter table user add column default_role char(80) default '' not null
> COLLATE utf8_general_ci
Heh, interesting that it worked before
good catch
> after is_role;
> alter table user add max_statement_time decimal(12,6) default 0 not null
> diff --git a/sql/structs.h b/sql/structs.h
> index 9a5006e8b47..db065c49931 100644
> --- a/sql/structs.h
> +++ b/sql/structs.h
> @@ -599,11 +599,140 @@ class DDL_options: public DDL_options_st
...
> +class Lex_collation: public Lex_collation_st
> +{
> +public:
> + Lex_collation(CHARSET_INFO *collation, Type type)
> + {
> +m_collation= collation;
> +m_type= type;
> + }
> + static Lex_collation national(bool bin_mod)
> + {
> +if (bin_mod)
> + return Lex_collation(&my_charset_utf8mb3_bin, TYPE_EXPLICIT);
> +return Lex_collation(&my_charset_utf8mb3_general_ci, TYPE_IMPLICIT);
utf8mb3? not utf8?
> + }