Skip to content

MDEV-36896 - Assertion `marked_for_read()' failed in virtual String *Field_varstring::val_str(String *, String *)#5158

Open
pranavktiwari wants to merge 1 commit into
10.11from
10.11-MDEV-36896
Open

MDEV-36896 - Assertion `marked_for_read()' failed in virtual String *Field_varstring::val_str(String *, String *)#5158
pranavktiwari wants to merge 1 commit into
10.11from
10.11-MDEV-36896

Conversation

@pranavktiwari
Copy link
Copy Markdown

@pranavktiwari pranavktiwari commented Jun 1, 2026

fixes MDEV-36896

Problem:

Executing queries that require virtual/generated column evaluation during filesort trigger sdebug assertions due to missing columns in read_set.

Cause:

find_all_keys() temporarily assigns TABLE::tmp_set as both read_set and write_set. Later, TABLE::update_virtual_field() clears tmp_set before evaluating virtual column dependencies.

Since all three pointers reference the same bitmap, clearing tmp_set also clears the active column maps, causing required columns to be missing during execution.

Fix:

Remove the bitmap_clear_all(&tmp_set) call from TABLE::update_virtual_field(). The dependency walk populates the required bits for virtual column evaluation, and clearing the shared bitmap can unintentionally invalidate the active read_set/write_set.

@pranavktiwari pranavktiwari marked this pull request as draft June 1, 2026 13:06
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request comments out the column_bitmaps_set call in sql/filesort.cc to address an assertion failure. However, the reviewer notes that this is incorrect because it bypasses the temporary read map optimization and pollutes the original read set. The recommended approach is to identify the missing field and ensure it is properly registered instead of disabling the bitmap switch.

Comment thread sql/filesort.cc Outdated
…Field_varstring::val_str(String *, String *)

update_virtual_field() uses tmp_set only as an accumulator during dependency traversal. The walk itself sets all required bits for the current virtual-column evaluation. Clearing tmp_set beforehand is not required to discover the current dependencies, but it does remove previously established bits. Since find_all_keys() aliases tmp_set as both read_set and write_set, clearing tmp_set also clears the active column maps. Replacing tmp_set with an independent bitmap or avoiding the clear eliminates the issue and all regression tests continue to pass.
@pranavktiwari pranavktiwari changed the title DRAFT - Assertion `marked_for_read()' failed in virtual String *Field_varstring::val_str(String *, String *) MDEV-36896 - Assertion `marked_for_read()' failed in virtual String *Field_varstring::val_str(String *, String *) Jun 2, 2026
@pranavktiwari pranavktiwari marked this pull request as ready for review June 2, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants