Skip to content

Resolve an object-vs-scalar mapping conflict to the scalar - #5756

Open
ahkcs wants to merge 1 commit into
opensearch-project:mainfrom
ahkcs:fix/object-scalar-merge-rule
Open

Resolve an object-vs-scalar mapping conflict to the scalar#5756
ahkcs wants to merge 1 commit into
opensearch-project:mainfrom
ahkcs:fix/object-scalar-merge-rule

Conversation

@ahkcs

@ahkcs ahkcs commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Description

When a wildcard spans indices that disagree on whether a path is an object or a scalar — as happens when a mapping changes at a rollover boundary — the merged type is decided by whichever index is merged last, and that order is randomized per JVM. The same query therefore resolves the path as string on some nodes and struct on others, and flips when a node restarts.

Measured on six freshly started single-node clusters with one keyword index and one object index, identical fixture each time:

  run 1 -> labels.zone : struct        run 4 -> labels.zone : struct
  run 2 -> labels.zone : string        run 5 -> labels.zone : string
  run 3 -> labels.zone : string        run 6 -> labels.zone : string

When it lands on the object side, every value aggregates to null and timechart ... by <path> fails (#5750, #5751). When it lands on the scalar side the query works. Same data, same code, different node.

Why it is nondeterministic. MergeRuleHelper tries its rules in order:

Rule Matches Object vs scalar?
DeepMergeRule both sides share an ExprCoreType (STRUCT or ARRAY) no — the core types differ
TextKeywordConflictRule text vs keyword, or text with/without a keyword sub-field no — neither side is text
LatestRule everything yes — target.put(key, source), last write wins

The order comes from the mapping map that OpenSearchNodeClient.getIndexMappings builds with Collectors.toUnmodifiableMap; the JDK randomizes iteration order of its immutable maps per JVM via ImmutableCollections.SALT. Reduced to the merge decision alone over 11 index names of which 4 map the path as an object, object won 15 of 20 JVM runs and keyword 5.

This change adds ObjectScalarConflictRule ahead of LatestRule, resolving such a path to the scalar side. The scalar is the only side holding a value that can be grouped, sorted or charted, and it keeps doc-values pushdown available — resolving to text instead (what the text/keyword rule does for its case) would force _source retrieval and a full scan, i.e. the PIT-exhaustion path of #5634/#5646.

The predicate decides on the mapping type rather than the ExprCoreType, because text, match_only_text, geo_point and binary all resolve to ExprCoreType.UNKNOWN while still holding one value per document. Keying on the core type would have left the most common conflict — text vs object — on the coin flip.

Behaviour, measured before and after

Query over the wildcard Before After
fields labels.zone struct or string, per node string, always
stats count() by labels.zone all buckets null, or real values real values, object side in the missing bucket[1,null] [1,"z1"] [1,"z2"]
timechart span=1m count() by labels.zone 400 (#5751) or a chart, per node a chart, always
fields labels.zone.name 200 with values, or 400, per node 400 Field [labels.zone.name] not found. Did you mean: labels.zone
source=<object index> | fields labels.zone.name values values, unchanged

Two consequences worth a reviewer's attention:

  1. The object's sub-fields go with it. path.sub over the wildcard now fails deterministically, where previously it worked whenever the object side happened to win. I implemented sub-field retention first and measured it to be worse: the row cannot hold both a scalar and a subtree at one path, so _source parsing never descends and the retained column resolved but always read null — silently wrong rather than an error. Querying the object-mapped index directly still returns the sub-fields, which is the workaround for anyone who needs the leaf.
  2. Aggregation keeps the object side's documents in the missing bucket rather than excluding them, so counts still add up.

Not included, deliberately

PartialResultAggregatePushdown.resolveBucketSignature emits a t:Object token for an object-mapped index, which counts as a second "aggregatable group" and makes plan() bail — so on a wildcard holding text + keyword + object indices, the partial-result feature of #5657 declines to engage for a reason unrelated to its own conflict. That is a separate defect with its own fixture, and it is not needed here: the aggregation pushes down fine without it, and excluding those indices would drop their documents from the count, which the missing bucket does not.

Related Issues

Resolves #5752
Related: #5750, #5751 (the crash this conflict caused), #5685 / #5618 (the value-decode side of the same conflict), #5610 (schema-conflict policy), #4659 (TextKeywordConflictRule, the precedent for resolving a conflict deterministically)

Testing

ObjectScalarConflictRuleTest — 9 cases: object/nested/text against a scalar in both directions; no match when neither or both sides are containers; no match on a null side; the scalar wins regardless of merge order; sub-fields are dropped.

integ-test/.../ppl/object_scalar_conflict.yml — 6 cases over HTTP: the path resolves to string; grouping returns the scalar values with the object side in the missing bucket; timechart can split by the path; the sub-fields fail with "field not found"; the sub-fields still work when the object index is queried directly; a text mapping also wins the path against an object.

Suite Result
ObjectScalarConflictRuleTest 9/9
yamlRestTest -Dtests.rest.suite=ppl/object_scalar_conflict 6/6, green on 3 consecutive freshly started clusters (the determinism check — the same fixture was a coin flip before)
All module unit tests (./gradlew test) pass
yamlRestTest issues/5685 (object/scalar decode conflict) pass
yamlRestTest ppl/non_scalar_coercion (#5751) pass
*Conflict*IT, *DataType*IT, *Nested*IT pass

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…h-project#5752)

When a wildcard spans indices that disagree on whether a path is an object
or a scalar -- as happens when a mapping changes at a rollover boundary --
the pair matches no merge rule: DeepMergeRule needs both sides to share an
ExprCoreType and TextKeywordConflictRule only matches the text family. It
therefore falls through to LatestRule, so the winner is whichever index is
merged last.

That order comes from the mapping map, which OpenSearchNodeClient builds
with Collectors.toUnmodifiableMap. The JDK randomizes iteration order of
its immutable maps per JVM (ImmutableCollections.SALT), so the resolved
type differs between nodes and changes when a node restarts. Measured on
six freshly started clusters with one keyword index and one object index,
the same query resolved the path as an object three times and as a string
three times. When it lands on the object side, every value aggregates to
null and `timechart ... by <path>` fails (opensearch-project#5750).

Add ObjectScalarConflictRule, ahead of LatestRule, resolving such a path to
the scalar side. The scalar is the only side holding a value that can be
grouped, sorted or charted, and it keeps doc-values pushdown available;
resolving to text instead (what the text/keyword rule does for its case)
would force _source retrieval and a full scan. Documents from the indices
that map the path as an object have no scalar there, so they aggregate into
the missing bucket, the same as a field absent from an index.

The predicate decides on the mapping type rather than the ExprCoreType,
because text, match_only_text, geo_point and binary all resolve to UNKNOWN
while still holding one value per document -- keying on the core type would
have left the most common conflict, text vs object, on the coin flip.

Two consequences worth noting for review:

- The object's sub-fields go with it, so `path.sub` over the wildcard now
  fails with "field not found" where it previously worked whenever the
  object side happened to win. Retaining the sub-fields was tried first and
  is worse: the row cannot hold both a scalar and a subtree at one path, so
  the retained column resolved but always read null. Querying the
  object-mapped index directly still returns the sub-fields.
- Aggregating over the path puts the object side's documents in the missing
  bucket rather than excluding them, so counts still add up.

Signed-off-by: Kai Huang <ahkcs@amazon.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Object-vs-scalar mapping conflict across a wildcard resolves nondeterministically (last-write-wins)

1 participant