MDEV-39418: Revert most of MDEV-39240 for MDEV-32188#5048
MDEV-39418: Revert most of MDEV-39240 for MDEV-32188#5048ParadoxV5 wants to merge 10 commits intobb-11.8-releasefrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the validation and normalization logic for TIMESTAMP values during row unpacking, effectively reverting changes associated with MDEV-39240. It also cleans up the corresponding test cases and result files. A potential compilation error was identified in sql/rpl_record.cc where the guard for a debug variable was narrowed to require DBUG_TRACE, despite the variable still being used in a DBUG_PRINT statement that is active whenever DBUG_OFF is not defined.
| #if !defined DBUG_OFF && defined DBUG_TRACE | ||
| const uchar *old_pack_ptr= pack_ptr; | ||
| #endif |
There was a problem hiding this comment.
The guard for old_pack_ptr has been changed to require DBUG_TRACE, but the variable is used in a DBUG_PRINT statement on line 111. In MariaDB, DBUG_PRINT is active whenever DBUG_OFF is not defined, regardless of whether DBUG_TRACE is enabled. This change will cause a compilation error in debug builds where DBUG_TRACE is disabled but DBUG_PRINT is active. It is recommended to revert the guard to #ifndef DBUG_OFF while keeping the const qualifier improvement.
| #if !defined DBUG_OFF && defined DBUG_TRACE | |
| const uchar *old_pack_ptr= pack_ptr; | |
| #endif | |
| #ifndef DBUG_OFF | |
| const uchar *old_pack_ptr= pack_ptr; | |
| #endif |
There was a problem hiding this comment.
This change will cause a compilation error in debug builds where
DBUG_TRACEis disabled butDBUG_PRINTis active.
The code on base 11.8 is already this way.
Is this true?
improvement
This contextless bot doesn’t know that this is a revert, not an improvement.
|
Oh, right, Buildbot runs on the incoming branch ( |
MDEV-39240 fixed how servers before 11.5/11.4-enterprise accepted timestamps beyond Year 2038 from row-based replication, which were invalid until 11.5/11.4-enterprise’s MDEV-32188. MDEV-39240 does not apply after MDEV-32188 extended the valid range, so those versions should exclude this fix, as if MDEV-32188 already covers it. This commit reverts commits 3234045 and most of f9c34a1, keeping only the tweak to the MTR script `include/check_type` for consistency.
sanja-byelkin
force-pushed
the
bb-11.8-release
branch
from
5d8020d to
9cb70f0
Compare
2 participants
MDEV-39240 fixed how servers before 11.5/11.4-enterprise accepted timestamps beyond Year 2038 from row-based replication, which were invalid until 11.5/11.4-enterprise’s MDEV-32188.
MDEV-39240 does not apply after MDEV-32188 extended the valid range, so those versions should exclude this fix, as if MDEV-32188 already covers it.
This commit reverts commits 3234045 and most of f9c34a1, keeping only the tweak to the MTR script
include/check_typefor consistency.