MDEV-39223 Reversed executable comments incorrectly written in binlog

[tonychen2001]

Description

This addresses an issue introduced in #4724.

When a reversed executable comment (/*!!version */ or /*M!!version */)
is not executed on the master (server version >= specified version),
the binlog writer only neutralized the second '!' to a space, leaving
/*! command */, an executable comment the slave would then try to
execute.

Fix (non-executed path): also replace the first '!' with a space so the
SQL becomes /* (a plain comment). The restore path for unclosed comments
also restores both '!' characters.

Additionally, when a reversed executable comment IS executed on the
source (server version < specified version), the /!!version syntax was
preserved verbatim in the binlog. A replica with a higher version could
re-evaluate the reversed condition, fail it, and skip the command —
causing source/replica divergence. Unlike forward executable comments
(/!version */), where replication to same-or-higher versions guarantees
the replica also executes, reversed comments have the opposite semantics
and are not safe to preserve as-is.

Fix (executed path): overwrite the version digits with spaces in the raw
query buffer so the binlog contains /*!! (a non-versioned reversed
executable comment) which the replica always executes regardless of its
version.

Ultimately, if a reverse comment was executed on the source it must be executed on the replica regardless of
versioning. Similarly, if a reverse comment was not executed on the source it must not be executed on the replica regardless of versioning.

Additionally, we mark queries that are patched in-place as non-cacheable when query_cache_strip_comments=OFF.
The lexer rewrites the raw query buffer when it takes a reversed executable
comment (digits -> spaces) or skips a forward/reversed comment (the '!' ->
space). With query_cache_strip_comments=OFF the cache key reads from this
buffer, so the lookup key (built before parsing) and the store key (built
after) disagree: the entry is inserted but never hit. Mark such queries as not
safe to cache to avoid polluting the cache. When query_cache_strip_comments=ON
caching remains enabled since its key comes from a separate comment-stripped
copy of the query buffer.

How can this PR be tested?

Added rpl_reversed_comments.test mirroring rpl_conditional_comments.test
with equivalent test cases:

• mix of applied/not-applied
• prepared statements
• unclosed comments,
• nested comments
• delimiter edge case
• MariaDB-specific /*M!! syntax

Also added main.query_cache_executable_comments.result for testing query cache behavior.

Basing the PR against the correct MariaDB version

• This is a bug fix, and the PR is based against the branch main as the 13.0 branch does not exist at the moment.

Copyright

All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.

[gkodinov]

Thank you for fixing this so fast! This is a preliminary review.

LGTM.

Please stand by for the final review.

[bnestere]

Thank you for the patch, @tonychen2001 !

i've left a couple comments.

[bnestere]

Looks good, @tonychen2001! Thanks for updating the test with a quick turnaround.

Please also remember to update the git commit message with your reviewer (me)

Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>

[vuvova]

@tonychen2001, would you also like to fix MDEV-39421 ? After all it was discovered in your branch. Just mark any query where a conditional comment was patched in-place as non-cacheable.

[tonychen2001]

@tonychen2001, would you also like to fix MDEV-39421 ? After all it was discovered in your branch. Just mark any query where a conditional comment was patched in-place as non-cacheable.

Yes of course.

Btw, it may be a couple weeks before I'm able to get to it as I'm going to be taking a couple weeks off starting April 28 and I'm caught up with some other work I'm trying to wrap up before then.

[tonychen2001]

Hi @vuvova, I have added a commit to address MDEV-39421. I wasn't sure how to go about creating a new PR since the changes are quite related to each other so I just added it to this PR.

Please let me know your thoughts.

[gkodinov]

FWIW, this PR aims to resolve both https://jira.mariadb.org/browse/MDEV-39223 and https://jira.mariadb.org/browse/MDEV-39421