Add Ids DSL query support via Lucene delegation

[ask-kamal-nayan]

Description

Adds support for the OpenSearch ids query in the Calcite-based DSL query executor.
The query is translated into a fieldless delegated predicate (IDS(MAP(...), ...)) and
serialized back to a vanilla IdsQueryBuilder for execution by the Lucene backend.

What: A translator (IdsQueryTranslator) converts IdsQueryBuilder into a Calcite
RexCall, and a serializer (IdsQuerySerializer) reconstructs the IdsQueryBuilder
on the backend side so Lucene can execute it natively.

Why: The ids query is a fundamental filtering primitive. Users need it for
point-lookups, deletion scopes, and boolean compositions with other predicates.

Where: Two modules are touched:

• dsl-query-executor — translator, registry wiring, golden-file tests, integration test stub.
• analytics-backend-lucene — serializer, registry wiring, serializer tests.

A ScalarFunction.IDS enum constant is added to analytics-framework to register the
function in the capability matrix.

---

Why Lucene Delegation

_id is stored Uid-encoded in both the Parquet and Lucene segments:

• IdFieldMapper.preParse calls Uid.encodeId before indexing.
• IdParquetField writes those same encoded bytes to the Parquet column.
• GetService calls Uid.decodeId on read to recover the original string.

A naïve native comparison of a raw query string against the stored bytes silently
returns zero rows. Delegating to vanilla's IdsQueryBuilder keeps encoding inside
IdFieldType.termsQuery where it belongs — the single canonical encode/compare path.

A future optimisation could pre-encode the query literals with Uid.encodeId and push
a native byte-comparison predicate directly to Parquet. This is legitimate but not
necessary for correctness today; the delegation path is zero-risk.

---

Why the Call Is Fieldless

This is the design's most notable property.

The ids query targets the implicit _id metadata field, which is not a column in
the user-visible Calcite row type. No field operand is emitted in the RexCall:

IDS(MAP('values.0', 'doc1'), MAP('values.1', 'doc2'))

Routing works because OpenSearchFilterRule.java (lines 238-240) routes FULL_TEXT
functions that carry no field reference to filterBackendsAnyFormat(function, FieldType.TEXT). The capability pair (IDS, TEXT) is already registered by the
existing loop over ScalarFunction values in LuceneAnalyticsBackendPlugin.

Consequence: _id is never added to the Calcite row type. The change is entirely
outside the schema shared with the PPL and SQL paths — no schema pollution, no
downstream breakage risk.

---

Supported and Rejected Parameters

Parameter	Status	Rationale
values	Supported	Core id list. Encoded as indexed MAP operands.
boost	Rejected (ConversionException)	The delegated path returns a match bitset with no scores; a non-default boost is meaningless.
_name	Rejected (ConversionException)	matched_queries is not surfaced by SearchResponseBuilder.
types	No check needed	Removed in OpenSearch 2.0. IdsQueryBuilder has no field for it. The strict two-arg ObjectParser at IdsQueryBuilder.java:135 throws XContentParseException at parse time — a runtime rejection check could never fire.

---

Operand Encoding

Each id is carried as a separate indexed MAP operand:

MAP('values.0', 'id0'), MAP('values.1', 'id1'), ...

Why not a comma-joined string? An _id may legally contain commas. A joined
string would be lossy. The indexed-MAP scheme preserves each id verbatim through the
extractOptionalParams contract.

Both IdsQueryTranslatorTests.testIdContainingComma and
IdsQuerySerializerTests.testSerializeIdContainingComma assert lossless round-trip
for ids containing commas.

---

Behaviour

Condition	Result
Empty values	Returns a FALSE RexLiteral (match-nothing). Mirrors IdsQueryBuilder.doRewrite line 157 rewriting to MatchNoneQueryBuilder.
Duplicate ids	Collapsed. IdsQueryBuilder.ids() returns a Set; the translator emits one operand per unique id. Verified by testDuplicateIdsAreDeduplicated.
Ordering	Ids are sorted lexicographically before emission to produce deterministic plans regardless of HashSet iteration order.

---

Plan Shape

Single-id plan (ids_query_single.json):

LogicalFilter(condition=[IDS(MAP('values.0', 'doc1'))])
  LogicalTableScan(table=[[test-index]])

Multi-id plan (ids_query_multiple.json):

LogicalFilter(condition=[IDS(MAP('values.0', 'doc1'), MAP('values.1', 'doc2'), MAP('values.2', 'doc3'))])
  LogicalTableScan(table=[[test-index]])

---

E2E Validation

Live-cluster validation was performed on a combined test branch against a composite-format
index (Parquet primary, Lucene secondary) with six documents.

Row	Query Shape	Verdict
1	ids=[id0]	PASS
2	ids=[id0, id1]	PASS
13	bool.must=[ids(id0,id1,id3), term(region=us-east-1)]	PASS
14	bool.must_not=[ids(id0)]	PASS
15	bool.should=[ids(id0,id1), ids(id2,id3)] MSM=2	PASS
20	bool.should 3 wildcards MSM=2 + stats	PASS
21	extended_stats on latency	PASS
12	exists on _id + stats	EXPECTED-FAIL (500: Field '_id' not found in schema)
19	terms agg on _id	EXPECTED-FAIL (500: Group-by field '_id' not found in schema)

Rows 13, 14, and 15 are the bool-composition shapes (mixed-backend AND, negated,
OR with minimum_should_match). All three match the baseline exactly.

Note: ids inside a bool requires the bool translator, which lives on a separate
branch. That coverage came from a combined test branch that merged both feature branches.

---

Known Divergences from Legacy _search

Behaviour	This PR	Vanilla _search	Reason
Scoring	No scores returned	Scored	Delegated path returns a match bitset; boost rejected.
matched_queries	Not surfaced	Supported	SearchResponseBuilder does not propagate named queries; _name rejected.
Predicate pruning	No page/row-group skipping	N/A (Lucene handles internally)	delegated_predicate is opaque to the Calcite pruning planner.
exists query on _id	Fails: Field '_id' not found in schema	Works	_id is deliberately not a schema column; matches pre-existing behaviour.
terms aggregation on _id	Fails: Group-by field '_id' not found in schema	Works	Same reason as above.
sort on _id	Fails: Field '_id' not found in schema	Works	Same reason as above.
End-to-end hits	Empty (SearchHits.empty(true))	Full hits returned	SearchResponseBuilder.build() returns empty hits for all queries; not ids-specific. Integration tests parked with @AwaitsFix.

---

Testing

Module	Unit Tests	Failures	spotlessJavaCheck
dsl-query-executor	151	0	PASSED
analytics-backend-lucene	319	0	PASSED
analytics-framework	113	0	PASSED

Golden files:

• src/test/resources/golden/ids_query_single.json
• src/test/resources/golden/ids_query_multiple.json

Unit test coverage (translator — IdsQueryTranslatorTests):

• Single id, multiple ids, comma-containing id (lossless encoding)
• Empty values → FALSE literal
• Non-default boost → ConversionException
• _name → ConversionException
• Duplicate ids deduplicated (operand count + value set assertion)
• getQueryType() returns IdsQueryBuilder.class

Unit test coverage (serializer — IdsQuerySerializerTests):

• Single id round-trip through NamedWriteableRegistry
• Multiple ids preserved
• Comma-containing id survives serialization
• Malformed values.notAnInt key → IllegalArgumentException with diagnostic message
• QuerySerializerRegistry contains ScalarFunction.IDS entry

Integration tests (DslIdsQueryIT):

• Three test methods (single, multiple, empty) compiled successfully.
• Parked with class-level @AwaitsFix because SearchResponseBuilder.build() returns
  SearchHits.empty(true) — no query returns end-to-end hits yet.

Suites not run:

• DslIdsQueryIT — blocked by SearchResponseBuilder empty-hits behaviour (see above).
• Full E2E with _search endpoint — requires multi-plugin cluster startup; validated
  manually on a combined branch (results table above).

---

Check List

• Functionality includes testing.
  • 9 translator unit tests, 6 serializer unit tests, 2 golden-file plan assertions,
    integration test stub compiled and parked.
• API changes companion pull request created, if applicable.
  • Not applicable — no public API surface change; ids query is already part of the
    OpenSearch Query DSL specification.
• Public documentation issue/PR created, if applicable.
  • Not applicable — this enables existing DSL syntax in a new execution path; no
    user-facing documentation change needed until the feature exits experimental status.

---

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 4a65abf)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Sorted IDs Change Semantics sortedIds reorders IDs alphabetically before emitting them into values.N. While this yields deterministic plans, it means the values.N index no longer reflects the user's original input order. If any downstream consumer (or future feature such as explain/highlight ordering) relies on positional correspondence to the original ids array, this reordering will silently produce inconsistent output. Consider preserving input order via a LinkedHashSet upstream or documenting that order is not preserved. List<String> sortedIds = ids.stream().sorted().toList(); int index = 0; for (String id : sortedIds) { operands.add(rex.makeCall(SqlStdOperatorTable.MAP_VALUE_CONSTRUCTOR, rex.makeLiteral("values." + index), rex.makeLiteral(id))); index++; } Possible Duplicate Entry The diff adds Map.entry(ScalarFunction.SARG_PREDICATE, new SargSerializer()) inside the map initializer, but the old hunk shows this entry was already present (only trailing comma changed). If SARG_PREDICATE is now listed twice in the Map.ofEntries(...) call, Map.ofEntries will throw IllegalArgumentException: duplicate key at class-load time. Verify only one SARG_PREDICATE entry exists after the edit. Map.entry(ScalarFunction.SARG_PREDICATE, new SargSerializer()), Map.entry(ScalarFunction.IDS, new IdsQuerySerializer())

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 4a65abf
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Emit properly typed false literal RexBuilder.makeLiteral(false) may produce a literal whose type does not match the boolean type expected by the enclosing filter, and downstream Calcite optimizers can reject a bare non-nullable boolean literal where a nullable boolean is required. Use the type-aware overload or rexBuilder.makeLiteral(false, booleanType, false) / RelOptUtil.createFalse equivalent to ensure the resulting RexNode is a proper boolean predicate. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/IdsQueryTranslator.java [71-73] if (ids.isEmpty()) { - return ctx.getRexBuilder().makeLiteral(false); + RexBuilder rb = ctx.getRexBuilder(); + return rb.makeLiteral(false, rb.getTypeFactory().createSqlType(org.apache.calcite.sql.type.SqlTypeName.BOOLEAN), false); } Suggestion importance[1-10]: 4 __ Why: RexBuilder.makeLiteral(false) returns a non-nullable BOOLEAN literal which is usually acceptable, but explicitly typing it can improve compatibility with downstream planner rules. The concern is speculative without evidence of actual failure.	Low
	Validate value keys before sorting The stream filters keys by prefix but does not guard against non-numeric suffixes before sorting; parseValueIndex will throw for any key starting with values. but not followed by an integer. Move the malformed-key check up-front or filter to numeric suffixes only, so unrelated values.* metadata (if ever added) does not crash serialization. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/IdsQuerySerializer.java [41-46] String[] ids = params.entrySet() .stream() .filter(e -> e.getKey().startsWith(VALUES_PREFIX)) + .peek(e -> parseValueIndex(e.getKey())) .sorted((a, b) -> Integer.compare(parseValueIndex(a.getKey()), parseValueIndex(b.getKey()))) .map(Map.Entry::getValue) .toArray(String[]::new); Suggestion importance[1-10]: 3 __ Why: The suggestion is marginal: the existing sorted comparator already calls parseValueIndex, so malformed keys will throw with a diagnostic message anyway. The peek addition provides only slight ordering benefit and is a minor robustness improvement.	Low

---

Previous suggestions

Suggestions up to commit 5152851

Category	Suggestion	Impact
General	Validate id values before addIds call IdsQueryBuilder.addIds rejects null or empty id strings and throws. Since the serializer receives untrusted map values, validate that no entry value is null before invoking addIds, or filter them out, to avoid an unexpected NullPointerException/IllegalArgumentException that surfaces far from the offending input. sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/serializers/IdsQuerySerializer.java [41-47] Map<String, String> params = ConversionUtils.extractOptionalParams(call, 0); IdsQueryBuilder builder = new IdsQueryBuilder(); // Collect values in index order for deterministic behaviour String[] ids = params.entrySet() .stream() .filter(e -> e.getKey().startsWith(VALUES_PREFIX)) .sorted((a, b) -> Integer.compare(parseValueIndex(a.getKey()), parseValueIndex(b.getKey()))) - .map(Map.Entry::getValue) + .map(e -> { + if (e.getValue() == null) { + throw new IllegalArgumentException("Ids query received null value for key [" + e.getKey() + "]"); + } + return e.getValue(); + }) .toArray(String[]::new); builder.addIds(ids); Suggestion importance[1-10]: 3 __ Why: Adding null validation is a minor defensive improvement; in practice values come from Calcite RexLiteral string operands and are unlikely to be null, so the impact is low.	Low
	Guard against null ids during translation The translator sorts ids alphabetically, but the serializer sorts by the numeric index in the values.N key. If the ids set exceeds 10 entries, alphabetical sort produces indices where values.10 sorts before values.2, but this is safe because the serializer parses the numeric suffix. However, if any id is null (which IdsQueryBuilder allows in theory before validation), stream().sorted() will NPE. Guard against null ids to produce a clearer error. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/query/IdsQueryTranslator.java [87-92] // Sort for deterministic plan output (IdsQueryBuilder.ids() is a HashSet with no order guarantee). +if (ids.stream().anyMatch(id -> id == null)) { + throw new ConversionException("Ids query contains a null id"); +} List<String> sortedIds = ids.stream().sorted().toList(); int index = 0; for (String id : sortedIds) { operands.add(rex.makeCall(SqlStdOperatorTable.MAP_VALUE_CONSTRUCTOR, rex.makeLiteral("values." + index), rex.makeLiteral(id))); index++; } Suggestion importance[1-10]: 2 __ Why: IdsQueryBuilder generally rejects null ids upstream, so this defensive check offers marginal value and mostly improves error messaging.	Low

[github-actions]

❕ Gradle check result for 5152851: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.54%. Comparing base (c8e2303) to head (4a65abf).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22684      +/-   ##
============================================
+ Coverage     71.48%   71.54%   +0.06%     
- Complexity    76960    77014      +54     
============================================
  Files          6156     6156              
  Lines        358444   358444              
  Branches      52246    52246              
============================================
+ Hits         256240   256457     +217     
+ Misses        81792    81597     -195     
+ Partials      20412    20390      -22     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 4a65abf

[github-actions]

✅ Gradle check result for 4a65abf: SUCCESS