Skip to content

MDEV-37864 json_array_intersect result has incorrect length#5114

Open
grooverdan wants to merge 1 commit into
MariaDB:11.4from
grooverdan:MDEV-37864
Open

MDEV-37864 json_array_intersect result has incorrect length#5114
grooverdan wants to merge 1 commit into
MariaDB:11.4from
grooverdan:MDEV-37864

Conversation

@grooverdan
Copy link
Copy Markdown
Member

MDEV-37864 added a length setting to the function
Item_func_json_array_intersect::prepare_json_and_create_hash however some paths of Item_func_json_array_intersect::fix_length_and_dec never reach this function.

Leave the length calculation the same but place in the ::fix_length_and_dec function.

Collation is based on the arg[0], but potentially after they are swapped.

Corrects cursor protocol for the test case added in MDEV-36808.

@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label May 23, 2026
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 refactors the Item_func_json_array_intersect class by moving the result length calculation and collation setting from prepare_json_and_create_hash to fix_length_and_dec. Feedback suggests consolidating the duplicated collation.set calls at the end: label to ensure all execution paths are covered correctly. Additionally, the length calculation formula 2 * MY_MIN(...) should be adjusted to prevent potential JSON truncation for small input values.

Comment thread sql/item_jsonfunc.cc Outdated
Comment on lines +5552 to +5558
collation.set(args[0]->collation);
parse_for_each_row= true;
goto end;
}
}

collation.set(args[0]->collation);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The call to collation.set(args[0]->collation) is duplicated at lines 5552 and 5558. Furthermore, it appears to be missing from the if (args[0]->const_item() && args[1]->const_item()) path (the portion not shown in the diff).

A more robust and cleaner approach would be to move this call to the end: label, just before fix_char_length_ulonglong. This ensures the collation is set exactly once for all successful paths, including those that use goto end;.

Comment thread sql/item_jsonfunc.cc Outdated
Comment on lines +5567 to +5569
fix_char_length_ulonglong((ulonglong)
2* MY_MIN(args[0]->max_char_length(),
args[1]->max_char_length()));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The formula 2 * MY_MIN(...) used for length calculation may result in a value that is too small for very short inputs, potentially leading to truncation of the JSON result.

For example, if the minimum input length is 1 (e.g., a single-character string 'a'), the calculated length would be 2 * 1 = 2. However, the resulting JSON array ["a"] requires 5 characters. While this logic was moved from prepare_json_and_create_hash, moving it to fix_length_and_dec is a good opportunity to ensure a safer upper bound (e.g., MY_MIN(...) * 2 + 2 or a minimum floor) to avoid invalid JSON output due to truncation.

MDEV-37864 added a length setting to the function
Item_func_json_array_intersect::prepare_json_and_create_hash
however some paths of Item_func_json_array_intersect::fix_length_and_dec
never reach this function.

Leave the length calculation the same but place in the
::fix_length_and_dec function.

Collation is based on the arg[0], but potentially after they
are swapped.

Corrects cursor protocol for the test case added in MDEV-36808.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

2 participants