- developers - lists.mariadb.org

Re: [Maria-developers] 0bef50e50b5: MDEV-20501: Assertion `maybe_null || !null_value' failed in Item_func_round::date_op
by Sergei Golubchik 14 Jan '23

14 Jan '23

Hi, Sergei, On Jan 13, Sergei Petrunia wrote: > revision-id: 0bef50e50b5 (mariadb-10.4.27-33-g0bef50e50b5) > parent(s): 5db970fc760 > author: Sergei Petrunia > committer: Sergei Petrunia > timestamp: 2023-01-04 16:50:12 +0300 > message: > > MDEV-20501: Assertion `maybe_null || !null_value' failed in Item_func_round::date_op > > When the optimizer finds a constant (or system) table $TBL which is empty > or has no matching row, it would set table->null_row=true. This is done > even for tables with table->maybe_null==0. > > Then, it would proceed to perform query optimization phases (what for?) > which will attempt to evaluate Item expressions referring to $TBL. > > Eventually some Item expression will get the value of $TBL.not_null_field, > get SQL NULL and fail an assertion. > > Fixed by not performing any query optimization steps after we've got > constant/empty tables with no matching rows. > > Test result changes contain a lot of changes like > - ... Impossible WHERE noticed after reading const tables > + ... no matching row in const table > > as well as other changes caused by slightly-different processing of > the special case of empty constant tables. > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 6a441c5047b..57ffd58f8b4 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -2382,6 +2382,9 @@ int JOIN::optimize_stage2() > if (subq_exit_fl) > goto setup_subq_exit; > > + if (zero_result_cause) > + goto setup_subq_exit; In many other cases it's, like zero_result_cause= "No matching min/max row"; subq_exit_fl= true; that is, subq_exit_fl is set whenever zero_result_cause is set. Meaning, perhaps you should do the same in your new code below and then you wouldn't need those lines you've added above? > if (unlikely(thd->check_killed())) > DBUG_RETURN(1); > > @@ -5392,7 +5398,32 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list, > } > } > } while (ref_changed); > - > + > + /* > + ConstRowNotFoundShortcut-1: > + Some constant/system tables have mo matching rows. This means that > + the join operation output will be empty. > + Short-cut further optimization steps. > + Note that some query plan steps will still be performed to handle > + implicit grouping, join result setup, etc. > + See also: ConstRowNotFoundShortcut-{2,3}. > + > + Do not do this if we're optimizing for some UNION's fake_select_lex. > + We might be running UglySubqueryReoptimization (grep for name) and > + in this case constant tables are not reliable. > + */ > + if ((join->const_table_map & ~found_const_table_map) && > + !(join->select_lex->master_unit() && > + join->select_lex->master_unit()->fake_select_lex == join->select_lex)) why wouldn't you check for `describe` here? "reoptimization" hack is only needed for EXPLAIN. > + { > + join->zero_result_cause= "no matching row in const table"; > + join->table_count=0; > + join->const_tables= 0; > + join->join_tab= NULL; > + > + DBUG_RETURN(0); > + } > + > join->sort_by_table= get_sort_by_table(join->order, join->group_list, > join->select_lex->leaf_tables, > join->const_table_map); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 5dcde8f6523: MDEV-27653 long uniques don't work with unicode collations
by Sergei Golubchik 06 Jan '23

06 Jan '23

Hi, Alexander, Looks good. A couple of comments, see below On Jan 06, Alexander Barkov wrote: > revision-id: 5dcde8f6523 (mariadb-10.4.26-64-g5dcde8f6523) > parent(s): ce443c85547 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-10-28 15:37:44 +0400 > message: > > MDEV-27653 long uniques don't work with unicode collations > > diff --git a/mysql-test/main/ctype_utf8.test b/mysql-test/main/ctype_utf8.test > index cc61c2ae0fe..e2d4e4ab906 100644 > --- a/mysql-test/main/ctype_utf8.test > +++ b/mysql-test/main/ctype_utf8.test > @@ -2310,3 +2310,133 @@ VALUES (_latin1 0xDF) UNION VALUES(_utf8'a' COLLATE utf8_bin); > --echo # > --echo # End of 10.3 tests > --echo # > + > + > +--echo # > +--echo # Start of 10.4 tests > +--echo # > + > +--echo # > +--echo # MDEV-27653 long uniques don't work with unicode collations > +--echo # > + > +SET NAMES utf8mb3; > + > +# CHAR > + > +CREATE TABLE t1 ( > + a CHAR(30) COLLATE utf8mb3_general_ci, > + UNIQUE KEY(a) USING HASH > +); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > +CREATE TABLE t1 ( > + a CHAR(30) COLLATE utf8mb3_general_ci, > + UNIQUE KEY(a(10)) USING HASH > +); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > + > +# VARCHAR > + > +CREATE TABLE t1 ( > + a VARCHAR(30) COLLATE utf8mb3_general_ci, > + UNIQUE KEY(a) USING HASH > +); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > +CREATE TABLE t1 ( > + a VARCHAR(30) COLLATE utf8mb3_general_ci, > + UNIQUE KEY(a(10)) USING HASH > +); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > + > +# TEXT > + > +CREATE TABLE t1 (a TEXT COLLATE utf8mb3_general_ci UNIQUE); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > +CREATE TABLE t1 ( > + a LONGTEXT COLLATE utf8mb3_general_ci, > + UNIQUE KEY(a(10)) USING HASH > +); > +SHOW CREATE TABLE t1; > +INSERT INTO t1 VALUES ('a'); > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +SELECT * FROM t1; > +DROP TABLE t1; > + > + > +# Testing upgrade: > +# Prior to MDEV-27653, the UNIQUE HASH function errorneously > +# took into account string octet length. > +# Old tables should still open and work, but with a wrong results. > + > +copy_file std_data/mysql_upgrade/mdev27653_100422_text.frm $MYSQLD_DATADIR/test/t1.frm; > +copy_file std_data/mysql_upgrade/mdev27653_100422_text.MYD $MYSQLD_DATADIR/test/t1.MYD; > +copy_file std_data/mysql_upgrade/mdev27653_100422_text.MYI $MYSQLD_DATADIR/test/t1.MYI; > +SHOW CREATE TABLE t1; > +SELECT a, OCTET_LENGTH(a) FROM t1 ORDER BY BINARY a; > +CHECK TABLE t1; > + > +# There is already a one byte value 'a' in the table > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('A'); > + > +# There is already a two-byte value 'ä' in the table > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('Ä'); > + > +# There were no three-byte values in the table so far. > +# The below value violates UNIQUE, but it gets inserted. > +# This is wrong but expected for a pre-MDEV-27653 table. > +INSERT INTO t1 VALUES ('Ấ'); > +SELECT a, OCTET_LENGTH(a) FROM t1 ORDER BY BINARY a; > +CHECK TABLE t1; > + > +# ALTER FORCE fails: it tries to rebuild the table > +# with a correct UNIQUE HASH function, but there are duplicates! > +--error ER_DUP_ENTRY > +ALTER TABLE t1 FORCE; please, try ALTER IGNORE TABLE too > + > +# Let's remove all duplicate values, so only the one-byte 'a' stays. > +# ALTER..FORCE should work after that. > +DELETE FROM t1 WHERE OCTET_LENGTH(a)>1; > +ALTER TABLE t1 FORCE; > + > +# Make sure that 'a' and 'ä' cannot co-exists any more, > +# because the table was recreated with a correct UNIQUE HASH function. > +--error ER_DUP_ENTRY > +INSERT INTO t1 VALUES ('ä'); > +DROP TABLE t1; > + > +--echo # > +--echo # End of 10.4 tests > +--echo # > diff --git a/sql/table.cc b/sql/table.cc > index b9260853381..97f71284005 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -1267,7 +1286,11 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table, > list_item= new (mem_root) Item_field(thd, keypart->field); > field_list->push_back(list_item, mem_root); > } > - Item_func_hash *hash_item= new(mem_root)Item_func_hash(thd, *field_list); > + > + Item_func_hash *hash_item= make_unique_hash_func(thd, mem_root, > + table->s->mysql_version, > + field_list); would be good to fix CHECK ... FOR UPGRADE too. > + > Virtual_column_info *v= new (mem_root) Virtual_column_info(); > field->vcol_info= v; > field->vcol_info->expr= hash_item; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 92ff948d021: MDEV-29231 View returns wrong value with SQL_MODE 'NO_BACKSLASH_ESCAPES'
by Sergei Golubchik 06 Jan '23

06 Jan '23

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(a)mariadb.org

1 0

Re: [Maria-developers] 6ff49e48e4f: MDEV-26161: fix of view protocol
by Sergei Golubchik 06 Jan '23

06 Jan '23

Hi, Oleksandr, On Jan 06, Oleksandr Byelkin wrote: > revision-id: 6ff49e48e4f (mariadb-10.3.37-54-g6ff49e48e4f) > parent(s): 92ff948d021 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2022-12-06 10:22:06 +0100 > message: > > MDEV-26161: fix of view protocol could you please amend the commit comment to explain why the statement produces a different error in a view protocol? > diff --git a/mysql-test/main/gis.result b/mysql-test/main/gis.result > index 358be520b06..37613bc71a0 100644 > --- a/mysql-test/main/gis.result > +++ b/mysql-test/main/gis.result > @@ -4980,11 +4980,11 @@ ERROR HY000: Illegal parameter data type geometry for operation 'is_used_lock' > # MDEV-26161 crash in Gis_point::calculate_haversine > # > select st_distance_sphere(x'01030000000400000004000000000000', multipoint(point(124,204)), 10); > -ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field > +Got one of the listed errors > select st_distance_sphere(x'010300000004000000040000', multipoint(point(124,204)), 10); > -ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field > +Got one of the listed errors > select st_distance_sphere(x'010300000001000000040000', multipoint(point(124,204)), 10); > -ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field > +Got one of the listed errors > # > # End of 10.3 tests > # > diff --git a/mysql-test/main/gis.test b/mysql-test/main/gis.test > index 716fab9bfeb..07b207dcdc9 100644 > --- a/mysql-test/main/gis.test > +++ b/mysql-test/main/gis.test > @@ -3093,11 +3093,11 @@ SELECT IS_USED_LOCK(POINT(1,1)); > --echo # > --echo # MDEV-26161 crash in Gis_point::calculate_haversine > --echo # > ---error ER_CANT_CREATE_GEOMETRY_OBJECT > +--error ER_CANT_CREATE_GEOMETRY_OBJECT,ER_INTERNAL_ERROR > select st_distance_sphere(x'01030000000400000004000000000000', multipoint(point(124,204)), 10); > ---error ER_CANT_CREATE_GEOMETRY_OBJECT > +--error ER_CANT_CREATE_GEOMETRY_OBJECT,ER_INTERNAL_ERROR > select st_distance_sphere(x'010300000004000000040000', multipoint(point(124,204)), 10); > ---error ER_CANT_CREATE_GEOMETRY_OBJECT > +--error ER_CANT_CREATE_GEOMETRY_OBJECT,ER_INTERNAL_ERROR > select st_distance_sphere(x'010300000001000000040000', multipoint(point(124,204)), 10); > > --echo # > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-28602 Wrong result with outer join, merged derived table and view
by Sergey Petrunia 04 Jan '23

04 Jan '23

Hi Rex, Please find below input for the commit. (I've actually committed an alternative fix now, but before doing that I wrote all the trivial comments below so I'm sending these anyway in order to show what is expected of a patch. I'm also including the rationale for creating an alternative patch) > commit 75af3195482d3da277825663d1150c0dfc55420a > Author: Rex <rex.johnston(a)mariadb.com> > Date: Fri Dec 23 05:28:59 2022 +1200 > > MDEV-28602 Wrong result with outer join, merged derived table and view > > const item reference being mishandled in outer to inner join on > filling in null values. > First, fixVersion. It is currently set to 10.11, but affectsVersion starts from 10.3 I think the bug and the fix are applicable for all versions. Second: commit comment. This is not descriptive enough: > const item reference being mishandled in outer to inner join on > filling in null values. Something like this would be better: <commit-comment> A LEFT JOIN with a constant as a column of the inner table produced wrong query result if the optimizer had to write the inner table column into a temp table: SELECT ... FROM (SELECT /*non-mergeable select*/ FROM t1 LEFT JOIN (SELECT 'Y' as Val) t2 ON ...) Fixed this by: 1. Making Item_ref::save_in_field() call check_null_ref() to see if the referred table has a NULL-complemented row. 2. In order to do #1, moved null_ref_table() and check_null_ref() from Item_direct_view_ref to Item_ref. </commit-comment> Note: it starts with a siccint problem description, which is followed by a description of the fix. > diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc > index 620c52a3f40..65bedc3d337 100644 > --- a/sql/sql_join_cache.cc > +++ b/sql/sql_join_cache.cc > @@ -2622,7 +2622,7 @@ enum_nested_loop_state JOIN_CACHE::join_null_complements(bool skip_last) > get_record(); > /* The outer row is complemented by nulls for each inner table */ > restore_record(join_tab->table, s->default_values); > - mark_as_null_row(join_tab->table); > + mark_as_null_row(join_tab->table); In the future please make sure patches do not contain unrelated meaningless changes like this one. > rc= generate_full_extensions(get_curr_rec()); > if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS) > goto finish; > diff --git a/mysql-test/main/merge.test b/mysql-test/main/merge.test > index 0485f3ed1c3..05b05287e1b 100644 > --- a/mysql-test/main/merge.test > +++ b/mysql-test/main/merge.test Why is the patch in the merge.test ? merge.test at the top says clearly "Test of MERGE TABLES". That is, this is a test for tables with ENGINE=MERGE, a feature not related to this bug. The right test files for this bug would be main/join_outer.test and main/derived.test, I would prefer the first one. > @@ -2886,3 +2886,50 @@ drop table tm, t; > --echo # > --echo # End of 10.8 tests > --echo # > + > +--echo # > +--echo # MDEV-28602 Wrong result with outer join, merged derived table and view > +--echo # > + > +drop table if exists t1, t2; ^^^ This is not needed as there is a DROP TABLE right above > +create table t1 ( > + Election int(10) unsigned NOT NULL > +); > + > +insert into t1 (Election) values (1); > + +create table t2 ( + VoteID int(10), + ElectionID int(10), + UserID int(10) +); + +insert into t2 (ElectionID, UserID) values (2, 30), (3, 30); +# INSERT INTO t2 (ElectionID, UserID) VALUES (1, 30); ^^ No need to have commented-out lines in the commit. +drop view if exists v1; ^^^ same as above: this is not needed. +create view v1 as select * from t1 + left join ( select 'Y' AS Voted, ElectionID from t2 ) AS T + on T.ElectionID = t1.Election +limit 9; +# limit X causes merge algorithm select as opposed to temp table +select * from v1; ... > diff --git a/sql/item.h b/sql/item.h > index 2d598546b91..8684477b558 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -5531,18 +5531,42 @@ class Item_sp > > class Item_ref :public Item_ident > { > +public: > + > +#define NO_NULL_TABLE (reinterpret_cast<TABLE *>(0x1)) > + > + void set_null_ref_table() > + { > + null_ref_table= NO_NULL_TABLE; > + } > + > + bool check_null_ref() This works, but looks like a change that is 1. Too big 2. Looks like a partial implementation I'm looking at methods of Item_direct_view_ref: - save_val() - save_org_in_field() - save_in_result_field() - val_real() and other val_XXX() and they all follow the same pattern: if (check_null_ref()) produce SQL NULL; else Call Item_direct_ref::same_method(); This suggests we could add Item_direct_view_ref::save_in_field() and use the same approach. Conversely, if we move check_null_ref() to be in Item_ref, we will end up with: - check_null_ref() is in Item_ref - methods that make use of it are in Item_direct_view_ref() - methods of Item_ref() do not make check_null_ref() check, except for save_in_field() which does it. I think the former looks more logical than the latter. I've made the patch for it and I'll ask Sanja (the Item_ref/views expert) to check it. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] 6a8268b7893: MDEV-28915: mysql_upgrade fails due to old_mode="", with "Cannot load from mysql.proc. The table is probably corrupted"
by Sergei Golubchik 28 Dec '22

28 Dec '22

Hi, Rucha, On Dec 28, Rucha Deodhar wrote: > revision-id: 6a8268b7893 (mariadb-10.6.11-62-g6a8268b7893) > parent(s): 4ca5a0ec98d > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2022-12-15 17:45:25 +0530 > message: > > MDEV-28915: mysql_upgrade fails due to old_mode="", with "Cannot load from > mysql.proc. The table is probably corrupted" > > Analysis: When mysql_upgrade runs statements for upgrade, characterset is > converted to utf8mb4 because server starts with old_mode that interprets > utf8 to utf8mb4, but mysql.proc table has "utf8mb3" as hardcoded, so > it crashes with corrupted table. > > Fix: Changed Table_check_intact::check() definition to allow both > utf8mb3 and utf8mb4 by checking prefix and changing the upgrade scripts > to explicitly use utf8mb3 > > diff --git a/mysql-test/main/mysql_upgrade-28915.opt b/mysql-test/main/mysql_upgrade-28915.opt > new file mode 100644 > index 00000000000..0be5567a64d > --- /dev/null > +++ b/mysql-test/main/mysql_upgrade-28915.opt > @@ -0,0 +1 @@ > +--old-mode=NO_DUP_KEY_WARNINGS_WITH_IGNORE it's a bit confusing, try to use --old-mode= that is, empty old_mode, otherwise one starts thinking what this bug has to do with NO_DUP_KEY_WARNINGS_WITH_IGNORE. > diff --git a/mysql-test/main/mysql_upgrade-28915.test b/mysql-test/main/mysql_upgrade-28915.test > new file mode 100644 > index 00000000000..61b1fc263cb > --- /dev/null > +++ b/mysql-test/main/mysql_upgrade-28915.test > @@ -0,0 +1,78 @@ > +--source include/mysql_upgrade_preparation.inc > + > +let $MYSQLD_DATADIR= `select @@datadir`; > + > +--echo # > +--echo # Beginning of 10.6 test > +--echo # > +--echo # MDEV-28915: mysql_upgrade fails due to old_mode="", with "Cannot load > +--echo # from mysql.proc. The table is probably corrupted" > + > +--echo # Show that tables are created with utf8mb3 even without UTF8_IS_UTF8MB3 (see the .opt file) I don't think you've shown it. mtr does not recreate system tables for every test. You need to force it to re-bootstrap with one of the options that do it, for example, --innodb-page-size=16k (search mtr for "rebootstrap" for details). But even that won't help, because it'll only add "rebootstrap" options to the server's command line when bootstrapping. May be a combination of .cnf and .opt can do what you want, I didn't try. > + > +SHOW CREATE TABLE mysql.proc; > +SHOW CREATE TABLE mysql.event; > + > +--exec $MYSQL_UPGRADE --force 2>&1 > + > +--remove_file $MYSQLD_DATADIR/mysql_upgrade_info > + > +SHOW CREATE TABLE mysql.proc; > +SHOW CREATE TABLE mysql.event; > + > +--echo # Emulate that tables were created with utf8mb4 by an older version > + > +ALTER TABLE mysql.proc MODIFY db CHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL, > + MODIFY name CHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL, > + MODIFY specific_name CHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL, > + MODIFY definer VARCHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL, > + MODIFY comment TEXT CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL, > + MODIFY character_set_client CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL, > + MODIFY collation_connection CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL, > + MODIFY db_collation CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci DEFAULT NULL; > + > +ALTER TABLE mysql.event MODIFY db CHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '', > + MODIFY name CHAR(64) CHARACTER SET utf8mb4 NOT NULL DEFAULT '', > + MODIFY definer VARCHAR(384) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '', > + MODIFY comment CHAR(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL DEFAULT '', > + MODIFY character_set_client CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL, > + MODIFY collation_connection CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL, > + MODIFY db_collation CHAR(32) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin DEFAULT NULL; > + > +--echo # mysql_upgrade changes columns from utf8mb4 to utf8mb3 > + > +--exec $MYSQL_UPGRADE --force 2>&1 > +--remove_file $MYSQLD_DATADIR/mysql_upgrade_info > + > +--vertical_results > + > +SHOW CREATE TABLE mysql.proc; > +SHOW CREATE TABLE mysql.event; > + > +CREATE TABLE t1 (id1 INT, val1 VARCHAR(5)); > + > +DELIMITER |; > + > +CREATE PROCEDURE sp1 () > + BEGIN > + SELECT val1 FROM t1; > + END| You've made changes in sp.cc and event_db_repository.cc to ensure that these tables are usable even with utf8mb4 charset. But you don't test it if it works. You create a procedure and an event here, *after* tables were converted back to utf8mb3. you likely want to do it earlier, before mysql_upgrade, just after ALTER TABLE. > + > +DELIMITER ;| > + > +SELECT name, body_utf8, body FROM mysql.proc WHERE name like 'sp1'; > +CALL sp1(); > +SELECT name, body_utf8, body FROM mysql.proc WHERE name like 'sp1'; > + > +SET GLOBAL event_scheduler=ON; > + > +SELECT name, body_utf8, body FROM mysql.event; > +CREATE EVENT ev1 ON SCHEDULE EVERY 1 SECOND DO INSERT INTO t1 VALUES (1, 'abc'); > +SELECT name, body_utf8, body FROM mysql.event; > + > +SET GLOBAL event_scheduler=OFF; > +DROP EVENT ev1; > +DROP PROCEDURE sp1; > +DROP TABLE t1; > + > +--echo # end of 10.6 test Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 4ddf606debf: MDEV-30151 parse error 1=2 not between/in
by Sergei Golubchik 28 Dec '22

28 Dec '22

Hi, Alexander, I'm not sure I understand everything you did there, so questions below On Dec 28, Alexander Barkov wrote: > revision-id: 4ddf606debf (mariadb-10.3.37-59-g4ddf606debf) > parent(s): 8f30973234d > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2022-12-13 10:35:16 +0400 > message: > > MDEV-30151 parse error 1=2 not between/in > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 1f6485dac6a..ca614ff63e9 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -1687,7 +1687,8 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); > > %left PREC_BELOW_NOT > > -%nonassoc NOT_SYM > +/* The precendence of boolean NOT is in fact here. See the comment below. */ > + > %left '=' EQUAL_SYM GE '>' LE '<' NE > %nonassoc IS > %right BETWEEN_SYM > @@ -1699,6 +1700,24 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); > %left '*' '/' '%' DIV_SYM MOD_SYM > %left '^' > %left MYSQL_CONCAT_SYM > +/* > + Boolean negation has a special branch in "expr" starting with NOT_SYM. > + The precedence of logical negation is determined by the grammar itself > + (without using Bison terminal symbol precedence) in this order > + - Boolean factor (i.e. logical AND) > + - Boolean NOT > + - Boolean test (such as '=', IS NULL, IS TRUE) > + > + But we also need a precedence for NOT_SYM in other contexts, > + to shift (without reduce) in these cases: > + predicate <here> NOT IN ... > + predicate <here> NOT BETWEEN ... > + predicate <here> NOT LIKE ... > + predicate <here> NOT REGEXP ... > + If the precedence of NOT_SYM was low, it would reduce immediately > + after scanning "predicate" and then produce a syntax error on "NOT". > +*/ > +%nonassoc NOT_SYM > %nonassoc NEG '~' NOT2_SYM BINARY > %nonassoc COLLATE_SYM > > @@ -1938,6 +1957,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); > literal insert_ident order_ident temporal_literal > simple_ident expr sum_expr in_sum_expr > variable variable_aux > + boolean_test boolean_factor > predicate bit_expr parenthesized_expr > table_wild simple_expr column_default_non_parenthesized_expr udf_expr > primary_expr string_factor_expr mysql_concatenation_expr > @@ -9840,79 +9860,87 @@ expr: > MYSQL_YYABORT; > } > } > - | NOT_SYM expr %prec NOT_SYM > + | boolean_factor what does that achieve? > + ; > + > +boolean_factor: > + NOT_SYM boolean_factor > { > $$= negate_expression(thd, $2); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS TRUE_SYM %prec IS > + | boolean_test %prec PREC_BELOW_NOT > + ; > + > +boolean_test: > + boolean_test IS TRUE_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_istrue(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS not TRUE_SYM %prec IS > + | boolean_test IS not TRUE_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isnottrue(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS FALSE_SYM %prec IS > + | boolean_test IS FALSE_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isfalse(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS not FALSE_SYM %prec IS > + | boolean_test IS not FALSE_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isnotfalse(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS UNKNOWN_SYM %prec IS > + | boolean_test IS UNKNOWN_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isnull(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS not UNKNOWN_SYM %prec IS > + | boolean_test IS not UNKNOWN_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isnotnull(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS NULL_SYM %prec PREC_BELOW_NOT > + | boolean_test IS NULL_SYM %prec PREC_BELOW_NOT can you try %prec IS here? I suspect that if this affects anything, it'll cause bugs > { > $$= new (thd->mem_root) Item_func_isnull(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr IS not NULL_SYM %prec IS > + | boolean_test IS not NULL_SYM %prec IS > { > $$= new (thd->mem_root) Item_func_isnotnull(thd, $1); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr EQUAL_SYM predicate %prec EQUAL_SYM > + | boolean_test EQUAL_SYM predicate %prec EQUAL_SYM > { > $$= new (thd->mem_root) Item_func_equal(thd, $1, $3); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr comp_op predicate %prec '=' > + | boolean_test comp_op predicate %prec '=' > { > $$= (*$2)(0)->create(thd, $1, $3); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | expr comp_op all_or_any '(' subselect ')' %prec '=' > + | boolean_test comp_op all_or_any '(' subselect ')' %prec '=' > { > $$= all_any_subquery_creator(thd, $1, $2, $3, $5); > if (unlikely($$ == NULL)) > MYSQL_YYABORT; > } > - | predicate > + | predicate %prec BETWEEN_SYM > ; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 4d9f8a3c31e: MDEV-28669 addendum: additional tests and mtr changes
by Sergei Golubchik 27 Dec '22

27 Dec '22

Hi, Julius, On Dec 27, Julius Goryavsky wrote: > commit 4d9f8a3c31e > Author: Julius Goryavsky <julius.goryavsky(a)mariadb.com> > Date: Mon Dec 19 10:29:55 2022 +0100 > > diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl > index 12afab4d28c..8df8198fc99 100755 > --- a/mysql-test/mysql-test-run.pl > +++ b/mysql-test/mysql-test-run.pl > @@ -313,7 +313,7 @@ my %mysqld_logs; > my $opt_debug_sync_timeout= 300; # Default timeout for WAIT_FOR actions. > my $warn_seconds = 60; > > -my $rebootstrap_re= '--innodb[-_](?:page[-_]size|checksum[-_]algorithm|undo[-_]tablespaces|log[-_]group[-_]home[-_]dir|data[-_]home[-_]dir)|data[-_]file[-_]path|force_rebootstrap'; > +my $rebootstrap_re= '--(?:loose[-_])?(?:innodb[-_](?:page[-_]size|file(?:[-_]format|per[-_]table)|compression[-_](?:default|algorithm)|checksum(?:s|[-_]algorithm)|undo[-_](?:directory|tablespaces)|(?:data|log[-_]group)[-_]home[-_]dir|buffer[-_]pool[-_]filename|data[-_]file[-_]path|default[-_](?:encryption[-_]key[-_]id|page[-_]encryption)|sys[-_]|(?:index|table)[-_]stats)|aria[-_]log[-_](?:dir[-_]path|file[-_]size))|force_rebootstrap'; 1. drop force_rebootstrap please 2. this code was already not very readable, but got fairly ridiculous now. Let's change the approach. E.g. put them all in a hash, like my %rebootstrap_check = map { $_ => 1 } qw( innodb-page-size innodb-file-format ... ); you'll need to normalize an option before looking it up in the hash > > sub testcase_timeout ($) { return $opt_testcase_timeout * 60; } > sub check_timeout ($) { return testcase_timeout($_[0]); } > @@ -2762,7 +2762,29 @@ sub mysql_server_start($) { > return; > } > > - my $datadir= $mysqld->value('datadir'); > + my $extra_opts= get_extra_opts($mysqld, $tinfo); > + > + # The test options can contain the --datadir parameter, but > + # the bootstrap function can only read datadir location from > + # a .cnf file. So we need to parse the options to get the > + # actual location of the data directory, considering both > + # the options and the .cnf file, and then store the resulting > + # value in a "$mysqld" hash - to use later, when we need to > + # know the actual location of the data directory: > + my $datadir=""; > + foreach my $opt ( @$extra_opts ) > + { > + if ($opt =~ /^--datadir=/) { > + $datadir = substr($opt, 10); > + last; strictly speaking, `last` is incorrect, there can be more than one --datadir > + } > + } > + # If datadir is not in the options, then read it from .cnf: > + if (! $datadir) { > + $datadir = $mysqld->value('datadir'); > + } > + $mysqld->{'datadir'} = $datadir; > + > if (not $opt_start_dirty) > { > > @@ -2785,17 +2807,59 @@ sub mysql_server_start($) { > } > } > > + # Run <tname>-master.sh > + if ($mysqld->option('#!run-master-sh') and > + defined $tinfo->{master_sh} and > + run_system('/bin/sh ' . $tinfo->{master_sh}) ) > + { > + $tinfo->{'comment'}= "Failed to execute '$tinfo->{master_sh}'"; > + return 1; > + } > + > + # Run <tname>-slave.sh > + if ($mysqld->option('#!run-slave-sh') and > + defined $tinfo->{slave_sh} and > + run_system('/bin/sh ' . $tinfo->{slave_sh})) > + { > + $tinfo->{'comment'}= "Failed to execute '$tinfo->{slave_sh}'"; > + return 1; > + } > + > my $mysqld_basedir= $mysqld->value('basedir'); > - my $extra_opts= get_extra_opts($mysqld, $tinfo); > > if ( $basedir eq $mysqld_basedir ) > { > if (! $opt_start_dirty) # If dirty, keep possibly grown system db > { > + # Find the name of the current section in the configuration > + # file and its suffix: > + my $section = $mysqld->{name}; > + my $server_name; > + my $suffix = ""; > + if (index($section, '.') != -1) { > + ($server_name, $suffix) = $section =~ /^\s*([^\s.]+)\s*\.\s*([^\s]+)/; > + } > + else { > + $server_name = $section =~ /^\s*([^\s]+)/; > + } Replace all 11 lines above with: my ($suffix) = ($mysqld->{name} =~ /\.(.*)/); > # Some InnoDB options are incompatible with the default bootstrap. > # If they are used, re-bootstrap > my @rebootstrap_opts; > @rebootstrap_opts = grep {/$rebootstrap_re/o} @$extra_opts if $extra_opts; > + # Let's store the additional bootstrap options in > + # the environment variable - they may be used later > + # in the mtr tests - for re-bootstrap or run the > + # recovery, etc: > + my $extra_text = "--datadir=$datadir"; that's rather random. Why do you add only this one option, that doesn't make sense. Better handle any additional requirements in the test. > + if (@rebootstrap_opts) { why do you need an if() ? > + $extra_text .= ' '.join(' ', @rebootstrap_opts); > + } > + if ($suffix) { > + $ENV{'MTR_BOOTSTRAP_OPTS_'.$suffix} = $extra_text; > + } > + else { > + $ENV{'MTR_BOOTSTRAP_OPTS'} = $extra_text; Is that even possible? > + } > if (@rebootstrap_opts) > { > mtr_verbose("Re-bootstrap with @rebootstrap_opts"); > diff --git a/mysql-test/suite/galera/disabled.def b/mysql-test/suite/galera/disabled.def > index 84babda2fa0..18e7c074059 100644 > --- a/mysql-test/suite/galera/disabled.def > +++ b/mysql-test/suite/galera/disabled.def > @@ -28,3 +28,4 @@ query_cache: MDEV-15805 Test failure on galera.query_cache > versioning_trx_id: MDEV-18590: galera.versioning_trx_id: Test failure: mysqltest: Result content mismatch > galera_wsrep_provider_unset_set: wsrep_provider is read-only for security reasons > pxc-421: wsrep_provider is read-only for security reasons > +galera_sst_rsync_innodb_nest : MDEV-29591 nesting innodb subdirectories in datadir causes SST to fail Why do you disable it if the whole commit is about fixing the test? How can I see that the test works? > diff --git a/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff b/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff > new file mode 100644 > index 00000000000..027f4860a39 > --- /dev/null > +++ b/mysql-test/suite/galera/r/galera_sst_rsync_innodb__innodb_dir,debug.rdiff > @@ -0,0 +1,114 @@ looks like a wrong file name? > +--- galera_sst_rsync_innodb_nest.result > ++++ galera_sst_rsync_innodb_nest.reject > +@@ -286,3 +286,111 @@ > + DROP TABLE t1; > + COMMIT; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] f984f49fba7: MDEV-30203 - deb fix piuparts
by Sergei Golubchik 21 Dec '22

21 Dec '22

Hi, Daniel, This is a review of `git diff 12786f0e779f 5f76981eb649`, that is, for commits: 5f76981eb64 rpm: packages that dont match component name 25eaa0aebfa MDEV-30205: [Cmtr: suite look in mariadb-test path 0981201f027 mtr - wsrep use mariadb executables eba0b35035a mtr: maradb names for *embedded and mariadb-client-test 974ca88f0fe deb: fix autotest - exclude for MDEV-30205 b19d75408bb Deb: Breaks/Replaces/COnflicts c60f0341950 MDEV-30203 deb breaks/replaces on compat packages f984f49fba7 MDEV-30203 - deb fix piuparts 310d025ff0d MDEV-30205: fix deb autopkgtest 89f6cf47ee2 rpm: fix packaging of compat/non-compat scripts links 494ed76d15a deb: mysqlhotcopy to mariadb-client-compat d03075f3a1f deb: server-core mysql* -> server-compat 9453812193d wsrep_sst_mariabackup to use mariadb-backup (and message accordingly) a4f843a7a5a Fix uncollected symlink eb9a469bf9a Add package recommended dependencies c135be62274 Remove duplicate entry 84c7cd29c61 Fix Debian packaging for mariabackup 5dbc7ae8bb4 MDEV-30203: compat packages - RPM remove mariabackup link eb4fa5d750f MDEV-30203: debian - make compat packages d4be0b9be73 MDEV-30203: Create mysql symlink packages (RPM) - scripts 441879fd493 MDEV-30203: Create mysql symlink packages (RPM) On Dec 19, Daniel Black wrote: > diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake > index 339363b2169..7b48fd64f4c 100644 > --- a/cmake/cpack_rpm.cmake > +++ b/cmake/cpack_rpm.cmake > @@ -85,6 +85,35 @@ SET(CPACK_RPM_server_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}") > SET(CPACK_RPM_test_PACKAGE_SUMMARY "MariaDB database regression test suite") > SET(CPACK_RPM_test_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}") > > +# "set/append array" - append a set of strings, separated by a space > +MACRO(SETA var) > + FOREACH(v ${ARGN}) > + SET(${var} "${${var}} ${v}") > + ENDFOREACH() > +ENDMACRO(SETA) > + > +FOREACH(SYM_COMPONENT Server Client Test) > + SET(SYM ${SYM_COMPONENT}_symlinks) > + string(TOLOWER ${SYM} SYM) * why all cmake commands are uppercase, but `string` is lowercase? * SET is redundant, you can do STRING(TOLOWER ${SYM_COMPONENT}_symlinks SYM) * may be, better to use ${SYM_COMPONENT}-symlinks ? Like in MariaDB-server-symlinks ? > + SET(SYMCOMP ${SYM_COMPONENT}Symlinks) > + string(TOUPPER ${SYMCOMP} SYMCOMP_UPPER) > + SET(CPACK_COMPONENT_${SYMCOMP_UPPER}_GROUP "${SYM}") > + SET(CPACK_COMPONENTS_ALL "${CPACK_COMPONENTS_ALL}" "${SYMCOMP}") > + SET(CPACK_RPM_${SYM}_PACKAGE_SUMMARY "MySQL compatible symlinks for MariaDB database ${SYM_COMPONENT} binaries/scripts") > + SET(CPACK_RPM_${SYM}_PACKAGE_DESCRIPTION "${CPACK_RPM_PACKAGE_DESCRIPTION}") > + SET(CPACK_RPM_${SYM}_PACKAGE_ARCHITECTURE "noarch") > + SETA(CPACK_RPM_${SYM_COMPONENT}_PACKAGE_RECOMMENDS "MariaDB-${SYM}") You forgot to say that symlink package DEPENDS on the non-symlink package > +ENDFOREACH() > + > +# TODO change to 11.0 on rebase > +SETA(CPACK_RPM_client_symlinks_PACKAGE_CONFLICTS > + "MariaDB-client < 10.11.2" > + "MariaDB-server < 10.11.2") > +SETA(CPACK_RPM_server_symlinks_PACKAGE_CONFLICTS > + "MariaDB-server < 10.11.2") > +SETA(CPACK_RPM_test_symlinks_PACKAGE_CONFLICTS > + "MariaDB-test < 10.11.2") > + > # libmariadb3 > SET(CPACK_RPM_shared_PACKAGE_SUMMARY "LGPL MariaDB database client library") > SET(CPACK_RPM_shared_PACKAGE_DESCRIPTION "This is LGPL MariaDB client library that can be used to connect to MySQL > diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > index 3f3b4e4a9b5..a4eb6c2b42c 100644 > --- a/cmake/symlinks.cmake > +++ b/cmake/symlinks.cmake > @@ -12,7 +12,6 @@ endmacro() > REGISTER_SYMLINK("mariadb" "mysql") > REGISTER_SYMLINK("mariadb-access" "mysqlaccess") > REGISTER_SYMLINK("mariadb-admin" "mysqladmin") > -REGISTER_SYMLINK("mariadb-backup" "mariabackup") better not. there're scripts that might be using it. This is why I suggested `strncmp(my_progname, "mariadb", 7)` and not `strncmp(my_progname, "maria", 5)` in your PR > REGISTER_SYMLINK("mariadb-binlog" "mysqlbinlog") > REGISTER_SYMLINK("mariadb-check" "mysqlcheck") > REGISTER_SYMLINK("mariadb-client-test-embedded" "mysql_client_test_embedded") > diff --git a/debian/control b/debian/control > index 81418c13656..6b225ca4b3d 100644 > --- a/debian/control > +++ b/debian/control > @@ -548,7 +548,8 @@ Provides: default-mysql-client, > virtual-mysql-client > Recommends: libdbd-mariadb-perl | libdbd-mysql-perl, > libdbi-perl, > - libterm-readkey-perl > + libterm-readkey-perl, > + mariadb-client-compat Please, no. deb and rpm packages are different enough already. don't use different suffixes for them. Choose either -symlink or -compat (or something else) and use it consistently both in deb and in rpm. > Description: MariaDB database client binaries > MariaDB is a fast, stable and true multi-user, multi-threaded SQL database > server. SQL (Structured Query Language) is the most popular database query > @@ -558,6 +559,110 @@ Description: MariaDB database client binaries > This package includes the client binaries and the additional tools > innotop and mariadb-report (mysqlreport). > > +Package: mariadb-client-compat > +Architecture: all > +Depends: mariadb-client > +Multi-Arch: foreign > +Description: MySQL compatibility links to mariadb-client binaries/scripts. > +Conflicts: mariadb-client (<< ${source:Version}), > + mariadb-client-10.0, I wonder, would it work, if you won't set any Conflicts/Breaks, but only set Depends? It'd be a lot simpler that way (if it'll work). > + mariadb-client-10.1, > + mariadb-client-10.2, > + mariadb-client-10.3, > + mariadb-client-10.4, > + mariadb-client-10.5, > + mariadb-client-10.6, > + mariadb-client-10.7, > + mariadb-client-10.8, > + mariadb-client-5.1, > + mariadb-client-5.2, > + mariadb-client-5.3, > + mariadb-client-5.5, > + mariadb-client-core (<< ${source:Version}), > + mariadb-client-core-10.0, > + mariadb-client-core-10.1, > + mariadb-client-core-10.2, > + mariadb-client-core-10.3, > + mariadb-client-core-10.4, > + mariadb-client-core-10.5, > + mariadb-client-core-10.6, > + mariadb-client-core-10.7, > + mariadb-client-core-10.8, > + mariadb-server (<< ${source:Version}), > + mariadb-server-10.0, > + mariadb-server-10.1, > + mariadb-server-10.2, > + mariadb-server-10.3, > + mariadb-server-10.4, > + mariadb-server-10.5, > + mariadb-server-10.6, > + mariadb-server-10.7, > + mariadb-server-10.8, > + mysql-client (<< 5.0.51), > + mysql-client-5.0, > + mysql-client-5.1, > + mysql-client-5.5, > + mysql-client-5.6, > + mysql-client-5.7, > + mysql-client-8.0, > + mysql-client-core-5.0, > + mysql-client-core-5.1, > + mysql-client-core-5.5, > + mysql-client-core-5.6, > + mysql-client-core-5.7, > + mysql-client-core-8.0, > + mysql-server-core-5.7, > + mysql-server-core-8.0, > + percona-server-server-5.6, > + percona-server-server, > + percona-xtradb-cluster-server-5.6, > + percona-xtradb-cluster-server-5.7, > + percona-xtradb-cluster-server > +Breaks: mariadb-client (<< ${source:Version}), > + mariadb-client-10.0, > + mariadb-client-10.1, > + mariadb-client-10.2, > + mariadb-client-10.3, > + mariadb-client-10.4, > + mariadb-client-10.5, > + mariadb-client-10.6, > + mariadb-client-10.7, > + mariadb-client-10.8, > + mariadb-client-5.1, > + mariadb-client-core (<< ${source:Version}), > + mariadb-client-core-10.0, > + mariadb-client-core-10.1, > + mariadb-client-core-10.2, > + mariadb-client-core-10.3, > + mariadb-client-core-10.4, > + mariadb-client-core-10.5, > + mariadb-client-core-10.6, > + mariadb-client-core-10.7, > + mariadb-client-core-10.8, > + mariadb-server (<< ${source:Version}), > + mariadb-server-10.0, > + mariadb-server-10.1, > + mariadb-server-10.2, > + mariadb-server-10.3, > + mariadb-server-10.4, > + mariadb-server-10.5, > + mariadb-server-10.6, > + mariadb-server-10.7, > + mariadb-server-10.8, > + mysql-server-5.5, > + mysql-server-5.6, > + mysql-server-5.7, > + mysql-server-8.0, > + mysql-server-core-5.5, > + mysql-server-core-5.6, > + mysql-server-core-5.7, > + mysql-server-core-8.0, > + percona-server-server-5.6, > + percona-server-server, > + percona-xtradb-cluster-server-5.6, > + percona-xtradb-cluster-server-5.7, > + percona-xtradb-cluster-server > + > Package: mariadb-server-core > Architecture: any > Depends: mariadb-common (>= ${source:Version}), > diff --git a/debian/mariadb-backup.install b/debian/mariadb-backup.install > index e450f8f46a0..a288377ddd0 100644 > --- a/debian/mariadb-backup.install > +++ b/debian/mariadb-backup.install > @@ -1,6 +1,4 @@ > -usr/bin/mariabackup same here. needs mariadb-backup-compat > usr/bin/mariadb-backup > usr/bin/mbstream > -usr/share/man/man1/mariabackup.1 > usr/share/man/man1/mariadb-backup.1 > usr/share/man/man1/mbstream.1 > diff --git a/debian/mariadb-client.links b/debian/mariadb-client.links > index 62e3651daf5..8ccce12c609 100644 > --- a/debian/mariadb-client.links > +++ b/debian/mariadb-client.links > @@ -2,11 +2,6 @@ usr/bin/mariadb-check usr/bin/mariadb-analyze > usr/bin/mariadb-check usr/bin/mariadb-optimize > usr/bin/mariadb-check usr/bin/mariadb-repair > usr/bin/mariadb-check usr/bin/mariadbcheck > -usr/bin/mariadb-check usr/bin/mysqlanalyze > -usr/bin/mariadb-check usr/bin/mysqlcheck > -usr/bin/mariadb-check usr/bin/mysqloptimize > -usr/bin/mariadb-check usr/bin/mysqlrepair > -usr/bin/mariadb-report usr/bin/mysqlreport > usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-analyze.1.gz > usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-optimize.1.gz > usr/share/man/man1/mariadb-check.1.gz usr/share/man/man1/mariadb-repair.1.gz you forgot to move symlinks to man pages > diff --git a/debian/mariadb-server-compat.install b/debian/mariadb-server-compat.install > new file mode 100644 > index 00000000000..36701d8e0c6 > --- /dev/null > +++ b/debian/mariadb-server-compat.install > @@ -0,0 +1,12 @@ > +usr/bin/mysqld_multi > +usr/bin/mysqld_safe > +usr/bin/mysqld_safe_helper > +usr/bin/mysql_install_db > +usr/bin/mysql_upgrade > +usr/sbin/mysqld > +usr/share/man/man1/mysqld_multi.1 > +usr/share/man/man1/mysqld_safe.1 > +usr/share/man/man1/mysqld_safe_helper.1 great. do you do the same for rpms? I didn't see where manpages are moved to -symlinks packages in rpms. > +usr/share/man/man1/mysql_install_db.1 > +usr/share/man/man1/mysql_upgrade.1 > +usr/share/man/man8/mysqld.8 > diff --git a/debian/mariadb-test-compat.install b/debian/mariadb-test-compat.install > new file mode 100644 > index 00000000000..7b498d7d1c8 > --- /dev/null > +++ b/debian/mariadb-test-compat.install > @@ -0,0 +1 @@ > +usr/share/mysql/mysql-test/mysql-test-run.pl You forgot mysql-test-run, mysqltest, mysql_client_test, and their *embedded variants. But, you know, it's probably not worth a new package at all. It's the *test* package (not interesting for users, never used in production, so no immutability requirements). I suggest to keep those symlinks in the mariadb-test package. Your other PR will add a warning. And eventually we drop them. Optional/Recommended/Suggested - sounds like too much toubles for a *test* package. > diff --git a/mysql-test/lib/mtr_cases.pm b/mysql-test/lib/mtr_cases.pm > index fe202279ac7..ad1ac6fdf88 100644 > --- a/mysql-test/lib/mtr_cases.pm > +++ b/mysql-test/lib/mtr_cases.pm > @@ -387,14 +387,14 @@ sub collect_suite_name($$) > else > { > my @dirs = my_find_dir(dirname($::glob_mysql_test_dir), > - ["mysql-test/suite", @plugin_suitedirs ], > + ["mariadb-test/suite", "mysql-test/suite", @plugin_suitedirs ], Eh, no. That is, yes, but not in this set of commits, not in this MDEV > $suitename); > # > # if $suitename contained wildcards, we'll have many suites and > # their overlays here. Let's group them appropriately. > # > for (@dirs) { > - m@^.*/(?:mysql-test/suite|$plugin_suitedir_regex)/(.*)$@o or confess $_; > + m@^.*/(?:mariadb-test/suite|mysql-test/suite|$plugin_suitedir_regex)/(.*)$@o or confess $_; > push @{$suites{$1}}, $_; > } > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] MDEV-14443 test cases only
by Sergei Golubchik 19 Dec '22

19 Dec '22

Hi, Vicențiu, This is a review of _test cases only_. Mainly, to discuss the behavior. Random thoughts here and comments inline in the diff. There are two models for DENY that I can think of: "a hole in grants" and "a mask". The difference lies in where DENY is applied. In the "a hole in grants" model one has a certain set of grants (may be with "holes"), a role has another set of grants, and when one does SET ROLE, one gets a union of those grants. Like, one has SELECT on db1.* and DENY to db1.t1, and some role has SELECT on db1.t1 - by setting this role a user will have full SELECT on db1.* While in the "mask" model, denies are separate entities, when setting a role a union of denies is subtracted from the union of grants. In the example above a user would still be denied access to db1.t1. I personally see the "hole" model as more logical and easier to understand. It also allows to "patch" holes with roles, what the "mask" model cannot do. The "mask" model needs getting used to, the fact that after SET ROLE one can end up having less access than before. It's not clear who can DENY what and to whom. And DENY is very difficult to work around, esp. as one can DENY IGNORE DENIES. But it allows to DENY something to PUBLIC, and the "hole" model cannot really do that. And I think it'd be a popular use case. Ideally, I'd prefer to have some kind of a compromise model that's as simple to understand as a "hole", but allows to deny to public. Unfortunately, I don't see how to do that. You've implemented the "mask" behavior. Okay. It solves an important use case. More comments below: > diff --git a/mysql-test/suite/deny/columns.result b/mysql-test/suite/deny/columns.result > --- /dev/null > +++ b/mysql-test/suite/deny/columns.result > @@ -0,0 +1,126 @@ > +create user foo; > +create database deny_db; > +create table deny_db.t1 (a int, b int, secret int); > +create table deny_db.t2 (a2 int, b2 int, secret2 int); > +insert into deny_db.t2 values (100, 200, 300); > +grant all on *.* to foo; > +revoke ignore denies on *.* from foo; > +grant all on deny_db.* to foo; > +grant all on deny_db.t1 to foo; > +grant all on deny_db.t2 to foo; > +grant select (a, b, secret) on deny_db.t1 to foo; > +grant insert (a, b, secret) on deny_db.t1 to foo; > +grant update (a, b, secret) on deny_db.t1 to foo; > +grant references (a, b, secret) on deny_db.t1 to foo; > +grant select (a2, b2, secret2) on deny_db.t2 to foo; > +grant insert (a2, b2, secret2) on deny_db.t2 to foo; > +grant update (a2, b2, secret2) on deny_db.t2 to foo; > +grant references (a2, b2, secret2) on deny_db.t2 to foo; this is quite confusing, tons of redundant grants on different levels, it makes it very unclear of what causes what. > +# > +# Test select and insert ... select commands. > +# > +deny select (secret) on deny_db.t1 to foo; > +connect con1,localhost,foo,,; > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' case in point. Normally, this should've been SELECT command denied to user 'foo'@'localhost' for table 't1' like in create database db; create table db.t1 (a int, b int, secret int); insert db.t1 values (1,2,100); create user foo@localhost; grant select (a,b) on db.t1 to foo@localhost; connect foo,localhost,foo; select a,b from db.t1; --error ER_TABLEACCESS_DENIED_ERROR select * from db.t1; --error ER_COLUMNACCESS_DENIED_ERROR select a,b,secret from db.t1; that is `select *` is ER_TABLEACCESS_DENIED_ERROR, and if I mention the column by name, then it's ER_COLUMNACCESS_DENIED_ERROR But you have so many redundant overlapping grants, that I really cannot see what error is correct in your test here. > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +# > +# These are different code paths. Check both. > +# > +insert into deny_db.t1 (a, b, secret) values (1, 2, 3); > +insert into deny_db.t1 values (1, 2, 3); > +# > +# Check INSERT SELECT as we need to account for 2 different kinds of > +# privileges. > +# > +insert into deny_db.t1 select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +insert into deny_db.t1 select * from deny_db.t2; > +show full columns from deny_db.t1; > +Field Type Collation Null Key Default Extra Privileges Comment > +a int(11) NULL YES NULL select,insert,update,references > +b int(11) NULL YES NULL select,insert,update,references > +secret int(11) NULL YES NULL insert,update,references do you have a test where a denied column isn't shown by SHOW COLUMNS? that is, where you don't have other privileges on it? > +show full columns from deny_db.t2; > +Field Type Collation Null Key Default Extra Privileges Comment > +a2 int(11) NULL YES NULL select,insert,update,references > +b2 int(11) NULL YES NULL select,insert,update,references > +secret2 int(11) NULL YES NULL select,insert,update,references > +disconnect con1; > +connection default; > +revoke deny select (secret) on deny_db.t1 from foo; > +deny select(secret2) on deny_db.t2 to foo; > +connect con1,localhost,foo,,; > +insert into deny_db.t1 (a, b, secret) values (1, 2, 3); > +insert into deny_db.t1 values (1, 2, 3); > +insert into deny_db.t1 select * from deny_db.t1; > +insert into deny_db.t1 select * from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret2' in table 't2' > +show full columns from deny_db.t1; > +Field Type Collation Null Key Default Extra Privileges Comment > +a int(11) NULL YES NULL select,insert,update,references > +b int(11) NULL YES NULL select,insert,update,references > +secret int(11) NULL YES NULL select,insert,update,references > +show full columns from deny_db.t2; > +Field Type Collation Null Key Default Extra Privileges Comment > +a2 int(11) NULL YES NULL select,insert,update,references > +b2 int(11) NULL YES NULL select,insert,update,references > +secret2 int(11) NULL YES NULL insert,update,references > +disconnect con1; > +# > +# Test insert commands > +# > +connection default; > +deny insert (secret) on deny_db.t1 to foo; > +connect con1,localhost,foo,,; > +insert into deny_db.t1 (a, b, secret) values (1, 2, 3); > +ERROR 42000: INSERT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +insert into deny_db.t1 values (1, 2, 3); > +ERROR 42000: INSERT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' didn't test this one, but likely should behave as select? that is, ER_TABLEACCESS_DENIED_ERROR please, test with GRANT (without DENY) before making any changes > +insert into deny_db.t1 select * from deny_db.t1; > +ERROR 42000: INSERT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +insert into deny_db.t1 select * from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret2' in table 't2' > +show full columns from deny_db.t1; > +Field Type Collation Null Key Default Extra Privileges Comment > +a int(11) NULL YES NULL select,insert,update,references > +b int(11) NULL YES NULL select,insert,update,references > +secret int(11) NULL YES NULL select,update,references > +disconnect con1; > +# > +# Test update commands. > +# > +connection default; > +deny select (secret) on deny_db.t1 to foo; > +deny select (secret2) on deny_db.t2 to foo; > +connect con1, localhost,foo,,; > +update deny_db.t1 set a=111, b=222 where secret=10; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +update deny_db.t1 set a=123 where a = (select secret2 from deny_db.t2); > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'secret2' in table 't2' > +show full columns from deny_db.t1; > +Field Type Collation Null Key Default Extra Privileges Comment > +a int(11) NULL YES NULL select,insert,update,references > +b int(11) NULL YES NULL select,insert,update,references > +secret int(11) NULL YES NULL update,references > +show full columns from deny_db.t2; > +Field Type Collation Null Key Default Extra Privileges Comment > +a2 int(11) NULL YES NULL select,insert,update,references > +b2 int(11) NULL YES NULL select,insert,update,references > +secret2 int(11) NULL YES NULL insert,update,references > +disconnect con1; > +connection default; > +deny update (secret) on deny_db.t1 to foo; > +connect con1, localhost,foo,,; > +update deny_db.t1 set a=111, b=222; > +update deny_db.t1 set secret=333, a=111; > +ERROR 42000: UPDATE command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +update deny_db.t1 set secret=333; > +ERROR 42000: UPDATE command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +update deny_db.t1 set a=123, secret=333; > +ERROR 42000: UPDATE command denied to user 'foo'@'localhost' for column 'secret' in table 't1' > +disconnect con1; > +connection default; > +drop user foo; > +drop database deny_db; > diff --git a/mysql-test/suite/deny/columns_role.result b/mysql-test/suite/deny/columns_role.result > --- /dev/null > +++ b/mysql-test/suite/deny/columns_role.result > @@ -0,0 +1,57 @@ > +create database some_db; > +create view some_db.v1 as (select 1 as col); > +create user foo; > +create role bar; > +grant select on *.* to foo; > +grant show view on *.* to foo; > +grant bar to foo; > +grant select(col) on some_db.v1 to foo; > +revoke ignore denies on *.* from foo; why foo has ignore denies here? I don't see where it was granted. > +deny select (col) on some_db.v1 to bar; > +connect con1,localhost,foo,,; > +describe some_db.v1; > +Field Type Null Key Default Extra > +col int(1) NO 0 > +set role bar; > +describe some_db.v1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'v1' this is very questionable. Should SET ROLE ever restrict what one can do? > +show create view some_db.v1; > +View Create View character_set_client collation_connection > +v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `some_db`.`v1` AS (select 1 AS `col`) latin1 latin1_swedish_ci > +disconnect con1; > +connection default; > +drop user foo; > +drop role bar; > +drop database some_db; > +# > +# Test if only role level denies are active. > +# > +create database some_db; > +create view some_db.v1 as (select 1 as col); > +create user foo; > +create role bar; > +grant bar to foo; > +grant select(col) on some_db.v1 to foo; > +deny select (col) on some_db.v1 to bar; > +connect con1,localhost,foo,,; > +# > +# Due to select(col) privilege on v1 we can see some_db here. > +# > +show databases; > +Database > +information_schema > +some_db > +test > +set role bar; > +# > +# Now the role's denies take effect and we can no longer see some_db. > +# > +show databases; > +Database > +information_schema > +test > +disconnect con1; > +connection default; > +drop user foo; > +drop role bar; > +drop database some_db; > diff --git a/mysql-test/suite/deny/deny_datastructures.result b/mysql-test/suite/deny/deny_datastructures.result > --- /dev/null > +++ b/mysql-test/suite/deny/deny_datastructures.result > @@ -0,0 +1,109 @@ > +create user foo; > +create role bar; > +# > +# Test deny format for global privileges. > +# > +deny select on *.* to foo; > +deny insert on *.* to bar; > +select user, host, JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +user host JSON_EXTRACT(priv, '$.deny') > +foo % {"global": 1, "version_id": VERSION_ID} > +select user, host, JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'bar'; > +user host JSON_EXTRACT(priv, '$.deny') > +bar {"global": 2, "version_id": VERSION_ID} > +flush privileges; > +deny select on some_db.* to foo; > +deny show view on some_db.* to foo; > +deny insert on some_other_db.* to foo; > +deny insert on some_other_db.* to bar; > +deny select on some_other_db.some_table to foo; > +deny insert on some_other_db.some_table_second to foo; > +deny update on some_other_db.some_table_third to foo; > +deny select(a, b) on some_other_db.some_table_third to foo; > +deny select, insert(a, b, c) on some_other_db.some_table_fourth to foo; > +select user, host, JSON_DETAILED(JSON_EXTRACT(priv, '$.deny')) from mysql.global_priv where user = 'foo'; > +user host JSON_DETAILED(JSON_EXTRACT(priv, '$.deny')) > +foo % { > + "global": 1, > + "db": > + [ > + > + { > + "name": "`some_db`", > + "access": 4194305 > + }, > + > + { > + "name": "`some_other_db`", > + "access": 2 > + } > + ], > + "table": > + [ > + > + { > + "name": "`some_other_db`.`some_table_second`", you sure you want that? you'd need to parse the name back when reading privileges, why would you need this extra complexity? I'd suggest something like "name" : [ "some_other_db", "some_table_second" ] (one component for db grants, three components for column grants) This way you don't need to parse the identifier to split on dots and remove backticks. You can even use [] for global grants and not split them into global/db/table/column lists, but have simply [ { "name ": [], "access": 1 }, { "name": ["some_db"], "access": 4194305 }, { "name": ["some_other_db"], "access": 2 } { "name": ["some_other_db", "some_table_second"], "access": 2 }, { "name": ["some_other_db", "some_table"], "access": 1 }, { "name": ["some_other_db", "some_table_third"], "access": 4 }, { "name": ["some_other_db", "some_table_fourth"], "access": 1 } { "name": ["some_other_db", "some_table_third", "a"], "access": 1 }, { "name": ["some_other_db", "some_table_fourth", "b"], "access": 2 }, { "name": ["some_other_db", "some_table_fourth", "c"], "access": 2 }, { "name": ["some_other_db", "some_table_third", "b"], "access": 1 }, ] May be it's easier if they're split into global/db/table/column, but there's defenitely no reason to put all identifier chain in one json property, quoted according to SQL rules. > + "access": 2 > + }, > + > + { > + "name": "`some_other_db`.`some_table`", > + "access": 1 > + }, > + > + { > + "name": "`some_other_db`.`some_table_third`", > + "access": 4 > + }, > + > + { > + "name": "`some_other_db`.`some_table_fourth`", > + "access": 1 > + } > + ], > + "column": > + [ > + > + { > + "name": "`some_other_db`.`some_table_third`.`a`", > + "access": 1 > + }, > + > + { > + "name": "`some_other_db`.`some_table_fourth`.`b`", > + "access": 2 > + }, > + > + { > + "name": "`some_other_db`.`some_table_fourth`.`c`", > + "access": 2 > + }, > + > + { > + "name": "`some_other_db`.`some_table_third`.`b`", > + "access": 1 > + }, > + > + { > + "name": "`some_other_db`.`some_table_fourth`.`a`", > + "access": 2 > + } > + ], > + "version_id": VERSION_ID > +} > +select user, host, JSON_DETAILED(JSON_EXTRACT(priv, '$.deny')) from mysql.global_priv where user = 'bar'; > +user host JSON_DETAILED(JSON_EXTRACT(priv, '$.deny')) > +bar { > + "global": 2, > + "db": > + [ > + > + { > + "name": "`some_other_db`", > + "access": 2 > + } > + ], > + "version_id": VERSION_ID > +} > +drop user foo; > +drop role bar; > diff --git a/mysql-test/suite/deny/dml.test b/mysql-test/suite/deny/dml.test > --- /dev/null > +++ b/mysql-test/suite/deny/dml.test > @@ -0,0 +1,241 @@ would be good to have a comment at the top of every test file you add to mention the 1) MDEV and 2) the purpose for that file to exist. E.g. in this case I wouldn't need to waste time guessing how dml.test is different from columns.test, if both have column-level grants. And even the content looks suspiciously the same. > +--source include/not_embedded.inc > + > +create user foo; > + > +create database deny_db; > +create table deny_db.t1 (a int, b int, secret int); > +insert into deny_db.t1 values (1, 2, 3); > + > +create table deny_db.t2 (a int, b int, secret int); > +insert into deny_db.t2 values (10, 20, 30); > + > +grant select (a) on deny_db.t1 to foo; > +grant insert (a) on deny_db.t1 to foo; > +grant update (a) on deny_db.t1 to foo; > +grant delete on deny_db.t1 to foo; > + > +--connect (con1,localhost,foo,,) > +insert into deny_db.t1 (a) values (1), (2), (3); > +disconnect con1; > + > +connection default; > +deny insert on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a) values (1), (2), (3); > +update deny_db.t1 set a=a*10; > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +connection default; > +select * from deny_db.t1 order by a; > +truncate deny_db.t1; > + > +revoke deny insert on deny_db.* from foo; > + > + > +--connect (con1,localhost,foo,,) > +insert into deny_db.t1 (a) values (1), (2), (3); > +--echo # > +--echo # Test that revoking insert deny allows inserting only into allowed > +--echo # to insert columns. > +--echo # > +insert into deny_db.t1 (a) values (1), (2), (3); > + > +--error ER_COLUMNACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > +disconnect con1; > + > +connection default; > +--echo # > +--echo # Test delete/update command denies. > +--echo # > +deny select on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--echo # > +--echo # Test not allowed to delete / update because of lack of select > +--echo # rights. > +--echo # > + > +show grants; > +--error ER_COLUMNACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10 where a = 1; Did you verify that in all cases the error is the same as if the privilege simply wasn't granted in the first place? In this case the error is correct, I've checked. > + > +--error ER_COLUMNACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > + > +connection default; > +--echo # > +--echo # Now deny update and delete directly, but keep select. > +--echo # > +revoke deny select on deny_db.* from foo; > +deny delete, update on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--echo # > +--echo # Test not allowed to delete / update because of lack of delete/update > +--echo # rights. > +--echo # > + > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > + > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +--echo # > +--echo # Test masking table level grants. > +--echo # > + > +connection default; > +grant select on deny_db.t1 to foo; > +grant insert on deny_db.t1 to foo; > +grant update on deny_db.t1 to foo; > +grant delete on deny_db.t1 to foo; > + > +--connect (con1,localhost,foo,,) > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > +disconnect con1; > + > +connection default; > +deny insert on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > + > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > + > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > + > +disconnect con1; > + > +connection default; > +revoke deny update, delete on deny_db.* from foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > + > +update deny_db.t1 set a=a*10; > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +--echo # > +--echo # Test masking database & global level grants. > +--echo # > +connection default; > +grant select, insert, update, delete on *.* to foo; > +grant select, insert, update, delete on deny_db.* to foo; > +grant select, insert, update, delete on deny_db.t1 to foo; > +deny select, insert on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > + > +--error ER_COLUMNACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > + > +--error ER_COLUMNACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +connection default; > +revoke deny select on deny_db.* from foo; > +deny update, delete on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > + > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > + > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +connection default; > +deny select on deny_db.* to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > + > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > + > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +--echo # > +--echo # Test revoking all denies. > +--echo # > +connection default; > +revoke deny select, insert, update, delete on deny_db.* from foo; by the way, what does REVOKE ALL PRIVILEGES do, does it remove denies? please, add a test. > +truncate deny_db.t1; > + > +--connect (con1,localhost,foo,,) > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > +update deny_db.t1 set a=a*10; > +select * from deny_db.t1; > +delete from deny_db.t1 where a = 20; > +disconnect con1; > + > +connection default; > + > +--echo # > +--echo # Test masking via table level denies > +--echo # > + > +deny select on deny_db.t1 to foo; > + > +--connect (con1,localhost,foo,,) > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > +--error ER_COLUMNACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > +--error ER_COLUMNACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10 where a = 10; > +--echo # > +--echo # Delete works without select. > +--echo # > +delete from deny_db.t1; > +--error ER_COLUMNACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > + > +--error ER_TABLEACCESS_DENIED_ERROR > +select * from deny_db.t1; > +disconnect con1; > + > +connection default; > +revoke deny select on deny_db.t1 from foo; > +deny insert on deny_db.t1 to foo; > +deny update on deny_db.t1 to foo; > +deny delete on deny_db.t1 to foo; > + > +--connect (con1,localhost,foo,,) > +--error ER_TABLEACCESS_DENIED_ERROR > +insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30); > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10; > +--error ER_TABLEACCESS_DENIED_ERROR > +update deny_db.t1 set a=a*10 where a = 10; > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1; > +--error ER_TABLEACCESS_DENIED_ERROR > +delete from deny_db.t1 where a = 20; > + > +select * from deny_db.t1; > + > +connection default; > +drop database deny_db; > +drop user foo; > diff --git a/mysql-test/suite/deny/global.result b/mysql-test/suite/deny/global.result > --- /dev/null > +++ b/mysql-test/suite/deny/global.result > @@ -0,0 +1,327 @@ > +create user foo; > +create database deny_db; > +create table deny_db.t1 (a int, b int, secret int); > +insert into deny_db.t1 values (1, 2, 3); > +create table deny_db.t2 (a int, b int, secret int); > +insert into deny_db.t2 values (10, 20, 30); > +# > +# Test global denies. > +# > +show databases; > +Database > +deny_db > +information_schema > +mtr > +mysql > +performance_schema > +sys > +test > +grant select (secret) on deny_db.t1 to foo; > +connect con1,localhost,foo,,; > +select * from deny_db2.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +show databases; > +Database > +deny_db > +information_schema > +test > +use deny_db; > +# > +# It is still possible to leak column names without denies. > +# > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1' this is a bug. See my test above, it's ER_TABLEACCESS_DENIED_ERROR. Apparently, it returns ER_TABLEACCESS_DENIED_ERROR if there's at least one granted column before the non-granted column. That is, if you'd have create table deny_db.t1 (secret int, a int, b int); you'd get ER_TABLEACCESS_DENIED_ERROR. please fix it (where applicable, 10.3, perhaps?) > +select a from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1' > +select b from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'b' in table 't1' > +select table_name, column_name from information_schema.columns > +where table_schema = 'deny_db'; > +table_name column_name > +t1 secret > +# > +# Invalid field error is returned if table exists. > +# > +select c from deny_db.t1; > +ERROR 42S22: Unknown column 'c' in 'field list' I feel like the whole column-leaking behavior is a bug too. This, likely, should be ER_COLUMNACCESS_DENIED_ERROR too > +# > +# Table access denied if table does exist but is not granted. > +# > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +# > +# Same error if table does not exist. > +# > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +secret > +3 > +# > +# Now apply deny. > +# > +connection default; > +deny select on *.* to foo; I see you reconnect to apply denies. What are the rules here? With grants, table/column grants work right away, while global and current-db grants are cached in the thd. but table/column denies aren't stored in separate tables, that's why the question. What are the rules here? > +disconnect con1; > +connect con1,localhost,foo,,; > +show databases; > +Database > +information_schema > +test > +use information_schema; > +use deny_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'deny_db' > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +# > +# Grant table access then test deny. > +# > +connection default; > +grant select on deny_db.t1 to foo; > +connection con1; > +show databases; > +Database > +information_schema > +test > +use information_schema; > +use deny_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'deny_db' > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +# > +# Grant database access then test deny. > +# > +connection default; > +grant select on deny_db.* to foo; > +connection con1; > +show databases; > +Database > +information_schema > +test > +use information_schema; > +use deny_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'deny_db' > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +# > +# Grant global access then test deny. > +# > +connection default; > +grant select on *.* to foo; > +connection con1; > +show databases; > +Database > +information_schema > +test > +use information_schema; > +use deny_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'deny_db' > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +connection default; > +# > +# Test underlying data loading. > +# > +select user, host, JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +user host JSON_EXTRACT(priv, '$.deny') > +foo % {"global": 1, "version_id": 101100} > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY SELECT ON *.* TO `foo`@`%` > +GRANT SELECT ON `deny_db`.* TO `foo`@`%` > +GRANT SELECT, SELECT (secret) ON `deny_db`.`t1` TO `foo`@`%` meaning, the user will see what is denied to him? I expect it'll be a questionable item, some users will want denies to be not shown. Although I don't see how they cannot be. > +flush privileges; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY SELECT ON *.* TO `foo`@`%` > +GRANT SELECT ON `deny_db`.* TO `foo`@`%` > +GRANT SELECT, SELECT (secret) ON `deny_db`.`t1` TO `foo`@`%` > +connection con1; > +show databases; > +Database > +information_schema > +test > +use information_schema; > +use deny_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'deny_db' > +select * from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select c from deny_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select c from deny_db.t_not_exists; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't_not_exists' > +select secret from deny_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +disconnect con1; > +connection default; > +# > +# Now test insert not being denied. > +# > +grant insert(secret) on deny_db.t1 to foo; > +connect con1,localhost,foo,,; > +show databases; > +Database > +deny_db > +information_schema > +test > +insert into deny_db.t1(secret) values (10000); > +disconnect con1; > +connection default; > +drop database deny_db; > +drop user foo; > +# > +# Test denying all rights globally. please, add tests with DENY TO PUBLIC > +# User should still be able to connect and see information_schema > +# *at least*. > +# > +create user foo; > +create database some_db; > +grant select on *.* to foo; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +deny select on *.* to foo; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY SELECT ON *.* TO `foo`@`%` > +deny all on *.* to foo; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY ALL PRIVILEGES ON *.* TO `foo`@`%` > +connect con1,localhost,foo,,information_schema; > +show databases; > +Database > +information_schema > +disconnect con1; > +connection default; > +drop user foo; > +drop database some_db; > +############################################## > +# Test SELECT command interacting with deny. # > +############################################## > +create user foo; > +create user bar; > +create database some_db; > +create table some_db.t1 (a int, secret int); > +insert into some_db.t1 values (1, 100); > +use some_db; > +create view v1 as (select a from t1); > +create view v2 as (select secret from t1); > +connect con1,localhost,foo,,; > +select table_name, table_type from information_schema.tables where table_schema like 'some_db'; > +table_name table_type > +disconnect con1; > +connection default; > +grant select on *.* to foo; > +grant select on some_db.* to foo; > +grant select on some_db.t1 to foo; > +grant select(a) on some_db.t1 to foo; > +# > +# See what foo sees before denies. > +# > +connect con1,localhost,foo,,; > +select table_name, table_type from information_schema.tables where table_schema like 'some_db'; > +table_name table_type > +t1 BASE TABLE > +v1 VIEW > +v2 VIEW > +disconnect con1; > +connection default; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +GRANT SELECT ON `some_db`.* TO `foo`@`%` > +GRANT SELECT, SELECT (a) ON `some_db`.`t1` TO `foo`@`%` > +deny select on *.* to foo; > +show grants for foo; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY SELECT ON *.* TO `foo`@`%` > +GRANT SELECT ON `some_db`.* TO `foo`@`%` > +GRANT SELECT, SELECT (a) ON `some_db`.`t1` TO `foo`@`%` > +connect con1,localhost,foo,,; > +show grants; > +Grants for foo@% > +GRANT SELECT ON *.* TO `foo`@`%` > +DENY SELECT ON *.* TO `foo`@`%` > +GRANT SELECT ON `some_db`.* TO `foo`@`%` > +GRANT SELECT, SELECT (a) ON `some_db`.`t1` TO `foo`@`%` > +use some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +select * from some_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +select * from some_db.v1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'v1' > +select * from some_db.v2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'v2' > +select table_name, table_type from information_schema.tables where table_schema like "some_db"; > +table_name table_type > +select table_name, column_name from information_schema.columns where table_schema like "some_db"; > +table_name column_name > +disconnect con1; > +connection default; > +grant insert(a) on some_db.t1 to bar; > +grant insert(a) on some_db.t1 to foo; > +connect con1,localhost,foo,,; > +select table_name, column_name from information_schema.columns where table_schema like "some_db"; > +table_name column_name > +t1 a > +disconnect con1; > +connect con2,localhost,bar,,; > +select table_name, column_name from information_schema.columns where table_schema like "some_db"; > +table_name column_name > +t1 a > +disconnect con2; > +connection default; > +deny insert on *.* to foo; > +deny insert on *.* to bar; > +connect con1,localhost,foo,,; > +select table_name, column_name from information_schema.columns where table_schema like "some_db"; > +table_name column_name > +disconnect con1; > +connect con2,localhost,bar,,; > +select table_name, column_name from information_schema.columns where table_schema like "some_db"; > +table_name column_name > +disconnect con2; > +connection default; > +drop user foo; > +drop user bar; > +drop database some_db; > diff --git a/mysql-test/suite/deny/ignore_denies.result b/mysql-test/suite/deny/ignore_denies.result > --- /dev/null > +++ b/mysql-test/suite/deny/ignore_denies.result > @@ -0,0 +1,76 @@ > +create user foo; > +create role bar; > +grant select, ignore denies on *.* to foo; > +deny select on *.* to foo; > +deny select on mysql.* to foo; > +deny select on mysql.global_priv to foo; > +deny select(user) on mysql.global_priv to foo; > +grant bar to foo; > +grant ignore denies on *.* to bar; > +connect con1,localhost,foo,,; > +# > +# The user should still be able to access mysql.global_priv due > +# to the IGNORE DENIES privilege being active. > +# > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +JSON_EXTRACT(priv, '$.deny') > +{"global": 1, "db": [{"name": "`mysql`", "access": 1}], "table": [{"name": "`mysql`.`global_priv`", "access": 1}], "column": [{"name": "`mysql`.`global_priv`.`user`", "access": 1}], "version_id": VERSION_ID} > +disconnect con1; > +connection default; > +# > +# Now test inheriting IGNORE DENIES from a role. > +# > +revoke ignore denies on *.* from foo; > +connect con1,localhost,foo,,; > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'global_priv' > +# > +# Bar provides foo with IGNORE DENIES privilege. > +# > +set role bar; > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +JSON_EXTRACT(priv, '$.deny') > +{"global": 1, "db": [{"name": "`mysql`", "access": 1}], "table": [{"name": "`mysql`.`global_priv`", "access": 1}], "column": [{"name": "`mysql`.`global_priv`.`user`", "access": 1}], "version_id": VERSION_ID} > +disconnect con1; > +connection default; > +# > +# Denying IGNORE DENIES, overrides a user's ability to ignore denies. This is very questionable. One can DENY IGNORE DENIES TO PUBLIC and basically lock the server down for everyone, there will be no way to override that. (this is an observation, not ready to suggest a solution atm) > +# > +deny ignore denies on *.* to foo; > +grant ignore denies on *.* to foo; > +connect con1,localhost,foo,,; > +# > +# Foo can't ignore the SELECT priv deny. > +# > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'global_priv' > +# > +# And neither if bar role provides the ignore deny priv. > +# > +set role bar; > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'global_priv' > +disconnect con1; > +connection default; > +# > +# Now test what happens when IGNORE DENIES is denied from a role. > +# > +revoke deny ignore denies on *.* from foo; > +deny ignore denies on *.* to bar; > +connect con1,localhost,foo,,; > +# > +# Foo can ignore the SELECT priv deny when Bar isn't activated > +# > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +JSON_EXTRACT(priv, '$.deny') > +{"global": 1, "db": [{"name": "`mysql`", "access": 1}], "table": [{"name": "`mysql`.`global_priv`", "access": 1}], "column": [{"name": "`mysql`.`global_priv`.`user`", "access": 1}], "version_id": VERSION_ID} > +# > +# But when Bar is active, denies can not be ignored any more. > +# > +set role bar; > +select JSON_EXTRACT(priv, '$.deny') from mysql.global_priv where user = 'foo'; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 'global_priv' > +disconnect con1; > +connection default; > +drop user foo; > +drop role bar; > diff --git a/mysql-test/suite/deny/show_generic.result b/mysql-test/suite/deny/show_generic.result > --- /dev/null > +++ b/mysql-test/suite/deny/show_generic.result > @@ -0,0 +1,197 @@ > +# > +# This test checks that denies take part in SHOW commands properly. > +# > +create user foo; > +create database some_db; > +create table some_db.t1 (id int); > +connect con1,localhost,foo,,; > +show tables from some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +disconnect con1; > +connection default; > +grant insert on some_db.t1 to foo; > +connect con1,localhost,foo,,; > +show tables from some_db; > +Tables_in_some_db > +t1 > +disconnect con1; > +connection default; > +deny insert on some_db.* to foo; > +connect con1,localhost,foo,,; > +show databases; > +Database > +information_schema > +test > +show tables from some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +disconnect con1; > +connection default; > +# > +# Test that dropping a user does not give access to connections that > +# previously had elements denied. > +# > +# We will use two users, foo which will receive denies, bar which will > +# not. This will showcase the difference in behaviour. > +# > +create user bar; > +# > +# This global grant is cached in sctx->master_access on connection. > +# > +grant select on *.* to foo; > +grant select on *.* to bar; > +connect con1,localhost,foo,,; > +show tables from some_db; > +Tables_in_some_db > +t1 > +disconnect con1; > +connection default; > +# > +# Here we add a mask for some_db, it should no longer be visible to foo. > +# > +deny select on some_db.* to foo; > +connect con1,localhost,foo,,; > +use some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +show tables from some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +# > +# bar user has access to the database (no denies present) > +# > +connect con2,localhost,bar,,; > +use some_db; > +show tables from some_db; > +Tables_in_some_db > +t1 > +# > +# Do not disconnect foo & bar, but drop the users. > +# > +connection default; > +drop user foo; > +drop user bar; > +# > +# Our current running connection, because it featured denies previously > +# now doesn't have access. > +# > +connection con1; > +use some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +show tables from some_db; > +ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db' > +disconnect con1; > +# > +# However, bar does not have any denies active. In this case we can keep > +# the previous MariaDB behaviour of using the "cache" within > +# sctx->master_access to grant access. This comment is confusing. As far as I can see both foo and bar preserve the behavior after `drop user`. Which is fine and what I was expected anyway. But comments imply there's some difference in behavior between them? > +# > +connection con2; > +use some_db; > +show tables from some_db; > +Tables_in_some_db > +t1 > +disconnect con2; > +connection default; > +drop database some_db; > +# > +# Test table level denies interacting with show tables > +# > +create database some_db; > +create user foo; > +create table some_db.t1 (a int, b int); > +create table some_db.t2 (a int, b int); > +grant select on *.* to foo; > +deny select on some_db.t1 to foo; > +deny insert on some_db.t2 to foo; > +select user, host, JSON_EXTRACT(priv, '$.deny') from mysql.global_priv > +where user = 'foo'; > +user host JSON_EXTRACT(priv, '$.deny') > +foo % {"table": [{"name": "`some_db`.`t2`", "access": 2}, {"name": "`some_db`.`t1`", "access": 1}], "version_id": VERSION_ID} > +connect con1,localhost,foo,,; > +show tables from some_db; > +Tables_in_some_db > +t2 > +show columns from some_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +show columns from some_db.t2; > +Field Type Null Key Default Extra > +a int(11) YES NULL > +b int(11) YES NULL > +select table_name, column_name, privileges from information_schema.columns > +where table_schema like 'some_db' order by table_name, column_name; > +table_name column_name privileges > +t2 a select > +t2 b select > +disconnect con1; > +connection default; > +grant insert on some_db.* to foo; > +connect con1,localhost,foo,,; > +show tables from some_db; > +Tables_in_some_db > +t1 > +t2 > +# > +# See MDEV-28783, this should not error out when global/db grants exist > +# (except for SELECT priv). > +# > +show columns from some_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +show columns from some_db.t2; > +Field Type Null Key Default Extra > +a int(11) YES NULL > +b int(11) YES NULL > +select table_name, column_name, privileges from information_schema.columns > +where table_schema like 'some_db' order by table_name, column_name; > +table_name column_name privileges > +t1 a insert > +t1 b insert > +t2 a select > +t2 b select > +disconnect con1; > +connection default; > +deny insert on some_db.t1 to foo; > +deny select on some_db.t2 to foo; > +connect con1,localhost,foo,,; > +# > +# some_db should still be visible, but it should show up as empty. > +# > +show databases; > +Database > +information_schema > +mtr > +mysql > +performance_schema > +some_db > +sys > +test > +show tables from some_db; > +Tables_in_some_db > +show columns from some_db.t1; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1' > +show columns from some_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select table_name, column_name, privileges from information_schema.columns > +where table_schema like 'some_db' order by table_name, column_name; > +table_name column_name privileges > +disconnect con1; > +connection default; > +grant select(a) on some_db.t1 to foo; > +grant update(a) on some_db.t1 to foo; > +connect con1,localhost,foo,,; > +# > +# Update privilege on the column is not masked, only see a column. > +# > +show tables from some_db; > +Tables_in_some_db > +t1 > +show columns from some_db.t1; > +Field Type Null Key Default Extra > +a int(11) YES NULL > +show columns from some_db.t2; > +ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't2' > +select table_name, column_name, privileges from information_schema.columns > +where table_schema like 'some_db' order by table_name, column_name; > +table_name column_name privileges > +t1 a update > +disconnect con1; > +connection default; > +drop user foo; > +drop database some_db; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0