Repository navigation
fix(api): refuse a reverse_nested path that is not a parent of the scope - #3009
Open
amartya1523 wants to merge 2 commits into
Open
amartya1523 wants to merge 2 commits into
amartya1523 wants to merge 2 commits into
Conversation
reverse_nested joins back from the nested documents it sits in to the root (no path) or to a nested object that contains the current one. ES refuses a path naming a nested object below or beside the current scope with "Reverse nested path [p] is not a parent of the current nested scope [s]". XERJ ignored the path and answered 200 with a count of root documents, which reads as a valid join. search_impl now collects the nested field paths of the target indices and walks the aggregation tree carrying the nested scope (set by nested, reset by reverse_nested). Only a path that is a mapped nested field is checked, so an unmapped path keeps today's answer, as in ES. Retires `aggregations/nested.yml · Illegal reverse nested aggregation to a child nested object`.
A child path and a sibling path are a 400 with ES's reason; joining back to a parent, to the root, an unmapped path and a reverse_nested below a filter still answer.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AI-assisted change, run and reviewed by @amartya1523.
What this changes, and why
A
reverse_nestedaggregation joins back from the nested documents it sits in to an enclosing level: the root document (nopath), or a nested object that contains the current one. In this mapping,coursesis nested andcourses.sessionsis nested inside it:ES refuses this (
ReverseNestedAggregationBuilder, capabilityreject_invalid_reverse_nesting, 8.16+) with:XERJ ignored
path(run_reverse_nestedinaggs.rsjoins back to the root unconditionally) and answered 200 with a root-document count, which looks like a valid join. Onecatch-lenient.txtline is this case:aggregations/nested.yml · Illegal reverse nested aggregation to a child nested object.Fix (es_compat.rs):
nested_field_pathscollects every field mappedtype: nestedin the target indices, by full dotted path (courses,courses.sessions). It takes the union over all target indices.find_invalid_reverse_nestingwalks the aggregation tree carrying the current nested scope. Anestedaggregation sets the scope to itspath; areverse_nestedresets it to its ownpath, or to the root when it has none; every other aggregation passes the scope down unchanged.A
reverse_nestedwith apathis refused when all of these hold:pathorpath.…).An unmapped path keeps today's answer; in ES it's an unmapped aggregation, not an error.
It runs in
search_implright afterfind_unsupported_metric_agg_field, with the existingsearch_shard_validation_errorenvelope: 400search_phase_execution_exception, root causeillegal_argument_exception.No engine change: valid
reverse_nestedrequests run exactly as before.Evidence
New test module
reverse_nesting_tests(es_compat.rs), run against main first:nested courses.sessions→reverse_nested staff_list)nested courses.sessions→reverse_nested courses(a parent)reverse_nested {}fromcourses.sessionsand fromcourses(the root)nested courses→filter→nested courses.sessions→reverse_nested {}, the shape of the passing YAML case "Nested then filter then nested then reverse nested"Checks
cargo fmt -p xerj-api -- --checkcargo test --profile quick -p xerj-api --lib: 432 passed, 0 failedcargo clippy --profile quick -p xerj-api --lib --tests: no warnings in the changed codenested.ymlcase now enforces it; its catch-lenient line is deleted. The othernested.ymlcases usereverse_nested: {}or no reverse join, so they're unaffected.Not run locally:
--releasebuilds and the integration binaries. CI runs these.Provenance
quickprofile, based on main34bf35b05.IllegalArgumentException.reverse_nestedpath equal to the current scope isn't refused here. It's a no-op join, and I wasn't certain ES refuses it._msearchand the template paths don't go throughsearch_impl, so they aren't covered.