Skip to content

TODO-6835 MVI and JSON: Optimizer fixes - #6025

Draft
mariadb-YuchenPei wants to merge 4 commits into
bb-serg-MDEV-25848-mvifrom
bb-main-todo-6385
Draft

mariadb-YuchenPei wants to merge 4 commits into
bb-serg-MDEV-25848-mvifrom
bb-main-todo-6385

Conversation

@mariadb-YuchenPei

@mariadb-YuchenPei mariadb-YuchenPei commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

This is based on #5841 .

get_claimed_key_part() appended clones at param->key_parts_end, so
every later get_mm_parts() call iterated over them too and cloned each
clone again. N independent predicates on the same RTREE/ARRAY field
thus produced 2^(N-1) key parts, exhausting MAX_KEY at the 8th
predicate (e.g. 8 ORed "x MEMBER OF (js)" lost range access).

Keep clones in a separate region after key_parts_end, tracked by
RANGE_OPT_PARAM::key_part_clones_end. get_mm_parts() only walks
key_parts..key_parts_end, so each predicate creates at most one clone
and N predicates use N key parts.

Assisted-By: Claude Opus 5.5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-main-todo-6385 branch 3 times, most recently from 44bf6c0 to 6fd9997 Compare October 29, 2026 05:12
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-main-todo-6385 branch 9 times, most recently from 9cc6ec2 to 71bfa1c Compare November 4, 2026 06:10
@mariadb-YuchenPei mariadb-YuchenPei changed the title MDEV-25848 do not let get_mm_parts() match cloned key parts MDEV-25848 MVI and JSON: Optimizer fixes Nov 4, 2026
@mariadb-YuchenPei mariadb-YuchenPei changed the title MDEV-25848 MVI and JSON: Optimizer fixes TODO-6835 MVI and JSON: Optimizer fixes Nov 4, 2026
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-main-todo-6385 branch 2 times, most recently from 7373477 to f7f154d Compare November 5, 2026 03:33
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-main-todo-6385 branch 3 times, most recently from bade957 to bce0ae1 Compare November 5, 2026 05:53
Implemented get_mm_tree for JSON_OVERLAPS and JSON_CONTAINS.

For each JSON_OVERLAPS/JSON_CONTAINS(field, value), parse the array or
scalar element into a list. Map with get_mm_parts over the list and
tree_or/tree_and the results. At leaf level uses a helper extracted
from MEMBER OF.

Also moved JSON_OVERLAPS to inherit from Item_bool_func2_with_rev due
to its symmetry.
@mariadb-YuchenPei
mariadb-YuchenPei force-pushed the bb-main-todo-6385 branch 2 times, most recently from 4c16f31 to 46ddba7 Compare November 6, 2026 05:26
Before, every MEMBER OF / JSON_OVERLAPS / JSON_CONTAINS point got its
own cloned KEY_PART and key slot, and points were combined only through
index_merge (OR) or by picking one slot (AND).

Now all points on an ARRAY key part share the original slot:

* SEL_ARG_ARRAY holds a list of make_key() points ("the array contains
  any of them") and the summed records_in_range() estimate and cost,
  taken once when the point is built.
* key_or() of two ARRAY ranges takes the union of the points;
  key_and() keeps the operand with the lower estimate, the WHERE
  condition checks the rest.
* KEY_PART cloning (get_claimed_key_part()) is now RTREE-only, so
  ARRAY-only queries size SEL_TREE::keys by param->keys again.
* QUICK_RANGE_SELECT_ARRAY gets one QUICK_RANGE per point and passes
  all points to hlindex_read_first(); json_index reads them in turn
  and skips trefs it has already returned.
* check_quick_select_array() uses the stored estimate, and rejects
  non-ARRAY SEL_ARGs (e.g. IS NULL) on the ARRAY key part.

ORed MEMBER OF and JSON_OVERLAPS now give a single range scan instead
of index_merge sort_union, with no MAX_KEY limit on the number of
points.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@spetrunia

Copy link
Copy Markdown
Member

The only piece of input to 2bf55f8 :
Need to note near the declaration of key_parts_end that it's "not the end anymore" and cloned keyparts follow.

I continue looking

@spetrunia

Copy link
Copy Markdown
Member

For: 3c3e6aa
Need a comment saying what RANGE_OPT_PARAM::max_keys is.

Comment thread sql/item_jsonfunc.cc
*/
Json_range_elements json_get_range_elements(THD *thd, Item *value,
CHARSET_INFO *cs,
List<Item> *elements)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this need to be List or List<LEX_STRING> would suffice?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As far as I understand, if you change this to be List<LEX_STRING> (or something else not using Item objects), then you won't need mem_root juggling in json_get_func_mm_tree():

  MEM_ROOT *tmp_root= param->mem_root;
  param->thd->mem_root= param->old_root;

  Json_range_elements jre=
    json_get_range_elements(param->thd, value, field->charset(), &elements);

  param->thd->mem_root= tmp_root;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another argument for not using Item objects:

"Range checked for each record" plans can call range optimizer O(#examined records) times. If MVI index is one among the considered, this could cause O(#examined records) Item objects to be allocated...

Comment thread sql/opt_range.cc
template <class Get_mm_parts_func>
static SEL_TREE *json_get_func_mm_tree(RANGE_OPT_PARAM *param, Field *field,
Item *value, bool conjunctive,
Get_mm_parts_func &&get_mm_parts)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What was the goal to make this a template and then use Get_mm_parts_func &&get_mm_parts reference? Was it to get "perfect forwarding"? And if yes, why that was needed?

Comment thread sql/item_jsonfunc.cc
if (je.state != JST_VALUE || json_read_value(&je))
return Json_range_elements::NO_RANGE;
}
/* TODO: nested array and object fail the check here? */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, checked that this happens here.

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.

4 participants