MDEV-40996 Support --sync_with_master 0, $variable in mysqltest - #5619
MDEV-40996 Support --sync_with_master 0, $variable in mysqltest#5619ParadoxV5 wants to merge 1 commit into
--sync_with_master 0, $variable in mysqltest#5619Conversation
|
@ParadoxV5 mtr/mariadb-test changes can (and should) go into the earliest version, 10.11. They don't affect the server or production use, and it's rather annoying to remember what mtr feature was added in what version and adjust tests when merging up. Better keep mtr/mariadb-test the same everywhere whenever possible |
153ee96 to
8b8d062
Compare
ParadoxV5
changed the title
--sync_with_master 0, $variable--sync_with_master 0, $variable in mysqltest
ParadoxV5
added
the
Rework
8b8d062 to
d557ea8
Compare
|
verified under ASAN, the test fails without the patch and passes with it. I amended two cosmetic things: declared |
`--sync_with_master` uses `get_string()`, which has `$variable` support, but it only uses the read buffer, which is written with the unexpanded string and not the variable value. Reviewed-by: KhaledR57 <khaled57.dev@gmail.com>
d557ea8 to
9622f5f
Compare
|
@vuvova approved |
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation correctly handles variable expansion and allocation cleanup with suitable regression coverage.
Pull request overview
Adds $variable expansion support for named connections in --sync_with_master.
Changes:
- Uses
get_string()’s returned expanded value. - Preserves the allocation base for correct cleanup.
- Adds literal and variable-based replication tests.
File summaries
| File | Description |
|---|---|
client/mysqltest.cc |
Correctly consumes expanded string values. |
mysql-test/suite/rpl/t/mysqltest_sync_with_master_named.test |
Tests named connection synchronization. |
mysql-test/suite/rpl/r/mysqltest_sync_with_master_named.result |
Adds expected test output. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
midenok
left a comment
midenok
left a comment
There was a problem hiding this comment.
Do not create new files per one test case, concat to existing files if possible. Ok to push.
--sync_with_masterusesget_string(), which has$variablesupport, but it only uses the read buffer, which is written with the unexpanded string and not the variable value.