-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MDEV-35948: Test OG replicate_*
s with long lists
#3795
base: 10.5
Are you sure you want to change the base?
Conversation
@@ -32,11 +36,23 @@ SET @@GLOBAL.replicate_wild_ignore_table="test.t, t2"; | |||
--error ER_WRONG_ARGUMENTS | |||
SET @@GLOBAL.replicate_wild_ignore_table="test.,t1"; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over-max-sized identifiers don’t cause errors. Possible reasons:
- 🐞
- It’s the user’s problem that they was filtering for/against impossible names.
- I made junior dev mistakes.
SET @name= REPEAT("t", 64); | |
SET @@GLOBAL.replicate_wild_ignore_table= CONCAT(@name, ".t"); | |
#--error ER_WRONG_ARGUMENTS? | |
SET @@GLOBAL.replicate_wild_ignore_table= CONCAT(@name, "1.t"); | |
SET @@GLOBAL.replicate_wild_ignore_table= CONCAT("t.", @name); | |
#--error ER_WRONG_ARGUMENTS? | |
SET @@GLOBAL.replicate_wild_ignore_table= CONCAT("t.", @name, "1"); | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't think that size of each identifier is ever validated (quickly looking into sys_vars.cc
, e.g. static Sys_var_rpl_filter Sys_replicate_wild_ignore_table
). Perhaps that would be a good validation to add 😇 (in due time)
SELECT GROUP_CONCAT(CONCAT("database_name.long_table_name_", seq) SEPARATOR ",") | ||
INTO @name FROM seq_1_to_8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MariaDB is awesome 😆
--source include/have_sequence.inc | ||
# have show_slave_status | ||
--source include/have_log_bin.inc | ||
CHANGE MASTER TO master_host='example.com'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the time was spent on sys_vars.replicate_wild_ignore_table_basic
(the other files simply modified from copy-pastes), and about half were spend working around how SHOW REPLICA STATUS is empty without CHANGE MASTER TO 😵 the hard way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only had a quick moment today to look over the tests, generally, they look good! I only left a few small notes.
Also, as to the PR plan, I'd say these should go in with PR #3772, and then once that makes it into 11.7
, we can backport it and extend the tests for the rest of the SHOW SLAVE STATUS
variables.
@@ -32,11 +36,23 @@ SET @@GLOBAL.replicate_wild_ignore_table="test.t, t2"; | |||
--error ER_WRONG_ARGUMENTS | |||
SET @@GLOBAL.replicate_wild_ignore_table="test.,t1"; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't think that size of each identifier is ever validated (quickly looking into sys_vars.cc
, e.g. static Sys_var_rpl_filter Sys_replicate_wild_ignore_table
). Perhaps that would be a good validation to add 😇 (in due time)
Assert that 1st-gen `replicate_*` filter variables display their input – including long but reasonable lists – correctly (without truncation) in * direct SELECT * [semi-new] INFORMATION_SCHEMA.GLOBAL_VARIABLES.VARIABLE_VALUE * [new] SHOW REPLICA STATUS This tests the fields affected by MDEV-35693 (an 11.6 oversignt). Also test the input `DEFAULT` as part of this edge-case testing.
dc590a4
to
3a7cbfa
Compare
Amend + Rebase diff --git a/mysql-test/suite/sys_vars/t/replicate_do_db_basic.test b/mysql-test/suite/sys_vars/t/replicate_do_db_basic.test
index a99429fe9a..4b72d14d6a 100644
--- a/mysql-test/suite/sys_vars/t/replicate_do_db_basic.test
+++ b/mysql-test/suite/sys_vars/t/replicate_do_db_basic.test
@@ -1,7 +1,7 @@
--source include/have_sequence.inc
# have show_slave_status
---source include/have_log_bin.inc
-CHANGE MASTER TO master_host='example.com';
+--source include/not_embedded.inc
+CHANGE MASTER TO master_host='127.0.0.1', master_user='root';
--let $status_items= Replicate_Do_DB
--echo #
@@ -58,5 +58,4 @@ SET @@GLOBAL.replicate_do_db=DEFAULT;
SELECT @@GLOBAL.replicate_do_db;
--echo # Cleanup.
-RESET REPLICA ALL;
SET @@GLOBAL.replicate_do_db = @save_replicate_do_db; CHANGE MASTER TO is simply required for some reason, even if it’s changing to the same configs. |
Changes should have accompanying tests. Assert that 1st-gen `replicate_*` filter variables display long but reasonable lists correctly (without truncation) in SHOW REPLICA STATUS. See that PR for details
Resize the types and widths of SHOW REPLICA STATUS (technically `INFORMATION_SCHEMA.SLAVE_STATUS`) columns to better match their possible values In case of intentionally but absurdly long lists, text columns that list an uncapped number of elements have expanded to accept as many bytes as we could support. Particularly, the first-gen `Replicate_` filters were incorrectly typed as singlular `Name()`s during MDEV-33526. Under `Name`s’ 64-char limit, they could overflow (read: truncate) even before their lengths got absurd. In response to `‘MAX_SLAVE_ERRMSG’ was not declared in this scope` in Embedded builds, a new `#ifdef HAVE_REPLICATION` guard wraps `slave_status_info` to skip this unused data in Replication-less builds. For testing, this commit forward-ports a modified cherry-pick of #3795 (the latter targets our oldest maintained LTS as part of MDEV-35948). > Assert that 1st-gen `replicate_*` filter variables display > their input – including long but reasonable lists – > correctly (without truncation) in > * direct SELECT > * [semi-new] INFORMATION_SCHEMA.GLOBAL_VARIABLES.VARIABLE_VALUE > * [new] SHOW REPLICA STATUS Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Resize the types and widths of SHOW REPLICA STATUS (technically `INFORMATION_SCHEMA.SLAVE_STATUS`) columns to better match their possible values In case of intentionally but absurdly long lists, text columns that list an uncapped number of elements have expanded to accept as many bytes as we could support. Particularly, the first-gen `Replicate_` filters were incorrectly typed as singlular `Name()`s during MDEV-33526. Under `Name`s’ 64-char limit, they could overflow (read: truncate) even before their lengths got absurd. In response to `‘MAX_SLAVE_ERRMSG’ was not declared in this scope` in Embedded builds, a new `#ifdef HAVE_REPLICATION` guard wraps `slave_status_info` to skip this unused data in Replication-less builds. For testing, this commit forward-ports a modified cherry-pick of #3795 (the latter targets our oldest maintained LTS as part of MDEV-35948). > Assert that 1st-gen `replicate_*` filter variables display > their input – including long but reasonable lists – > correctly (without truncation) in > * direct SELECT > * [semi-new] INFORMATION_SCHEMA.GLOBAL_VARIABLES.VARIABLE_VALUE > * [new] SHOW REPLICA STATUS Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Description
Assert that 1st-gen
replicate_*
filter variables display their input – including long but reasonable lists – correctly (without truncation) inThis tests the fields affected by MDEV-35693 (an 11.6 oversignt).
Also test the input
DEFAULT
as part of this edge-case testing.Release Notes
N/A – This is a dev task.
How can this PR be tested?
run the modified files
Basing the PR against the correct MariaDB version
This is a new feature or a refactoring, and the PR is based against themain
branch.PR quality check