MDEV-38899 Fix Assertion `!((f->flags & 4096U) && f->vcol_info)` upon RBR with NOBLOB and unique blob by itzanway · Pull Request #4763 · MariaDB/server

itzanway · 2026-03-09T13:10:06Z

Fixes: Regression introduced by MDEV-36290.

Problem

When using Row-Based Replication (RBR) with binlog_row_image = NOBLOB, operations on a table with a UNIQUE constraint on a BLOB NOT NULL column cause a debug assertion failure in prepare_record() on the replica.

A UNIQUE index on a BLOB creates a hidden virtual column (to store the blob's hash). This hidden virtual column inherits the NOT NULL property and receives the NO_DEFAULT_VALUE_FLAG (4096U). During replication, if the blob isn't included in the row image, prepare_record() evaluates missing columns and hits the overly strict assertion.

Solution

Virtual columns evaluate their own expressions and do not use the default value mechanism. It is completely valid for them (especially automatically generated hidden ones) to have NO_DEFAULT_VALUE_FLAG set.

This PR removes the invalid assertion: DBUG_ASSERT(!((f->flags & NO_DEFAULT_VALUE_FLAG) && f->vcol_info)); and includes an MTR test case to prevent future regressions.

gkodinov

Thank you for your contribution! Before I do an actual review please:

fix the buildbot failures
squash into a single commit
add a good commit message to it compliant with CODING_STANDARDS.md
Make sure there's no extra changes into the PR (there's now removal of the BUILD dir for some reason)
rebase to 12.3 (as the first affected version): this is a bugfix.

itzanway · 2026-03-10T18:09:54Z

Hi @gkodinov,

I have addressed all the review comments:

Restored the accidentally deleted BUILD/ directory.
Squashed everything into a single commit with a CODING_STANDARDS compliant message.
Rebased to 12.3.
Fixed the line endings (CRLF -> LF) and result mismatch in the new MTR test.

My replication tests are now passing! The only remaining failure on the buildbot is main.opt_hints, which appears to be an unrelated flaky test. Could you please re-trigger the buildbot or take another look? Thank you!

grooverdan

There's no code change here. Make 100% sure there is no space only changed lines.

There's no .test file here.

When you rebase, there shouldn't be a need for a merge commit.

You have a decent commit message. Could additionally include (from MDEV) the commit/MDEV when this error was introduced in this commit message.

grooverdan · 2026-03-19T04:22:25Z

@@ -0,0 +1,20 @@
+include/master-slave.inc


should be part of another RBR replication test.

Rebased & Squashed: Squashed into a single clean commit on the 12.3 branch (no merge commits).

Code & Test Verified: Verified the fix in sql/rpl_record.cc and appended the MTR test/result cases to the existing rpl_row_img_eng_noblob files.

Commit Message: Updated to include the regression source (MDEV-36290) and a detailed description.

Ready for final review!

gkodinov

Thank you for your work so far.

One thing to note: when updating or rebasing a branch from the github UI, please always use "rebase" instead of "merge". Merge creates an extra commit. And you now again have 2 commits.

Also be careful about editors and how they change files on save.

Also, there are some tests failing because of these space only changes.

FWIW, I will not insist on fixing the assert() as suggested. Feel free to explore this or leave it for the final review.

gkodinov · 2026-03-19T12:38:20Z

+DROP TABLE t1;

--source include/rpl_end.inc
+--source include/rpl_end.inc


This is why your test fails in buildbot: mtr has a long standing bug that if you have a mtr command (as you do here) on the last line without a line terminator to follow it, mtr will consider it "trash", won't execute it and will instead fail the test with an error.

gkodinov · 2026-03-19T12:38:51Z

 --let $rpl_topology= 1->2->3
 --source include/rpl_init.inc
-- source include/have_binlog_format_row.inc
+--source include/have_binlog_format_row.inc


don't do space only changes of existing code please.

gkodinov · 2026-03-19T12:39:44Z

  DBUG_RETURN(0);
 }
-#endif // HAVE_REPLICATION
+#endif // HAVE_REPLICATION


Please set your editor not to strip the tailing new line in files! And also please revert this space only change.

gkodinov · 2026-03-19T12:55:08Z

      continue;

    Field *const f= *field_ptr;
-    DBUG_ASSERT(!((f->flags & NO_DEFAULT_VALUE_FLAG) && f->vcol_info)); // QQ


I'm guessing this is a left-over from times when there was no has_no_default_value() helper function.
I would model this check to be as close as possible to the check in check_that_all_fields_are_given_values(). I'd even consider addopting check_that_all_fields_are_given_values() itself.

has_no_default_value() has the expression including system version interval rows:

if ((field->flags & (NO_DEFAULT_VALUE_FLAG | VERS_ROW_START | VERS_ROW_END)) == NO_DEFAULT_VALUE_FLAG && field->real_type() != MYSQL_TYPE_ENUM)

And no !(*field)->vcol_info (added with be237b3), however given the other use of has_no_default_value it could be moved there.

Yes looks scarily close.

gkodinov · 2026-03-19T12:56:01Z

this should not be changed.

itzanway · 2026-03-19T18:09:07Z

Hi @gkodinov,

Thanks for the review! I’ve updated the PR to address your feedback:

Fixed editor settings to include trailing newlines and avoid space-only changes.

Reverted accidental changes to the .result and .test files.

I've kept the DBUG_ASSERT as-is for now per your note. Ready for another look!"

grooverdan · 2026-03-26T01:53:27Z

Hi @gkodinov,

Thanks for the review! I’ve updated the PR to address your feedback:

Fixed editor settings to include trailing newlines

like the test file.

look at the failures on https://buildbot.mariadb.org/#/grid?branch=refs%2Fpull%2F4763%2Fhead

and avoid space-only changes.

But the first 1/2 of mysql-test/suite/rpl/t/rpl_row_img_eng_noblob.test is whitespace changes.

Reverted accidental changes to the .result and .test files.

Except for ‎mysql-test/suite/rpl/r/rpl_noblob_unique_blob.result which is still here.

I've kept the DBUG_ASSERT as-is for now per your note. Ready for another look!"

That's not what the comment said.

gkodinov

Please squash your commits into a single one.

And fix the test failure as I've requested in my previous comment: mtr has an issue that if an "--command" is on the list line of a test it's considered junk and not executed.

Please have an empty line after it.

…que BLOB column Problem: When using Row-Based Replication (RBR) with binlog_row_image=NOBLOB, operations on a table with a UNIQUE constraint on a BLOB NOT NULL column cause a debug assertion failure in prepare_record() on the replica. A UNIQUE index on a BLOB creates a hidden virtual column (storing the blob's hash). This hidden virtual column inherits NOT NULL and receives NO_DEFAULT_VALUE_FLAG (4096U). During replication, if the blob is absent from the row image, prepare_record() evaluates missing columns and hits the overly strict assertion. Fix: Virtual columns evaluate their own expressions and do not use the default value mechanism. It is valid for them (especially auto-generated hidden ones) to have NO_DEFAULT_VALUE_FLAG set. Remove the invalid assertion and add an MTR test case appended to the existing rpl_row_img_eng_noblob test to prevent future regressions. Regression introduced by: MDEV-36290 Fix formatting and newlines Restore original result file Pull in rpl_row_img_eng_noblob.test from upstream/12.3 Add test case and results for MDEV-38899

grooverdan

Test case needs work. removal of assertion was ok.

grooverdan · 2026-04-22T05:00:28Z

-
-
--source include/rpl_end.inc
+--echo #


I'm confused why the original part of this test is removed.

I didn't follow "Pull in rpl_row_img_eng_noblob.test from upstream/12.3" from commit message, because as this is based on 12.3, the should only be the addition of the test for this MDEV.

Per the GH status failure, they pertain to this test. Replication test aren't supported in embedded mode requring --source include/not_embedded.inc in the prefix. Its assumed that part of the original test had this already.

grooverdan · 2026-04-22T05:01:06Z

+
+INSERT INTO t1 VALUES (1, 'hello');
+INSERT INTO t1 VALUES (2, 'world');
+--source include/rpl_sync.inc


per the comments in in the top of this file, this requires rpl_int.inc`.

grooverdan · 2026-04-22T05:04:22Z

+--echo # MDEV-38899: Assert !((f->flags & NO_DEFAULT_VALUE_FLAG) && f->vcol_info)
+--echo # in prepare_record() with binlog_row_image=NOBLOB and UNIQUE BLOB column
+--echo #
+--connection server_1


binlog_row_image=NOBLOB isn't set by this test.

grooverdan · 2026-04-22T05:06:18Z

+--source include/rpl_sync.inc
+
+--connection server_1
+SELECT id, CONVERT(b USING utf8) AS b FROM t1 ORDER BY id;


Its not clear why you think the result is changing. If you really want to check, perhaps testing on the replica connection.

grooverdan · 2026-04-22T05:07:24Z

+SELECT id, CONVERT(b USING utf8) AS b FROM t1 ORDER BY id;
+
+--connection server_1
+UPDATE t1 SET id = 10 WHERE id = 1;


as this isn't changing the unique blobs, what is the exercising?

grooverdan · 2026-04-22T05:12:07Z

+SELECT id, CONVERT(b USING utf8) AS b FROM t1 ORDER BY id;
+
+--connection server_1
+DROP TABLE t1;


The original test included include/rpl_row_img.test. This might be a good place to add a BLOB with index. Per the MDEV the only real test required is to insert and ensure it replicates.

grooverdan · 2026-04-22T05:25:08Z

      continue;

    Field *const f= *field_ptr;
-    DBUG_ASSERT(!((f->flags & NO_DEFAULT_VALUE_FLAG) && f->vcol_info)); // QQ


has_no_default_value() has the expression including system version interval rows:

if ((field->flags & (NO_DEFAULT_VALUE_FLAG | VERS_ROW_START | VERS_ROW_END)) == NO_DEFAULT_VALUE_FLAG && field->real_type() != MYSQL_TYPE_ENUM)

And no !(*field)->vcol_info (added with be237b3), however given the other use of has_no_default_value it could be moved there.

Yes looks scarily close.

itzanway changed the title ~~Fix Assertion !((f->flags & 4096U) && f->vcol_info) upon RBR with NOBLOB and unique blob~~ MDEV-38899 Fix Assertion !((f->flags & 4096U) && f->vcol_info) upon RBR with NOBLOB and unique blob Mar 9, 2026

itzanway force-pushed the fix-mdev-36290-assert branch from d2d6371 to c3f5419 Compare March 9, 2026 17:06

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

gkodinov requested changes Mar 10, 2026

View reviewed changes

itzanway changed the base branch from main to 12.3 March 10, 2026 11:43

itzanway force-pushed the fix-mdev-36290-assert branch from 45ada02 to 45b0f05 Compare March 10, 2026 11:49

bnestere added the Replication Patches involved in replication label Mar 10, 2026

itzanway force-pushed the fix-mdev-36290-assert branch from 45b0f05 to 8e5fbb3 Compare March 10, 2026 16:30

itzanway closed this Mar 10, 2026

itzanway force-pushed the fix-mdev-36290-assert branch from 8e5fbb3 to 620733d Compare March 10, 2026 17:09

itzanway reopened this Mar 10, 2026

itzanway requested a review from gkodinov March 18, 2026 17:41

grooverdan requested changes Mar 19, 2026

View reviewed changes

itzanway force-pushed the fix-mdev-36290-assert branch 5 times, most recently from 6740685 to 720255c Compare March 19, 2026 09:22

gkodinov requested changes Mar 19, 2026

View reviewed changes

itzanway force-pushed the fix-mdev-36290-assert branch from 17f3df6 to d412b16 Compare March 19, 2026 17:57

gkodinov requested changes Apr 9, 2026

View reviewed changes

itzanway force-pushed the fix-mdev-36290-assert branch 6 times, most recently from 548760f to 7c54d89 Compare April 10, 2026 11:06

itzanway force-pushed the fix-mdev-36290-assert branch 5 times, most recently from 149bb16 to d3cc590 Compare April 10, 2026 11:51

itzanway force-pushed the fix-mdev-36290-assert branch 2 times, most recently from 610be89 to 4a46cdc Compare April 22, 2026 04:03

itzanway force-pushed the fix-mdev-36290-assert branch from 4a46cdc to 53996c1 Compare April 22, 2026 04:39

grooverdan self-requested a review April 22, 2026 04:54

grooverdan reviewed Apr 22, 2026

View reviewed changes

Uh oh!

Conversation

itzanway commented Mar 9, 2026

Problem

Solution

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

itzanway commented Mar 10, 2026

Uh oh!

grooverdan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itzanway commented Mar 19, 2026

Uh oh!

grooverdan commented Mar 26, 2026

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

grooverdan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants