- developers - lists.mariadb.org

7 PRs pending second approver - can somebody do it?
by Otto Kekäläinen 11 Apr '24

11 Apr '24

Hi Sergei, Andrew or anyone else who wants to help, These PRs have already been approved by one reviewer 2-8 weeks ago and are pending the second reviewer to approve/merge. These indicate that Sergei's review is requested, but I guess anybody could be the second reviewer: https://github.com/MariaDB/server/pull/2169 https://github.com/MariaDB/server/pull/2671 The indicate that Andrewi's review is requested, but I guess anybody could be the second reviewer: https://github.com/MariaDB/server/pull/3100 https://github.com/MariaDB/server/pull/3120 https://github.com/MariaDB/server/pull/3125 https://github.com/MariaDB/server/pull/3142 https://github.com/MariaDB/server/pull/3154 I am sending this to the mailing list as well just to raise general awareness that while Andrew and Daniel are putting in a lot of effort to review "external" contributions, it is not enough and even approved PRs get stalled. Other core contributors should perhaps get into the habit of reviewing/approving PRs as well. If there was a rule in github.com/MariaDB/server that prevents any commits from being merged unless it has two approvals, it would both help improve the code quality AND also force core contributors to spend more time adding approvals on GitHub and thus automatically increase the likelihood that there are no one-approval PRs around for weeks and weeks without getting the second approval. - Otto

3 2

MariaDB supports DBdeployer and MySqlJS, but they don't support MariaDB?
by Otto Kekäläinen 09 Apr '24

09 Apr '24

Hi! While digging into the CI situation I noticed that https://buildbot.mariadb.org/ advertises: "We run other ecosystem tests for: * DBdeployer * MySqlJS" However, when I browse https://github.com/mysqljs/mysql and https://github.com/datacharmer/dbdeployer there are zero mentions of MariaDB. Just wondering is anybody working to get them to mention MariaDB in their docs? If they are tested as part of the MariaDB CI, surely they should reciprocate and at least tell in their README that MariaDB is also supported. Maybe somebody from the MariaDB Foundation could send a PR to update their README, so they don't need to do anything else than just accept it. This was done for example in https://github.com/mysql-net/MySqlConnector/pull/1460/files by Daniel Lenski, and now that project mentions both MySQL and MariaDB. - Otto

3 2

Safe use of sprint/printf/fprintf in MariaDB code base
by Otto Kekäläinen 08 Apr '24

08 Apr '24

Hi all, Reading about the xz-utils backdoor authors submission of converting safe_fprintf() to fprintf() in libarchive[1] presumably in order to introduce intentional vulnerability reminded me that the MariaDB code base still has a plenty unsafe sprint/printf/fprintf use that can easily be found with scanners such as Flawfinder[2] and cppcheck[3]. There are currently 6 merge requests open by two authors (CC'd) to fix some of these issues[4]. Could we please have some more attention on these by the core contributors? If core contributors are not happy with the submissions, could you perhaps write your own safe functions (there are already some in m_string.h[5]) like many other projects seem to have (also libarchive had[6]) and then ask all contributors to use them consistently? Use of specific memory safe functions could also be mandated via the coding standards[7]. [1] https://github.com/libarchive/libarchive/pull/1609 [2] https://github.com/MariaDB/server/blob/11.5/.gitlab-ci.yml#L461-L489 [3] https://github.com/MariaDB/server/blob/11.5/.gitlab-ci.yml#L522-L554 [4] https://github.com/MariaDB/server/pulls?q=is%3Apr+is%3Aopen+sprintf+ [5] https://github.com/MariaDB/server/blob/11.5/include/m_string.h [6] https://github.com/libarchive/libarchive/blob/6110e9c82d8ba830c3440f36b9904… [7] https://github.com/MariaDB/server/blob/11.5/CODING_STANDARDS.md

2 3

question about Revision #f24038b851
by ShawnYan 04 Apr '24

04 Apr '24

hi, there, when I check release notes, https://mariadb.com/kb/en/mariadb-10419-changelog/ <https://mariadb.com/kb/en/mariadb-10419-changelog/ > I notice this commit record, Revision #f24038b851 2021-03-07 14:06:01 +0100 mark Aria allocations for temp tables as MY_THREAD_SPECIFIC but, it can not find in the jira system, can't learn more detail/background about this commit, could anyone help with it, may we know which task refer to this commit? https://github.com/MariaDB/server/commit/f24038b851 <https://github.com/MariaDB/server/commit/f24038b851 > Thank you. hi, there, when I check release notes, https://mariadb.com/kb/en/mariadb-10419-changelog/ <https://mariadb.com/kb/en/mariadb-10419-changelog/ > I notice this commit record, Revision #f24038b851 2021-03-07 14:06:01 +0100 mark Aria allocations for temp tables as MY_THREAD_SPECIFIC but, it can not find in the jira system, can't learn more detail about this commit, could anyone help with it, may we know which task refer to this commit? https://github.com/MariaDB/server/commit/f24038b851 <https://github.com/MariaDB/server/commit/f24038b851 > Thank you.

2 1

Re: [PATCH 0/2] Suggestion for smaller fix to XA parallel replication performance, MDEV-31949
by Kristian Nielsen 19 Mar '24

19 Mar '24

Andrei Elkin <andrei.elkin(a)mariadb.com> writes: > In normal cases, like that of not so many duplicate xids, the size > of maybe_active_xid must remain constrained because of "continuous" > relocation of elements from the tail to replace garbage (normally found > in head indexes). > Nevertheless as the array can be dynamically re-allocated to reflect some > some spike in the load, I suggest in > > origin/review__knielsen_xa_sched_minimal_fix > > to take care of that. I think your suggestion in review__knielsen_xa_sched_minimal_fix is fine. I'm not sure exactly how large the array can grow in pathological cases. As you wrote, in most normal cases we would expect it to not grow that much. But shrinking it if necessary surely cannot hurt. Thanks, - Kristian.

1 0

MariaDB topic on GitHub
by Otto Kekäläinen 19 Mar '24

19 Mar '24

Hi! I was browsing the GitHub topic 'mariadb' at https://github.com/topics/mariadb and to my surprise MariaDB itself isn't in that list at all. This seems to be due to https://github.com/MariaDB/server not having any topics defined at all. I recommend you add in the GitHub settings of https://github.com/MariaDB/server at least the topic 'mariadb', and while at it, also add link to mariadb.org. More ideas on how to improve the look and features of the GitHub page can be found by browsing projects like https://github.com/pulsar-edit and https://github.com/prisma which have all their GitHub account metadata polished. - Otto

1 0

Re: [PATCH] MDEV-33551: Semi-sync Wait Point AFTER_COMMIT Slow on Workloads with Heavy Concurrency
by Kristian Nielsen 18 Mar '24

18 Mar '24

> From: Brandon Nesterenko <brandon.nesterenko(a)mariadb.com> > > When using semi-sync replication with > rpl_semi_sync_master_wait_point=AFTER_COMMIT, the performance of the > primary can significantly reduce compared to AFTER_SYNC's > performance for workloads with many concurrent users executing > transactions. This is because all connections on the primary share Hi Brandon, Here is my second round review of the MDEV-33551 patch. I think these comments of mine from first round still need to be addressed: > > A bit of care must be taken about lifetime of Tranx_node and THD > > respectively (in case one goes away but not the other). But the whole > > > +void Repl_semi_sync_master::await_all_slave_replies() > > > + wait_result= cond_timewait(front.thd, &abstime); > > > > I'm wondering, is it safe here to wait on front.thd->COND_wakeup_ready? > > Isn't there a possibility that this might go away during the wait? I tried a simple testcase (attached) which disconnects the session thread while await_all_slave_replies() is running during shutdown. As I expected, I could see the COND_wakeup_ready of the session thread being de-allocated while await_all_slave_replies() is still waking up from that condition. That cannot be correct. Similarly, it seems clearly possible that a session thread is killed and exits while its entry is still present in the m_active_tranxs list, which will similarly lead to signalling a de-allocated COND_wakeup_ready. When a thread aborts the wait before the entry is removed, I think it should simply look up its entry (can use the m_trx_htb hash table, or even just iterate the list) and set its THD to NULL (or remove the entry, whatever is simpler). For the await_all_slave_replies() problem, I think the whole wait-at-shutdown is much too complex as it is now. The whole is_thd_awaiting_semisync_ack(), n_threads_awaiting_ack, special-case not killing threads waiting for semi-sync-ack, and now introducing unmark_thd_awaiting_ack. And it still has the bug waiting on a COND_wakeup_ready of another THD that can disappear during the wait. I think all of that should just go away, the wait in await_all_slave_replies() should be done much simpler with a dedicated condition variable to wait for. Eg. await_all_slave_replies() just does a mysql_cond_wait(COND_binlog_send) under LOCK_binlog until the last binlog position is ack'ed, and this condition is then signalled from report_reply_binlog() (either always, or await_all_slave_replies() can set a flag to request signal). Or have the shutdown thread enqueue its THD at the end of the m_active_tranxs list. Let's avoid all this complex interaction between SHUTDOWN WAIT FOR ALL SLAVES and session threads. BTW, COND_binlog_send is currently unused in your patch. Below detailed comments on individual parts of the patch: > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -1750,18 +1750,12 @@ static void close_connections(void) > - if (shutdown_wait_for_slaves && repl_semisync_master.get_master_enabled()) > + if (shutdown_wait_for_slaves && repl_semisync_master.get_master_enabled() && > + repl_semisync_master.sync_get_master_wait_sessions()) Do you need to check sync_get_master_wait_sessions() > 0 here? Why not wait unconditionally for the last binlog position sent? > --- a/sql/semisync_master.cc > +++ b/sql/semisync_master.cc > @@ -68,6 +68,14 @@ static ulonglong timespec_to_usec(const struct timespec *ts) > return (ulonglong) ts->tv_sec * TIME_MILLION + ts->tv_nsec / TIME_THOUSAND; > } > > +int signal_waiting_transaction(THD *waiting_thd, const char *binlog_file, > + my_off_t binlog_pos) > +{ > + if (waiting_thd->is_awaiting_semisync_ack) > + mysql_cond_signal(&waiting_thd->COND_wakeup_ready); The check on is_awaiting_semi_sync_ack seems wrong here. Normally, the THD will be waiting, otherwise it would not be in the list in the first place, so faster with an occasional redundant signal than a conditional always. And if the THD is not waiting, it's not safe to access it anyway. Instead, as mentioned above, have a check for thd != NULL here, and let a non-waiting thread clear the THD in its entry before it releases the LOCK_binlog. > -int Repl_semi_sync_master::cond_timewait(struct timespec *wait_time) > +int Repl_semi_sync_master::cond_timewait(THD *thd, struct timespec *_wait_time) > + if (_wait_time == NULL) > + { > + create_timeout(&gen_wait_time, NULL); > + real_wait_time= &gen_wait_time; > + } > + else > + { > + real_wait_time= _wait_time; > + } Don't pass in booleans that are always hard-coded true or false in the caller. Instead create two different functions, one that takes a time_spec, and one that does not (the latter creating it and calling the former). > @@ -533,7 +559,8 @@ void Repl_semi_sync_master::remove_slave() > Signal transactions waiting in commit_trx() that they do not have to > wait anymore. > */ > - cond_broadcast(); > + m_active_tranxs->clear_active_tranx_nodes(NULL, NULL, > + signal_waiting_transaction); This does not compile on my machine (the second NULL is passed to my_off_t position), is that a last-minute change that was never tested? Or NULL is defined differently on your machine, so it doesn't fail to compile? BTW, question about the old code: Why didn't the old code clear the m_active_tranxs list? Was it a bug? Or was it cleared elsewhere before (and if so where)? > -void Repl_semi_sync_master::await_slave_reply() > +void Repl_semi_sync_master::await_all_slave_replies() > + The lock is taken per iteration rather than over the whole loop to allow > + more opportunity for the user threads to handle their ACKs. That sounds like a mis-understanding. The lock is always released by mysql_cond_timedwait() in each iteration of the loop, no need to release/take it again for just a couple instructions. > + while (TRUE) > + { > + lock(); > + if (m_active_tranxs->is_empty() || !get_master_enabled() || !is_on()) > + { > + unlock(); > + break; > + } > + wait_result= m_active_tranxs->for_front( > + [](THD *thd, const char *binlog_file, my_off_t binlog_pos) -> int { > + return (thd->is_awaiting_semisync_ack) > + ? repl_semisync_master.cond_timewait(thd, NULL) > + : 0; > + }); As I wrote at the top, it is not safe to wait on the COND_wakeup_ready of another THD, since that THD might go away during the wait. Instead, let's do a specific wait for the end position here, either on COND_binlog or by enqueing an extra THD at the end. (And btw, in your code, why wait for the _front_ of the list and not the _rear_?) > diff --git a/sql/semisync_master.h b/sql/semisync_master.h > index 99f46869354..1eb690b572b 100644 > --- a/sql/semisync_master.h > +++ b/sql/semisync_master.h > > + * The pre_delete_hook parameter is a function pointer that will be invoked > + * for each Active_tranx node, in order, from m_trx_front to log_file_name, > + * e.g. to signal their wakeup condition. Repl_semi_sync_binlog::LOCK_binlog > + * is held while this is invoked. "from m_trx_front to log_file_name", I don't understand? (Typo s/log_file_name/m_trx_rear/ ?) > +/* > + Element in Repl_semi_sync_master::wait_queue to preserve the state of a > + transaction waiting for an ACK. > +*/ > +typedef struct _semisync_wait_trx { > + const char *binlog_name; > + my_off_t binlog_pos; > + THD *thd; > +} semisync_wait_trx_t; > + This is not longer used (should be removed). > * Input: (the transaction events' ending binlog position) > * trx_wait_binlog_name - (IN) ending position's file name > * trx_wait_binlog_pos - (IN) ending position's file offset > + * unmark_thd_awaiting_ack - (IN) Whether or not to unmark > + * thd->is_awaiting_semisync_ack after receiving > + * an ACK from the replica. If using wait_point > + * AFTER_SYNC with binlog_group_commit, only the > + * last transaction written to binlog in the > + * group should negate the boolean, because the > + * same thread (i.e. leader thread) will wait for > + * all transaction ACKs. Negating it too early > + * would break SHUTDOWN WAIT FOR ALL SLAVES, as > + * that is the condition tested to bypass killing > + * threads in phase 1. In all other situations, > + * the boolean should be negated immediately. That seems unnecessarily convoluted. I think the real problem here is that write_tranx_in_binlog() sets the is_awaiting_semi_sync_ack flag on the wrong THD, on the waiter_thd instead of the trx_thd. But once we fix the await_all_slave_replies(), do we even need the is_awaiting_semi_sync_ack flag any more? If so, why? If not, then this point becomes moot anyway. - Kristian. --source include/have_innodb.inc --source include/have_debug.inc --source include/have_binlog_format_mixed.inc --source include/master-slave.inc --connection master CREATE TABLE t1 (a INT PRIMARY KEY, b INT) ENGINE=InnoDB; INSERT INTO t1 VALUES (1,0); --sync_slave_with_master source include/stop_slave.inc; SET @old_semi_enabled= @@GLOBAL.rpl_semi_sync_slave_enabled; SET GLOBAL rpl_semi_sync_slave_enabled = 1; --source include/start_slave.inc --connection master SET GLOBAL rpl_semi_sync_master_enabled= 1; SET GLOBAL rpl_semi_sync_master_timeout= 5000; let $status_var= Rpl_semi_sync_master_clients; let $status_var_value= 1; source include/wait_for_status_var.inc; show status like 'Rpl_semi_sync_master_status'; show status like 'Rpl_semi_sync_master_clients'; --connection slave SET @old_dbug= @@GLOBAL.debug_dbug; SET GLOBAL debug_dbug="+d,simulate_delay_semisync_slave_reply"; --connect(con1, localhost, root,,) --connect(con2, localhost, root,,) --connection con1 send INSERT INTO t1 VALUES (2,0); --sleep 0.3 --disconnect con1 --connection default --write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect wait EOF #let $status_var= Rpl_semi_sync_master_wait_sessions; #let $status_var_value= 1; #source include/wait_for_status_var.inc; --sleep 0.1 --connection con2 SHUTDOWN WAIT FOR ALL SLAVES; --disconnect con2 --connection default --source include/wait_until_disconnected.inc --connection master --source include/wait_until_disconnected.inc --connection server_1 --source include/wait_until_disconnected.inc --connection default --append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect restart EOF --enable_reconnect --source include/wait_until_connected_again.inc --connection master --enable_reconnect --source include/wait_until_connected_again.inc --connection server_1 --enable_reconnect --source include/wait_until_connected_again.inc --save_master_pos --connection slave SET GLOBAL debug_dbug= @old_dbug; SET GLOBAL rpl_semi_sync_slave_enabled= @old_semi_enabled; --source include/stop_slave.inc --source include/start_slave.inc --sync_with_master --connection default DROP TABLE t1; --source include/rpl_end.inc

2 3

Re: e6cacca0b39: MDEV-23729 MDEV-32218 INFORMATION_SCHEMA table for user data
by Sergei Golubchik 15 Mar '24

15 Mar '24

Hi, Nikita, no test for FLUSH PRIVILEGES no test for a user with manually expired password doesn't compile in embedded hostname 'neo' is replaced, but not everywhere I'll try to fix it myself, but no guarantees On Mar 15, Nikita Malyavin wrote: > revision-id: e6cacca0b39 (mariadb-11.4.1-10-ge6cacca0b39) > parent(s): 929c2e06aae > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2024-03-15 15:32:45 +0100 > message: > > MDEV-23729 MDEV-32218 INFORMATION_SCHEMA table for user data > > * A new table INFORMATION_SCHEMA.USERS is introduced. > * It stores auxiliary user data > * An unprivileged user can access their own data, and that is the main > difference with what mysql.global_priv provides > * The fields are currently: USER, PASSWORD_ERRORS, PASSWORD_EXPIRATION_TIME > * If password_errors is ignored for the user, PASSWORD_ERRORS is NULL > * PASSWORD_EXPIRATION_TIME is a timestamp with exact point in time, calculated > from password_last_changed and password_lifetime (i.e. days) stored for the user > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 14450a5a610..a87b04f3d39 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc ... > +int fill_users_schema_table(THD *thd, TABLE_LIST *tables, COND *cond) > +{ > + int res= 0; > +#ifndef NO_EMBEDDED_ACCESS_CHECKS > + bool see_whole_table= check_access(thd, SELECT_ACL, "mysql", NULL, NULL, > + true, true) == 0; > + TABLE *table= tables->table; > + > + if (!see_whole_table) > + { > + mysql_mutex_lock(&acl_cache->lock); > + ACL_USER *cur_user= find_user_exact(thd->security_ctx->priv_host, > + thd->security_ctx->priv_user); > + if (!cur_user) > + { > + mysql_mutex_unlock(&acl_cache->lock); > + my_error(ER_INVALID_CURRENT_USER, MYF(0)); Hmm. Questionable. I'd just return an empty result set here, I_S tables normally don't throw errors. But this is an exceptional situation and I cannot say that an error is wrong here, let's keep it your way. > + return 1; > + } > + > + res= fill_users_schema_record(thd, table, cur_user); > + mysql_mutex_unlock(&acl_cache->lock); > + return res; > + } > + > + mysql_mutex_lock(&acl_cache->lock); > + for (size_t i= 0; res == 0 && i < acl_users.elements; i++) > + { > + ACL_USER *user= dynamic_element(&acl_users, i, ACL_USER*); > + res= fill_users_schema_record(thd, table, user); > + } > + mysql_mutex_unlock(&acl_cache->lock); > +#endif > + return res; > +} > + Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: [PATCH 0/2] Suggestion for smaller fix to XA parallel replication performance, MDEV-31949
by Kristian Nielsen 13 Mar '24

13 Mar '24

Andrei Elkin <andrei.elkin(a)mariadb.com> writes: > which makes a worker with dependent pieces of job be *effectively* placed at the > tail of the available worker ('fifo') queue. Indeed. And this BTW is why a simple sliding window of size 2*workers is not enough, because in principle a worker can sit in the fifo for many transactions without being scheduled, if there are many dependency restrictions. I coded the thing with "generations" as a quick way to handle this and have a way to clean out old dependency information. This could of course be improved in many ways. Maybe remember the two last XID for every worker and have a hash table mapping each XID to its worker, or something. But this is just a PoC. > No you are not. Thanks for the catch! To excuse myself it was supposed > to be a poc that I committed hastedly. Yes yes, no problem, I understand, it's good to do poc. I was just momentarily confused, but the intent is clear, thanks. - Kristian.

1 0

Re: a473db9e4fe: MDEV-25829 Change default collation to utf8mb4_1400_ai_ci
by Sergei Golubchik 13 Mar '24

13 Mar '24

Hi, Alexander, On Mar 08, Alexander Barkov wrote: > commit a473db9e4fe > Author: Alexander Barkov <bar(a)mariadb.com> > Date: Thu Nov 2 14:16:09 2023 +0400 > > MDEV-25829 Change default collation to utf8mb4_1400_ai_ci "utf8mb4_uca1400_ai_ci". It's still not too late to rename the MDEV and change commits to match. I'm not reviewing InnoDB changes, I hope Marko did > > Step#3 The main patch > > diff --git a/mysql-test/include/ctype_utf8mb4.inc b/mysql-test/include/ctype_utf8mb4.inc > index 436b0f2782f..73011cef81d 100644 > --- a/mysql-test/include/ctype_utf8mb4.inc > +++ b/mysql-test/include/ctype_utf8mb4.inc > @@ -58,11 +58,11 @@ select CONVERT(_koi8r' > # "a\0" < "a" > # "a\0" < "a " > > -SELECT 'a' = 'a '; > -SELECT 'a\0' < 'a'; > -SELECT 'a\0' < 'a '; > -SELECT 'a\t' < 'a'; > -SELECT 'a\t' < 'a '; > +SELECT 'a' = 'a ' collate utf8mb4_general_ci; > +SELECT 'a\0' < 'a' collate utf8mb4_general_ci; > +SELECT 'a\0' < 'a ' collate utf8mb4_general_ci; > +SELECT 'a\t' < 'a' collate utf8mb4_general_ci; > +SELECT 'a\t' < 'a ' collate utf8mb4_general_ci; 1. what will happen in utf8mb4_uca1400_ai_ci? 2. if you specifically want this file to test utf8mb4_general_ci, you can do SET NAMES, cannot you? Once for the whole file. Or set character_set_collations > > # > # The same for binary collation > diff --git a/mysql-test/main/ctype_euckr.result b/mysql-test/main/ctype_euckr.result > index 9e030f9cc6d..8de90f89a26 100644 > --- a/mysql-test/main/ctype_euckr.result > +++ b/mysql-test/main/ctype_euckr.result > @@ -2244,7 +2244,7 @@ FE80 > DELETE FROM t2 WHERE a='?'; > ALTER TABLE t2 ADD u VARCHAR(1) CHARACTER SET utf8, ADD a2 VARCHAR(1) CHARACTER SET euckr; > UPDATE IGNORE t2 SET u=a, a2=u; > -SELECT s as unassigned_code FROM t2 WHERE u='?'; > +SELECT s as unassigned_code FROM t2 WHERE u=binary'?'; why? > unassigned_code > A2E8 > A2E9 > diff --git a/mysql-test/main/ctype_like_range.result b/mysql-test/main/ctype_like_range.result > index e5e5e61126b..fa694b0c250 100644 > --- a/mysql-test/main/ctype_like_range.result > +++ b/mysql-test/main/ctype_like_range.result > @@ -284,7 +284,7 @@ id name val > 32 mn 63616161616161616161616161616161 > 32 mx 63616161616161616161616161616161 > 32 sp -------------------------------- > -ALTER TABLE t1 MODIFY a VARCHAR(32) CHARACTER SET utf8; > +ALTER TABLE t1 MODIFY a VARCHAR(32) CHARACTER SET utf8 COLLATE utf8_general_ci; do you have tests for ctype_like_range with utf8mb3_uca1400_ai_ci ? > INSERT INTO t1 (a) VALUES (_ucs2 0x0425),(_ucs2 0x045F); > INSERT INTO t1 (a) VALUES (_ucs2 0x2525),(_ucs2 0x5F5F); > SELECT * FROM v1; > diff --git a/mysql-test/main/ctype_utf8.test b/mysql-test/main/ctype_utf8.test > index a875fe51f3a..2f56d10bccf 100644 > --- a/mysql-test/main/ctype_utf8.test > +++ b/mysql-test/main/ctype_utf8.test > @@ -24,7 +24,7 @@ drop database if exists mysqltest; > --disable_warnings > drop table if exists t1,t2; > --enable_warnings > -set names utf8; > +set names utf8 collate utf8_general_ci; why not to change the test to work with utf8mb3_uca1400_ai_ci? > > select left(_utf8 0xD0B0D0B1D0B2,1); > select right(_utf8 0xD0B0D0B2D0B2,1); > diff --git a/mysql-test/main/dyncol.test b/mysql-test/main/dyncol.test > index 493e9b3842d..ba302861e03 100644 > --- a/mysql-test/main/dyncol.test > +++ b/mysql-test/main/dyncol.test > @@ -10,10 +10,10 @@ > --echo # column create > --echo # > select hex(COLUMN_CREATE(1, NULL AS char character set utf8)); > -select hex(COLUMN_CREATE(1, "afaf" AS char character set utf8)); > -select hex(COLUMN_CREATE(1, 1212 AS char character set utf8)); > -select hex(COLUMN_CREATE(1, 12.12 AS char character set utf8)); > -select hex(COLUMN_CREATE(1, 99999999999999999999999999999 AS char character set utf8)); > +select hex(COLUMN_CREATE(1, "afaf" AS char character set utf8 collate utf8_general_ci)); > +select hex(COLUMN_CREATE(1, 1212 AS char character set utf8 collate utf8_general_ci)); > +select hex(COLUMN_CREATE(1, 12.12 AS char character set utf8 collate utf8_general_ci)); > +select hex(COLUMN_CREATE(1, 99999999999999999999999999999 AS char character set utf8 collate utf8_general_ci)); why? > select hex(COLUMN_CREATE(1, NULL AS unsigned int)); > select hex(COLUMN_CREATE(1, 1212 AS unsigned int)); > select hex(COLUMN_CREATE(1, 7 AS unsigned int)); > diff --git a/mysql-test/main/func_json.result b/mysql-test/main/func_json.result > index ab35822a6ce..fce2caa2842 100644 > --- a/mysql-test/main/func_json.result > +++ b/mysql-test/main/func_json.result > @@ -414,6 +414,13 @@ select json_object('foo', json_unquote(json_object('bar', c)),'qux', c) as fld f > fld > {"foo": "{\"bar\": \"abc\"}", "qux": "abc"} > {"foo": "{\"bar\": \"def\"}", "qux": "def"} > +create table t2 as select json_object('foo', json_unquote(json_object('bar', c)),'qux', c) as fld from t1 limit 0; > +show create table t2; > +Table Create Table > +t2 CREATE TABLE `t2` ( > + `fld` varchar(39) DEFAULT NULL > +) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci > +drop table t2; why do you test here? > drop table t1; > select json_object("a", json_object("b", "abcd")); > json_object("a", json_object("b", "abcd")) > diff --git a/mysql-test/main/func_regexp_pcre.test b/mysql-test/main/func_regexp_pcre.test > index 77f5af6e0ff..7cf9e8007de 100644 > --- a/mysql-test/main/func_regexp_pcre.test > +++ b/mysql-test/main/func_regexp_pcre.test > @@ -23,17 +23,17 @@ SELECT 'Ã ' RLIKE '\\x{00C0}' COLLATE utf8_bin; > SELECT 'Ã' RLIKE '\\x{00C0}' COLLATE utf8_bin; > > # Checking how (?i) and (?-i) affect case sensitivity > -CREATE TABLE t1 (s VARCHAR(10) CHARACTER SET utf8); > +CREATE TABLE t1 (s VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_general_ci); why? > INSERT INTO t1 VALUES ('a'),('A'); > -CREATE TABLE t2 (p VARCHAR(10) CHARACTER SET utf8); > +CREATE TABLE t2 (p VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_general_ci); > INSERT INTO t2 VALUES ('a'),('(?i)a'),('(?-i)a'),('A'),('(?i)A'),('(?-i)A'); > SELECT s,p,s RLIKE p, s COLLATE utf8_bin RLIKE p FROM t1,t2 ORDER BY BINARY s, BINARY p; > DROP TABLE t1,t2; > > > # Checking Unicode character classes > -CREATE TABLE t1 (ch VARCHAR(22)) CHARACTER SET utf8; > -CREATE TABLE t2 (class VARCHAR(32)) CHARACTER SET utf8; > +CREATE TABLE t1 (ch VARCHAR(22)) CHARACTER SET utf8 COLLATE utf8_general_ci; > +CREATE TABLE t2 (class VARCHAR(32)) CHARACTER SET utf8 COLLATE utf8_general_ci; and here > INSERT INTO t1 VALUES ('Ð¯'),('Î£'),('A'),('Ã'); > INSERT INTO t1 VALUES ('Ñ'),('Ï'),('a'),('Ã '); > INSERT INTO t1 VALUES ('ã'),('ê°·'),('à¶´'); > diff --git a/mysql-test/main/mrr_icp_extra.result b/mysql-test/main/mrr_icp_extra.result > index 8f6ee88acc6..d7cde103288 100644 > --- a/mysql-test/main/mrr_icp_extra.result > +++ b/mysql-test/main/mrr_icp_extra.result > @@ -754,7 +754,7 @@ t1 CREATE TABLE `t1` ( > KEY `t` (`t`(5)) > ) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci > drop table t1; > -create table t1 (v char(10) character set utf8); > +create table t1 (v char(10) character set utf8 collate utf8_general_ci); why here? > show create table t1; > Table Create Table > t1 CREATE TABLE `t1` ( > diff --git a/mysql-test/main/myisam.test b/mysql-test/main/myisam.test > index 1a20f97a54f..4a283737122 100644 > --- a/mysql-test/main/myisam.test > +++ b/mysql-test/main/myisam.test > @@ -1039,7 +1039,7 @@ create table t1 (v varchar(65536)); > show create table t1; > drop table t1; > set statement sql_mode = 'NO_ENGINE_SUBSTITUTION' for > -create table t1 (v varchar(65530) character set utf8); > +create table t1 (v varchar(65530) character set utf8 collate utf8_general_ci); why? collation doesn't matter here, does it? > show create table t1; > drop table t1; > > diff --git a/mysql-test/main/order_by.result b/mysql-test/main/order_by.result > index 274f29e34dc..85059fb3370 100644 > --- a/mysql-test/main/order_by.result > +++ b/mysql-test/main/order_by.result > @@ -4142,7 +4142,7 @@ drop table t1; > # > # MDEV-21922: Allow packing addon fields even if they don't honour max_length_for_sort_data > # > -create table t1 (a varchar(200) character set utf8, b int); > +create table t1 (a varchar(200) character set utf8 collate utf8_general_ci, b int); why? I think I see too many forced utf8_general_ci in tests and it's difficult to believe that the collation actually matters in all these cases. > insert into t1 select seq, seq from seq_1_to_10; > select * from t1 order by a; > a b > diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc > index 508ea9f644e..5731ad1f04b 100644 > --- a/sql/item_jsonfunc.cc > +++ b/sql/item_jsonfunc.cc > @@ -842,7 +842,7 @@ String *Item_func_json_quote::val_str(String *str) > bool Item_func_json_unquote::fix_length_and_dec(THD *thd) > { > collation.set(&my_charset_utf8mb3_general_ci, > - DERIVATION_COERCIBLE, MY_REPERTOIRE_ASCII); > + DERIVATION_CAST, MY_REPERTOIRE_ASCII); why? I guess that's why you have json_unquote test above. > max_length= args[0]->max_length; > set_maybe_null(); > return FALSE; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1