MDEV-28911: Add --init-command option to mysqldump#4723
MDEV-28911: Add --init-command option to mysqldump#4723HNOONa-0 wants to merge 1 commit intoMariaDB:mainfrom
Conversation
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
Please consider extending the MDEV text with a full functional description of the new option so it could be documented well.
Also, please make sure the text of your commit message complies with CODING_STANDARDS.md.
Some answers to your questions below.
|
failed tests work fine on my machine |
gkodinov
left a comment
There was a problem hiding this comment.
LGTM. Thank you for taking the extra effort to implement the common headers!
Stand by for the final review.
c9620bd to
3ac70a9
Compare
Replaced redundant declarations with a unified include in mysql clients This prevents variable inconsistencies such as with the init command
|
Hello @gkodinov, I've implemented some of the modifications you suggested. For now, I've only applied them to a single client (mysql.cc) as a proof of concept. If you're happy with this selection of variables, I will apply them to other client files and maybe add a couple of tests too. |
Description
A quick note on the implementation: I set it up so the command runs before mariadb-dump initializes its own default session settings. I originally thought it made more sense to put it after (so users could override the tool's internal defaults), but looking at the rest of the codebase, doing it before seems to be the established pattern. I decided to stick with the existing convention to keep things consistent. I also added a small MTR test file to verify the flag works as expected. Do you agree with this order of execution?
Right now, only the last
init-commandoption will run. Is it desirable to allow users to specify multipleinitcommands?Release Notes
Add --init-command option to
mysqldumpHow can this PR be tested?
Basing the PR against the correct MariaDB version
mainbranch.PR quality check