MDEV-40353: Add eval parameter to write_file and append_file by KhaledR57 · Pull Request #5382 · MariaDB/server

KhaledR57 · 2026-07-13T20:58:50Z

The Jira issue number for this PR is: MDEV-40353

Description

write_file and append_file substitute variables in the file name, but not in the content, which is written byte for byte. Tests that need a computed value in a file have to fall back on --exec echo ... >> file.

This adds an optional third parameter, eval, which sends the content through the same substitution as --echo and let, so $var and $(expr) are substituted and \$ writes a literal $.

write_file <file_name> [<delimiter> [eval]];
<what to write line 1>
<...>
<what to write line n>
<delimiter>

Example:

let $version= 10.6;

--write_file $MYSQLTEST_VARDIR/tmp/my.cnf EOF eval
server-id=$(6+7)
comment=version $version
EOF

writes server-id=13 and comment=version 10.6.

How can this PR be tested?

./mtr main.mysqltest

Basing the PR against the correct MariaDB version

This is a new feature or a refactoring, and the PR is based against the main branch.
This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

gemini-code-assist

Code Review

This pull request introduces support for an optional 'eval' argument to the 'write_file' and 'append_file' commands in 'mysqltest', enabling variable and expression substitution within the written or appended content. It also adds comprehensive test cases and updates the command documentation. Feedback on the changes suggests improving code readability by extracting the assignment out of the 'if' condition when checking the 'eval' flag.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-13T21:00:40Z

+  if ((eval_content= (ds_eval_flag.length != 0)) &&
+      strcasecmp(ds_eval_flag.str, "eval"))
+    die("Invalid argument '%s' to '%.*b', only 'eval' is allowed",
+        ds_eval_flag.str, command->first_word_len, command->query);


Assigning a value within an if condition can reduce code readability and make it harder to maintain or debug. It is cleaner and less error-prone to perform the assignment beforehand and then evaluate the boolean variable in the condition.

eval_content= (ds_eval_flag.length != 0); if (eval_content && strcasecmp(ds_eval_flag.str, "eval")) die("Invalid argument '%s' to '%.*b', only 'eval' is allowed", ds_eval_flag.str, command->first_word_len, command->query);

Copilot

Pull request overview

This PR extends the mysqltest DSL commands write_file and append_file with an optional third argument, eval, allowing variable ($var) and expression ($(...)) substitution within the written file content (previously only the filename was evaluated). This improves test ergonomics by avoiding --exec echo ... >> file when generating computed file contents.

Changes:

Add an optional eval flag to write_file / append_file to run the content through do_eval() before writing.
Validate that the third argument (if provided) is exactly eval.
Add regression coverage in mysqltest.test and update expected output in mysqltest.result.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
mysql-test/main/mysqltest.test	Adds a new MDEV-40353 regression section covering literal vs evaluated content, custom delimiters, loop behavior, and error cases.
mysql-test/main/mysqltest.result	Updates expected output to reflect the new `eval` behavior and new regression section.
client/mysqltest.cc	Implements the optional `eval` argument for `write_file`/`append_file` and documents the updated syntax and behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sanja-byelkin

Generally good but need small changes

sanja-byelkin · 2026-07-15T11:56:36Z

+--write_file $file
+plain $var and $(1+1)
+a lone dollar sign: $
+a backslash: \\! echo foo


Add this line to the eval version (actually it would be better to have exactly the same content here and in eval to see the difference)

sanja-byelkin · 2026-07-15T11:57:15Z

+--error 1
+--exec echo "--write_file $file EOF junk" | $MYSQL_TEST 2>&1
+
 --echo End of tests


please change on

--echo End of 10.6 tests

sanja-byelkin · 2026-07-15T11:59:16Z

+    {
+      /* Evaluate on every execution, keep command->content unevaluated */
+      DYNAMIC_STRING ds_eval_content;
+      init_dynamic_string(&ds_eval_content, "", ds_content.length + 256, 256);


Add result of init_dynamic_string check (I know it is called many cases uncheck but let us make the code better)

sanja-byelkin · 2026-07-15T12:12:47Z


+  if ((eval_content= (ds_eval_flag.length != 0)) &&
+      strcasecmp(ds_eval_flag.str, "eval"))
+    die("Invalid argument '%s' to '%.*b', only 'eval' is allowed",


in other code used report_or_die for parameter checks (and so it can skip incorrect parameter)

mysqltest substituted variables in the file name of write_file and append_file, but not in the content, so tests had to fall back to "exec echo ... >> file" to put a computed value into a file. Add an optional third parameter, eval, which makes the content go through do_eval(). It is off by default, because existing tests write content that contains a literal $ or backslash.

sanja-byelkin

Fix 2 small issue and OK to push (10.6 is protected so prepare the fix and tell me I will push)

sanja-byelkin · 2026-07-16T11:30:53Z

+    DBUG_VOID_RETURN;
+  }
+
  if (bad_path(ds_filename.str))


IMHO it is better to check arguments in order of appearance so bad path should be checked first

sanja-byelkin · 2026-07-16T11:34:44Z

+--error 1
+--exec echo "--write_file $file EOF junk" | $MYSQL_TEST 2>&1
+
+--echo End of 10.6 tests


oops # is needed

-echo # End of 10.6 tests

KhaledR57 marked this pull request as draft July 13, 2026 20:59

gemini-code-assist Bot reviewed Jul 13, 2026

View reviewed changes

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Jul 14, 2026

KhaledR57 marked this pull request as ready for review July 14, 2026 12:37

gkodinov assigned gkodinov and unassigned gkodinov Jul 15, 2026

sanja-byelkin requested a review from Copilot July 15, 2026 11:47

Copilot started reviewing on behalf of sanja-byelkin July 15, 2026 11:47 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

sanja-byelkin requested changes Jul 15, 2026

View reviewed changes

KhaledR57 force-pushed the MDEV-40353-append-file-eval branch 3 times, most recently from 0326937 to 1d22668 Compare July 15, 2026 16:07

KhaledR57 requested a review from sanja-byelkin July 15, 2026 16:12

KhaledR57 force-pushed the MDEV-40353-append-file-eval branch from 1d22668 to 3e58b0f Compare July 15, 2026 16:24

KhaledR57 force-pushed the MDEV-40353-append-file-eval branch from 3e58b0f to aa4f470 Compare July 15, 2026 16:27

sanja-byelkin approved these changes Jul 16, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

MDEV-40353: Add eval parameter to write_file and append_file#5382

MDEV-40353: Add eval parameter to write_file and append_file#5382
KhaledR57 wants to merge 1 commit into
MariaDB:10.6from
KhaledR57:MDEV-40353-append-file-eval

KhaledR57 commented Jul 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

sanja-byelkin left a comment

Uh oh!

sanja-byelkin Jul 15, 2026

Uh oh!

sanja-byelkin Jul 15, 2026

Uh oh!

sanja-byelkin Jul 15, 2026

Uh oh!

sanja-byelkin Jul 15, 2026

Uh oh!

sanja-byelkin left a comment

Uh oh!

sanja-byelkin Jul 16, 2026

Uh oh!

sanja-byelkin Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

KhaledR57 commented Jul 13, 2026

Description

How can this PR be tested?

Basing the PR against the correct MariaDB version

PR quality check

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sanja-byelkin left a comment

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin left a comment

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

sanja-byelkin Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants