November 2023 - developers - lists.mariadb.org

[Maria-developers] MariaDB // Temporal tables
by Robert Palm 02 Oct '24

02 Oct '24

2 2

PR#2779: Fix merge commit 5ea5291: No test file or result files should be executable
by Otto Kekäläinen 28 Jan '24

28 Jan '24

Hi Sanja! Did you notice my PR https://github.com/MariaDB/server/pull/2779? I tagged you about a minor bug in you previous merge. I was hoping you would notice it and review that PR, but Daniel Black jumped on it. I am asking now because it seems the exact same thing happened for mysql-test/main/lowercase_table2.result in your commit fecd78b8378 from November 8th, so I thought I'd send an extra email now to ensure you are aware of this happening. - Otto

2 3

MTR failures on official Debian buildd after MariaDB 1:10.11.6-1 upload
by Otto Kekäläinen 17 Dec '23

17 Dec '23

Hi! I just wanted to let you know there are a bunch of MTR failures/regressions at https://buildd.debian.org/status/package.php?p=mariadb which I need to fix or add to skip-lists. The previous upload of MariaDB 1:10.11.5-3 had more 'green' than ever before, and now there are regressions on 6 archs. [image: image.png]

3 6

Re: d25ab9fdfa1: MDEV-31608 - Connector/NET fails to connect since 10.10
by Sergei Golubchik 30 Nov '23

30 Nov '23

Hi, Vladislav, On Nov 29, Vladislav Vaintroub wrote: > revision-id: d25ab9fdfa1 (mariadb-10.11.2-62-gd25ab9fdfa1) > parent(s): 034848c6c27 > author: Vladislav Vaintroub > committer: Vladislav Vaintroub > timestamp: 2023-10-08 19:37:20 +0200 > message: > > MDEV-31608 - Connector/NET fails to connect since 10.10 > > Connector/NET does not expect collation IDs returned by "show collations" > to be NULL, runs into an exception. > > The fix is to determine connector/net using its connection attributes, > exclude null-id collations from "show collations".The patch introduces > new old_mode NO_NULL_COLLATION_IDs, that is automatically set, > once MySQL Connector/NET connection is determined. I don't like this hack at all, but I don't have any better ideas, so just one question below > A test was added, that uses MySql.Data from powershell - only works > if MySql.Data is installed into GAC (i.e with C/NET MSI package) please, make sure that MySql.Data is installed on at least one buildbot builder. > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index bff55f7a431..bea993305be 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -6386,7 +6386,8 @@ int fill_schema_collation(THD *thd, TABLE_LIST *tables, COND *cond) > character set prefix). > */ > if (is_context && > - cmp(tmp_cl->cs_name, Lex_cstring(STRING_WITH_LEN("utf8mb4")))) > + ((thd->variables.old_behavior & OLD_MODE_NO_NULL_COLLATION_IDS) || > + cmp(tmp_cl->cs_name, Lex_cstring(STRING_WITH_LEN("utf8mb4"))))) wouldn't it be better to do instead - bool is_context= cmp(context_collation_name, full_collation_name); + bool is_context= cmp(context_collation_name, full_collation_name) + && !(thd->variables.old_behavior & OLD_MODE_NO_NULL_COLLATION_IDS); ? > continue; > restore_record(table, s->default_values); > table->field[0]->store(context_collation_name, scs); Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Re: [PATCH] MDEV-32551: "Read semi-sync reply magic number error" warnings on master
by Kristian Nielsen 23 Nov '23

23 Nov '23

Hi Monty, > From: Michael Widenius <monty(a)mariadb.com> > > commit 7abf83f86829ed94de618af4749eb96d5379bd90 (origin/bb-10.6-monty) > Author: Michael Widenius <monty(a)mariadb.com> > Date: Wed Nov 8 16:57:58 2023 -0700 > > MDEV-32551: "Read semi-sync reply magic number error" warnings on master I read through the patch, looks good. A few comments below, mostly minor things and typos that you can consider as you like. - Kristian. > - We should in 11.3 changed the default value for > rpl-semi-sync-master-wait-no-slave from TRUE to FALSE as the TRUE > Sys_semisync_master_wait_no_slave( > "rpl_semi_sync_master_wait_no_slave", > - "Wait until timeout when no semi-synchronous replication slave " > - "available (enabled by default).", > + "Wait until timeout when no semi-synchronous replication slave is " > + "available.", > GLOBAL_VAR(rpl_semi_sync_master_wait_no_slave), > CMD_LINE(OPT_ARG), DEFAULT(TRUE), > - NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0), > - ON_UPDATE(fix_rpl_semi_sync_master_wait_no_slave)); > + NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0)); > -my_bool rpl_semi_sync_master_wait_no_slave = 1; > +my_bool rpl_semi_sync_master_wait_no_slave = 0; There seems to be som inconsistency here: - The commit is based on 10.6, but the commit comment says 11.3 - The option help text is changed in the commit - The "DEFAULT(TRUE)" in the sysvar definition is _not_ changed in the commit - The C initialization values of the my_bool rpl_semi_sync_master_wait_no_slave _is_ changed in the commit. Should it be DEFAULT(FALSE) also in this patch, and the commit message say "10.6"? Or should these changes be removed from the 10.6 patch and put in 11.3 instead? At least this looks inconsistent. > diff --git a/mysql-test/include/wait_for_pattern_in_file.inc b/mysql-test/include/wait_for_pattern_in_file.inc > new file mode 100644 > index 00000000000..d088cb125c2 > --- /dev/null > +++ b/mysql-test/include/wait_for_pattern_in_file.inc > @@ -0,0 +1,54 @@ > +# ==== Purpose ==== > +# > +# Waits until pattern comes into log file or until a timeout is reached. Nice addition, this will be useful for other things! > +--let $keep_include_silent= 1 Why do you set this in the .inc? (Normally this is set in the .test files that want to skip the printout). +let SEARCH_SILENT=1; Maybe the .inc should restore the old value at the end (in case a .test will at some point set it globally for its own calls to include/search_pattern_in_file.inc) > +--let $keep_include_silent= 0 Here, better restore the original value (in case some .test has set it to 1 for itself). > +# Test is binlog format indepdendent, so save resources (Typo s/indepdendent/independent/) > +# The followowing command is likely to cause the slave master is not yet setup > +# for semi-sync (Typo s/followowing/following/) > - strcpy(m_reply_file_name, ""); > - strcpy(m_wait_file_name, ""); > + m_reply_file_name[0]= m_wait_file_name[0]= 0; Interestingly, GCC actually creates identical code for these (it knows to optimize strcpy()). (I'm just mentioning this as I know we both are interested in low-level code details, not asking you to revert the change). Example code: ----------------------------------------------------------------------- #include <string.h> struct X { char m_a[100]; char m_b[10]; void init1(); void init2(); }; void X::init1() { strcpy(m_a, ""); strcpy(m_b, ""); } void X::init2() { m_a[0]= m_b[0]= 0; } ----------------------------------------------------------------------- Result from gcc -O3: ----------------------------------------------------------------------- .type _ZN1X5init1Ev, @function _ZN1X5init1Ev: movb $0, (%rdi) movb $0, 100(%rdi) ret .type _ZN1X5init2Ev, @function _ZN1X5init2Ev: movb $0, 100(%rdi) movb $0, (%rdi) ret ----------------------------------------------------------------------- Identical, except that we now initialize in the opposite order ;-) > +#ifndef DBUG_OFF > +void dbug_verify_no_duplicate_slaves(Slave_ilist *m_slaves, THD *thd) > +{ You could make this (debug-only) function "static". > + Master is not responding (gone away?) or it has turned semi sync > + off. Turning off semi-sync repsonces as there is no point in sending (Typo s/repsonces/responses/)

2 1

Re: dadcf5ed7b1: MDEV-24486 Add `table_privileges` view to the `sys` schema
by Sergei Golubchik 21 Nov '23

21 Nov '23

Hi, Oleg, ok to push. Just one comment below. On Nov 20, Oleg Smirnov wrote: > revision-id: dadcf5ed7b1 (mariadb-11.0.1-259-gdadcf5ed7b1) > parent(s): b8f9f796ff8 > author: Oleg Smirnov > committer: Oleg Smirnov > timestamp: 2023-11-20 19:35:36 +0700 > message: > > MDEV-24486 Add `table_privileges` view to the `sys` schema > > The existing INFORMATION_SCHEMA.TABLE_PRIVILEGES displays only those > privileges that were specifically granted on the table level, > whereas it may be useful to see privileges granted at the database > and global level. > > diff --git a/scripts/sys_schema/NEWS.md b/scripts/sys_schema/NEWS.md > index 7aeefb7edf9..12ce1382b75 100644 > --- a/scripts/sys_schema/NEWS.md > +++ b/scripts/sys_schema/NEWS.md > @@ -1,5 +1,12 @@ > # Change history for the MySQL sys schema > > +## 1.5.2 (11/20/23) > + > +### Improvements > + > +* A new `table_privileges` view was added, which displays privileges to tables granted at all levels (user, schema, table) is it "privileges to tables"? I feel like "privileges on tables" would be more correct. It's GRANT privilege ON object TO user same it two other places. > + > + > ## 1.5.1 (07/07/16) Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

3 3

Question about STACK_DIRETION fix removal in merge commit 6c05edf
by Otto Kekäläinen 21 Nov '23

21 Nov '23

Hi Marko! I noticed you removed from debian/rules a section that flipped -DSTACK_DIRECTION=-1/1 based on architecture in the merge commit https://github.com/MariaDB/server/commit/6c05edfdcd335b9587ba140ebef5d9b082… Was this intentional? I am not sure, but it looks like a mistake to me, so I wanted to confirm. The code originates from cross-compilation fix in Debian in https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/f0ba31e1565fe…, which was later upstreamed to 10.5 (but not on 10.4) in https://github.com/MariaDB/server/commit/ea56841997995283f04e85e957c8d7f315…. - Otto

2 1

Re: 73097e18b77: MDEV-32771 Server crash upon online alter with concurrent XA
by Sergei Golubchik 14 Nov '23

14 Nov '23

Hi, Nikita, ok to push. spelling and renaming suggestions below On Nov 13, Nikita Malyavin wrote: > revision-id: 73097e18b77 (mariadb-11.2.1-20-g73097e18b77) > parent(s): f7646d890b9 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2023-11-13 15:28:19 +0100 > message: > > MDEV-32771 Server crash upon online alter with concurrent XA > > In case of a non-recovery XA rollback/commit in the same connection, > thon->rollback is called instead of rollback_by_xid, Though reviously, "previously" > thd_ha_data was moved to thd->transaction->xid_state.xid in hton->prepare. > > Like it wasn't enough, XA PREPARE can be skipped upon user and thus we > can end up in hton->commit/rollback with and unprepared XA, so checking > xid_state.is_explicit_XA is not enough -- we should check > xid_state.get_state_code() == XA_PREPARED, which will also guarantee > is_explicit_XA() == true. > --- a/sql/online_alter.cc > +++ b/sql/online_alter.cc > @@ -315,17 +315,35 @@ int online_alter_savepoint_rollback(handlerton *hton, THD *thd, sv_id_t sv_id) > > static int online_alter_commit(handlerton *hton, THD *thd, bool all) > { > - int res= online_alter_end_trans(get_cache_list(hton, thd), thd, > - ending_trans(thd, all), true); > - cleanup_tables(thd); > + int res; > + bool commit= ending_trans(thd, all); > + if (thd->transaction->xid_state.get_state_code() == XA_PREPARED && commit) > + { > + res= hton->commit_by_xid(hton, thd->transaction->xid_state.get_xid()); > + // cleanup is done by prepare() this is conusing, there's no prepare after commit_by_xid. may be you wanted to say "cleanup was already done by prepare()" ? > + } > + else > + { > + res= online_alter_end_trans(get_cache_list(hton, thd), thd, commit, true); > + cleanup_tables(thd); > + } > return res; > }; > > static int online_alter_rollback(handlerton *hton, THD *thd, bool all) > { > - int res= online_alter_end_trans(get_cache_list(hton, thd), thd, > - ending_trans(thd, all), false); > - cleanup_tables(thd); > + int res; > + bool commit= ending_trans(thd, all); 'commit' is weird here, on the rollback path. may be you rename it to, like "is_ending_tran" or "end_tran" or "is_persistent" or something along these lines. And then 'commit' in online_alter_commit too, for consistency. > + if (thd->transaction->xid_state.get_state_code() == XA_PREPARED && commit) > + { > + res= hton->rollback_by_xid(hton, thd->transaction->xid_state.get_xid()); > + // cleanup is done by prepare() same as in online_alter_commit > + } > + else > + { > + res= online_alter_end_trans(get_cache_list(hton, thd), thd, commit, false); > + cleanup_tables(thd); > + } > return res; > }; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: MDEV-27576 Use reverse index for max/min optimization
by Sergey Petrunia 13 Nov '23

13 Nov '23

Hi Yuchen, > commit c4c4bd8e47e5bfd4d0a650e286ea9daa3e6276f0 > Author: Yuchen Pei <ycp(a)mariadb.com> > Date: Thu Nov 2 11:55:42 2023 +1100 > > MDEV-27576 Use reverse index for max/min optimization > > We add a field in st_table_ref to indicate that the key used is > a descending index, which will flip the functions and flags used in > get_index_max_value() and get_index_min_value(), that allows correct > optimization for max/min for descending index. Generally the patch looks good. I have only a few cosmetic comments. Please rename the test file from mdev_27576.test to something more descriptive like desc_index_min_max.test (following desc_index_range.test) Also please rename the table in the test to t1. > diff --git a/sql/sql_select.h b/sql/sql_select.h > index e0bbadadc69..45d7527280a 100644 > --- a/sql/sql_select.h > +++ b/sql/sql_select.h > @@ -126,6 +126,7 @@ typedef struct st_table_ref > uint key_parts; ///< num of ... > uint key_length; ///< length of key_buff > int key; ///< key no > + bool reverse; ///< key is reverse > uchar *key_buff; ///< value to look for with key > uchar *key_buff2; ///< key_buff+key_length > store_key **key_copy; // It's actually not "key is reverse" but "last used key part is reverse"... also this member is never set for the most common scenario of TABLE_REF use, which is ref access. The member is set in a rather trivial logic in find_key_for_maxmin(), and then it's used in get_index_min_value() and get_index_max_value(). I suggest that we do without this member. get_index_min_value() and get_index_max_value() can check: bool reverse= table->key_info[ref->key].key_part[ref->key_part]->key_part_flag & HA_REVERSE_SORT > diff --git a/sql/opt_sum.cc b/sql/opt_sum.cc > index 794ec40fc66..1f94a4f4dc7 100644 > --- a/sql/opt_sum.cc > +++ b/sql/opt_sum.cc > @@ -114,7 +114,8 @@ static int get_index_min_value(TABLE *table, TABLE_REF *ref, > int error; > > if (!ref->key_length) > - error= table->file->ha_index_first(table->record[0]); > + error= ref->reverse ? table->file->ha_index_last(table->record[0]) : > + table->file->ha_index_first(table->record[0]); Please add { ... } as the if-branch is a multi-line statement now. > else > { > /* > @@ -206,8 +210,12 @@ static int get_index_max_value(TABLE *table, TABLE_REF *ref, uint range_fl) > table->file->ha_index_read_map(table->record[0], ref->key_buff, > make_prev_keypart_map(ref->key_parts), > range_fl & NEAR_MAX ? > - HA_READ_BEFORE_KEY : > - HA_READ_PREFIX_LAST_OR_PREV) : > + (ref->reverse ? HA_READ_AFTER_KEY : > + HA_READ_BEFORE_KEY) : > + (ref->reverse ? HA_READ_KEY_OR_NEXT : > + HA_READ_PREFIX_LAST_OR_PREV)) : > + ref->reverse ? > + table->file->ha_index_first(table->record[0]) : > table->file->ha_index_last(table->record[0])); Multiple levels of x?y:z statements are hard to read. Please change to if (ref->key_length) { ... } else { ... } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Blog: http://petrunia.net

2 2

Re: 28deba0a611: MDEV-32101 CREATE PACKAGE [BODY] for sql_mode=DEFAULT
by Sergei Golubchik 10 Nov '23

10 Nov '23

Hi, Alexander, Looks great! Just a couple of questions below On Nov 07, Alexander Barkov wrote: > revision-id: 28deba0a611 (mariadb-11.0.1-239-g28deba0a611) > parent(s): e9573c05965 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2023-09-22 14:38:52 +0400 > message: > > MDEV-32101 CREATE PACKAGE [BODY] for sql_mode=DEFAULT > > This patch adds PACKAGE support with SQL/PSM dialect for sql_mode=DEFAULT: > > - CREATE PACKAGE > - DROP PACKAGE > - CREATE PACKAGE BODY > - DROP PACKAGE BODY > - Package function and procedure invocation from outside of the package: > -- using two step identifiers > SELECT pkg.f1(); > CALL pkg.p1() > > -- using three step identifiers > SELECT db.pkg.f1(); > CALL db.pkg.p1(); > > This is a non-standard MariaDB extension. > > However, later this code can be used to implement > the SQL Standard and DB2 dialects of CREATE MODULE. > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index af2ec678daa..fca39b63312 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -1285,7 +1285,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); > %left TRANSACTION_SYM TIMESTAMP PERIOD_SYM SYSTEM USER COMMENT_SYM > > %left PREC_BELOW_SP_OBJECT_TYPE > -%left FUNCTION_SYM > +%left PACKAGE_MARIADB_SYM FUNCTION_SYM Why PACKAGE_ORACLE_SYM and PACKAGE_ALLMODES_SYM don't need it? (same for BODY_ORACLE_SYM and BODY_ALLMODES_SYM) > > > /* > @@ -3049,6 +3059,8 @@ sp_handler: > | PROCEDURE_SYM { $$= &sp_handler_procedure; } > | PACKAGE_ORACLE_SYM { $$= &sp_handler_package_spec; } > | PACKAGE_ORACLE_SYM BODY_ORACLE_SYM { $$= &sp_handler_package_body; } > + | PACKAGE_MARIADB_SYM { $$= &sp_handler_package_spec; } > + | PACKAGE_MARIADB_SYM BODY_MARIADB_SYM { $$= &sp_handler_package_body; } Why not to use here PACKAGE_ALLMODES_SYM and BODY_ALLMODES_SYM? (also, I'd personally call them simply PACKAGE_SYM and BODY_SYM) > ; > > > @@ -19428,35 +19489,33 @@ create_routine: > (Item_result) $8, $10)) > MYSQL_YYABORT; > } > - | create_or_replace definer_opt PACKAGE_ORACLE_SYM > + | create_or_replace definer_opt PACKAGE_ALLMODES_SYM > opt_if_not_exists sp_name opt_create_package_chistics_init > { > sp_package *pkg; > if (unlikely(!(pkg= Lex-> > - create_package_start(thd, > - SQLCOM_CREATE_PACKAGE, > - &sp_handler_package_spec, > - $5, $1 | $4)))) > + create_package_start(thd, &sp_handler_package_spec, > + $5, $1 | $4, > + Lex->sp_chistics)))) > MYSQL_YYABORT; > pkg->set_c_chistics(Lex->sp_chistics); Looks redundant > Lex->sphead->set_body_start(thd, YYLIP->get_cpp_tok_start()); > } > sp_tail_is > opt_package_specification_element_list END > - remember_end_opt opt_sp_name > + remember_end_opt opt_trailing_sp_name > { > if (unlikely(Lex->create_package_finalize(thd, $5, $12, $11))) > MYSQL_YYABORT; > } > - | create_or_replace definer_opt PACKAGE_ORACLE_SYM BODY_ORACLE_SYM > + | create_or_replace definer_opt PACKAGE_ALLMODES_SYM BODY_ALLMODES_SYM > opt_if_not_exists sp_name opt_create_package_chistics_init > { > sp_package *pkg; > if (unlikely(!(pkg= Lex-> > - create_package_start(thd, > - SQLCOM_CREATE_PACKAGE_BODY, > - &sp_handler_package_body, > - $6, $1 | $5)))) > + create_package_start(thd, &sp_handler_package_body, > + $6, $1 | $5, > + Lex->sp_chistics)))) > MYSQL_YYABORT; > pkg->set_c_chistics(Lex->sp_chistics); same > Lex->sphead->set_body_start(thd, YYLIP->get_cpp_tok_start()); Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2