Enable Lucene Filter delegation from Datafusion for Correctness by expani · Pull Request #21555 · opensearch-project/OpenSearch

expani · 2026-05-07T23:32:46Z

Description

This PR implements end-to-end filter delegation for indexed parquet execution. When a PPL query contains predicates that are better evaluated by Lucene (e.g., match(field, 'term')), the analytics engine delegates those predicates to the Lucene
backend, which produces bitsets that DataFusion uses during its indexed scan to filter rows.

What this enables

A query like source=my_index | where match(message, 'hello') on a composite (parquet + Lucene) index now:

The coordinator identifies match as a delegatable predicate and marks it in the execution plan
The data node's dispatch loop exchanges a delegation handle between the driving backend (DataFusion) and the accepting backend (Lucene)
During execution, DataFusion's Rust engine calls back into Java via FFM to obtain Lucene bitsets for the delegated predicates
Only matching rows are returned

Key changes

SPI layer (analytics-framework)

Introduced FilterDelegationHandle — the contract between driving and accepting backends for predicate evaluation
Added DelegationDescriptor to carry delegation metadata (tree shape, predicate count, expressions) through the wire protocol
New instruction node type for shard scans that carry delegation context

Core dispatch (analytics-engine)

Extended the data node dispatch loop to perform handle exchange after instruction handlers complete
Threaded IndexShard context (MapperService, IndexSettings, NamedWriteableRegistry) to the execution context for query compilation

DataFusion backend (driving side)

New instruction handler that creates a SessionContext pre-configured with a delegated predicate UDF and indexed table provider
FilterTreeCallbacks rewritten to route all FFM upcalls through the delegation handle

Lucene backend (accepting side)

Implements FilterDelegationHandle — compiles query expressions into Lucene Queries, creates Weights/Scorers, and produces FixedBitSets as MemorySegments
Generic relevance function serialization via QuerySerializerRegistry and ConversionUtils
Registered as an SPI extension of the analytics engine

Rust (DataFusion native)

BoolNode::Collector now uses annotation_id: i32 (was query_bytes: Arc<[u8]>)
New create_session_context_indexed FFM entry point for pre-configured indexed execution
Routing in df_execute_with_context based on whether the session has indexed config

Test infrastructure

Moved CoordinatorReduceIT tests from DF's internalClusterTest to the QA module (avoids shared-classloader SPI conflicts)
Arrow stream alignment fix for multi-node transport serialization
New FilterDelegationIT — end-to-end integration test validating the full delegation path

Credits

Co-authored with @bharath-techie who contributed:

Arrow stream alignment fix for multi-node clusters
Session context handle lifecycle management (consume-on-execute pattern)
Migration of coordinator reduce tests to the QA module

Testing

All sandbox unit tests pass
QA integration tests pass (FilterDelegationIT, WhereCommandIT, SortCommandIT, EvalCommandIT, CoordinatorReduceIT, CoordinatorReduceMemtableIT, StreamingCoordinatorReduceIT)
Rust tests pass (435/435)

github-actions · 2026-05-07T23:34:16Z

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit d931545.

Path	Line	Severity	Description
sandbox/plugins/analytics-backend-datafusion/build.gradle	88	high	External Maven dependency removal: three `org.opensearch.query` artifacts (`unified-query-api`, `unified-query-core`, `unified-query-ppl` at version `3.6.0.0-SNAPSHOT`) are removed from `internalClusterTestImplementation`. Per mandatory flagging rule, any dependency change — including removals — must be verified by maintainers to confirm these are genuinely internal OpenSearch coordinates, no artifact substitution occurred, and the corresponding tests have been correctly migrated rather than silently dropped.
sandbox/plugins/analytics-backend-datafusion/rust/src/indexed_executor.rs	450	low	Empty `for (i, seg) in segments.iter().enumerate() {}` loop with no body. The loop iterates over all segments but performs no action. This appears to be a leftover debugging stub; it is a no-op and not malicious, but is an anomaly worth investigating to confirm no logic was accidentally omitted.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 1 | Medium: 0 | Low: 1

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

…a node execution Signed-off-by: expani <anijainc@amazon.com>

Signed-off-by: expani <anijainc@amazon.com>

Signed-off-by: bharath-techie <bharath78910@gmail.com> Signed-off-by: expani <anijainc@amazon.com>

…ne test classpath Signed-off-by: expani <anijainc@amazon.com>

github-actions · 2026-05-08T02:19:57Z

✅ Gradle check result for d931545: SUCCESS

codecov · 2026-05-08T02:20:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (66e5a70) to head (d931545).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #21555   +/-   ##
=========================================
  Coverage     73.51%   73.52%           
- Complexity    74598    74636   +38     
=========================================
  Files          5978     5975    -3     
  Lines        338740   338724   -16     
  Branches      48842    48842           
=========================================
+ Hits         249016   249034   +18     
- Misses        69876    69881    +5     
+ Partials      19848    19809   -39

☔ View full report in Codecov by Sentry.
📢 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.

…erStageScanRowType Single-line spotless reformat in DataFusionFragmentConvertorTests: the OpenSearchStageInputScan constructor call now fits on one line (it was previously broken across multiple lines). Originally bumped sandbox/plugins/analytics-backend-datafusion's sqlUnifiedQueryVersion 3.6 -> 3.7 to align with test-ppl-frontend, but the entire internalClusterTest classpath block (including that pin) was removed upstream by opensearch-project#21555 (dbe4a42, "Enable Lucene Filter delegation from Datafusion for Correctness"). The build.gradle hunk dropped during rebase; only the spotless reformat survives. Signed-off-by: Kai Huang <ahkcs@amazon.com>

…ntyped-NULL fixes for multisearch (#21528) * [Analytics Backend / DataFusion] Fix Plan.Root.names mismatch for schema-reshaping wrappers `DataFusionFragmentConvertor.rewire` always populated the new `Plan.Root.names` list with the *inner* plan's names. For schema-preserving wrappers (Sort, Filter, Fetch) those happen to coincide with the wrapper's output schema, so the bug was hidden. For schema-reshaping wrappers (Aggregate, Project) the wrapper's output width differs from the inner's, and DataFusion's substrait consumer rejects the plan in `make_renamed_schema` with: Substrait error: Names list must match exactly to nested schema, but found {wrapper-width} uses for {inner-width} names This shape is hit by every PPL `multisearch` query whose coordinator stage is `Sort(Aggregate(Union(StageInputScan, StageInputScan)))` — the Aggregate narrows the wide Union row type, and the inner-names override surfaced the mismatch as a 500. Fix: derive the new `Plan.Root.names` from the wrapper RelNode's row type (`fragment.getRowType().getFieldList()`), not the inner plan. Both `attachFragmentOnTop` and `attachPartialAggOnTop` already have the wrapper RelNode in scope, so this is a local change with no signature ripple beyond adding a `List<String> wrapperNames` parameter to `rewire`. Test coverage: - `testAttachPartialAggOnTop_PlanRootNamesMatchWrapperOutput` — the partial-agg path with a 3-column inner scan and a 1-column wrapper aggregate; pins names to the wrapper's output. - `testAttachFragmentOnTop_AggregateOverMultiColumnInner_PlanRootNamesMatchWrapperOutput` — the multisearch coordinator-stage shape (`Aggregate(Union)`). - `testMultisearchShape_SortOverAggregateOverThreeWayUnion_PlanRootNamesMatchTopOutput` — full chain `Sort → Aggregate → Union(Sin × 3)` modeling the `testMultisearchWithThreeSubsearches` query plan. - `testMultisearchShape_SystemLimitOverSortOverAggregateOverUnion_NamesMatchTopOutput` — adds the implicit `LogicalSystemLimit` wrapper that `QueryService.convertToCalcitePlan` injects at the top of every analytics-engine plan, lowered to a Substrait `Fetch`. End-to-end validation against `:integ-test:integTestRemote --tests 'org.opensearch.sql.calcite.remote.CalciteMultisearchCommandIT'` with `-Dtests.analytics.force_routing=true -Dtests.analytics.parquet_indices=true`: 21 tests, **5 pass** (was 0/21 before this change). The remaining 16 are blocked on orthogonal issues that are out of scope here: - `Field [...] not found.` analyzer errors (8 tests) — pre-existing parquet-backed-index field-resolution gap. - TIMESTAMP / SPAN scalar functions unsupported (4 tests). - AssertionError on error-message format (2 tests). - One residual `Names list must match exactly to nested schema, but found 2 uses for 6 names` on `testMultisearchWithThreeSubsearches` — likely a different code path (PARTIAL/FINAL split via `OpenSearchAggregateSplitRule`) that the convertor unit tests don't exercise; tracked for follow-up. - Two timeouts/long-running on the largest queries. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [QA] Add MultisearchCommandIT for the analytics-engine REST path Lands a self-contained QA IT covering PPL `multisearch` so the analytics-route fix in this PR is exercised inside core without cross-plugin dependencies on the SQL plugin. Three tests, scoped to the surface analytics already supports end-to-end: | Test | Shape | |---|---| | `testMultisearchTwoBranchesByCategory` | Basic 2-way Union over int0 buckets — `Union(Filter+Eval+Project, Filter+Eval+Project)` followed by `Aggregate(count by) | sort`. Exercises the same convertReduceFragment chain (`attachFragmentOnTop(Sort, attachFragmentOnTop(Aggregate, convertFinalAggFragment(Union)))`) that the rewire fix targets. | | `testMultisearchThreeBranchesByStr0` | 3-way Union — the exact `Union(ER, ER, ER)` shape that surfaced the residual "2 uses for 6 names" failure I'd flagged as a follow-up in an earlier draft of the PR description; the rewire fix already covers it on a fresh cluster. | | `testMultisearchSingleSubsearchRejected` | Arity check — pinned at the parser layer (AstBuilder.visitMultisearchCommand rejects <2 subsearches with `SyntaxCheckException`). Regression-pin against accidental relaxation of that guard. | Each branch projects to a scalar-only field set (`fields int0, class` / `fields str0, bucket`) so the union row type sidesteps the calcs dataset's date/time/datetime columns — `ArrowSchemaFromCalcite.toArrowType` doesn't yet handle TIMESTAMP, tracked separately. Bumps `test-ppl-frontend`'s `unified-query-*` dependency from 3.6.0.0-SNAPSHOT to 3.7.0.0-SNAPSHOT so the bundled PPL grammar exposes the `multisearch` keyword (along with table/regex/rex/convert added since 3.6). The SQL Snapshots repo (already declared in the build) carries the published 3.7 artifacts; for local sql-repo HEAD development, run `./gradlew :ppl:publishUnifiedQueryPublicationToMavenLocal` from the sql repo. The version bump is independent of the in-flight test-ppl-frontend UnifiedQueryService.setting() change in #21526 — different files, no conflict. Validates: 3/3 MultisearchCommandIT pass; full `:sandbox:qa:analytics-engine-rest:integTest` suite still green (110 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Spotless reformat of testFinalAggInnerStageScanRowType Single-line spotless reformat in DataFusionFragmentConvertorTests: the OpenSearchStageInputScan constructor call now fits on one line (it was previously broken across multiple lines). Originally bumped sandbox/plugins/analytics-backend-datafusion's sqlUnifiedQueryVersion 3.6 -> 3.7 to align with test-ppl-frontend, but the entire internalClusterTest classpath block (including that pin) was removed upstream by #21555 (dbe4a42, "Enable Lucene Filter delegation from Datafusion for Correctness"). The build.gradle hunk dropped during rebase; only the spotless reformat survives. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Register CASE in project capabilities + QA IT Calcite emits SqlKind.CASE for any conditional expression — explicit `eval x = case(cond, val, …)` in PPL, plus the `count(eval(predicate))` conditional-count idiom (lowered to COUNT(CASE WHEN predicate THEN … END)) and several other shapes. Without CASE in `STANDARD_PROJECT_OPS`, the analytics planner rejected the operator with `No backend supports scalar function [CASE] among [datafusion]` before substrait emission. CASE doesn't need a backend adapter: isthmus translates SqlKind.CASE structurally to a Substrait IfThen rel, and DataFusion's substrait consumer handles IfThen natively. Just registering the capability is enough. Adds `testMultisearchEvalCaseProjection` to MultisearchCommandIT to pin the end-to-end path — multisearch + `eval bucket = case(cond, val else default)` + stats. Uses an explicit `else` arm so isthmus doesn't have to convert an untyped NULL literal; the implicit-else `count(eval(…))` shape that the v2-side testMultisearchSuccessRatePattern uses still hits a separate isthmus limitation (`Unable to convert the type NULL` from `TypeConverter` on a SqlTypeName.NULL literal — tracked separately, out of scope here). Validates: 4/4 MultisearchCommandIT pass; full :sandbox:qa:analytics-engine-rest:integTest suite still green (111 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Pre-isthmus untyped-NULL rewriter for CASE arms Calcite emits a `RexLiteral` with `SqlTypeName.NULL` for the implicit ELSE arm of `CASE WHEN cond THEN val END` — exactly the shape PPL `count(eval(predicate))` lowers to (`COUNT(CASE WHEN predicate THEN <projected> END)`). Isthmus' `TypeConverter.toSubstrait` rejects `SqlTypeName.NULL` with `Unable to convert the type NULL`, blocking the analytics path before substrait emission. Adds `UntypedNullPreprocessor` — a `RelHomogeneousShuttle` + `RexShuttle` pass applied in `convertToSubstrait` and `convertStandalone` *before* the SubstraitRelVisitor sees the plan. Walks every CASE call's value operands (THEN arms and the ELSE arm) and substitutes any `SqlTypeName.NULL` literal with a typed null literal matching the CASE's resolved return type. Calcite already widens the CASE's return type to the leastRestrictive of branches, so the substituted type is correct by construction. Scope is intentionally narrow: only CASE call operands are rewritten today. Other untyped-NULL contexts (function arguments, comparison RHS) are rare in PPL-generated plans and would need per-operator type inference to do safely; defer until a concrete test surfaces one. Test coverage: - `UntypedNullPreprocessorTests` (4 new): * `testCountEvalCaseRewritesElseNullToTypedNull` — the motivating shape `COUNT(CASE WHEN cond THEN 1 ELSE null END)`. * `testCaseWithThenNullIsAlsoRewritten` — null in the THEN arm. * `testCaseConditionOperandUnchanged` — even-index condition operands left alone. * `testCountOverRewrittenCaseProjectionTypechecks` — Aggregate(Project(CASE)) with the rewriter applied still type-checks end-to-end. - New `testMultisearchCountEvalConditionalCount` in MultisearchCommandIT — mirrors the v2-side `CalciteMultisearchCommandIT.testMultisearchSuccessRatePattern` shape (`count(eval(predicate))`) end-to-end on the analytics-engine REST path. Validates: 5/5 MultisearchCommandIT pass; 4/4 new + 12/12 existing FragmentConvertor unit tests; full :sandbox:qa:analytics-engine-rest:integTest suite still green (112 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Simplify testCaseConditionOperandUnchanged for spotless The original assertion wrapped a no-op single-iteration loop around an empty- body RexShuttle whose accept() result is just caseExpr.toString(). Spotless flagged the empty class body (`new RexShuttle() {}`) as a formatting violation and tried to wrap it across two lines, which read worse than the underlying intent — comparing the input CASE expression to the rewriter's output to prove no-op behavior when no untyped nulls are present. Replace the loop+shuttle with a direct `assertEquals(caseExpr.toString(), rewrittenCase.toString())` — same semantics, cleaner code, no awkward formatting. The test still asserts the rewriter doesn't touch CASE expressions whose operands are already typed. Sandbox check (`./gradlew check -p sandbox -Dsandbox.enabled=true`) now passes end-to-end. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>

…search-project#21555)

…ntyped-NULL fixes for multisearch (opensearch-project#21528) * [Analytics Backend / DataFusion] Fix Plan.Root.names mismatch for schema-reshaping wrappers `DataFusionFragmentConvertor.rewire` always populated the new `Plan.Root.names` list with the *inner* plan's names. For schema-preserving wrappers (Sort, Filter, Fetch) those happen to coincide with the wrapper's output schema, so the bug was hidden. For schema-reshaping wrappers (Aggregate, Project) the wrapper's output width differs from the inner's, and DataFusion's substrait consumer rejects the plan in `make_renamed_schema` with: Substrait error: Names list must match exactly to nested schema, but found {wrapper-width} uses for {inner-width} names This shape is hit by every PPL `multisearch` query whose coordinator stage is `Sort(Aggregate(Union(StageInputScan, StageInputScan)))` — the Aggregate narrows the wide Union row type, and the inner-names override surfaced the mismatch as a 500. Fix: derive the new `Plan.Root.names` from the wrapper RelNode's row type (`fragment.getRowType().getFieldList()`), not the inner plan. Both `attachFragmentOnTop` and `attachPartialAggOnTop` already have the wrapper RelNode in scope, so this is a local change with no signature ripple beyond adding a `List<String> wrapperNames` parameter to `rewire`. Test coverage: - `testAttachPartialAggOnTop_PlanRootNamesMatchWrapperOutput` — the partial-agg path with a 3-column inner scan and a 1-column wrapper aggregate; pins names to the wrapper's output. - `testAttachFragmentOnTop_AggregateOverMultiColumnInner_PlanRootNamesMatchWrapperOutput` — the multisearch coordinator-stage shape (`Aggregate(Union)`). - `testMultisearchShape_SortOverAggregateOverThreeWayUnion_PlanRootNamesMatchTopOutput` — full chain `Sort → Aggregate → Union(Sin × 3)` modeling the `testMultisearchWithThreeSubsearches` query plan. - `testMultisearchShape_SystemLimitOverSortOverAggregateOverUnion_NamesMatchTopOutput` — adds the implicit `LogicalSystemLimit` wrapper that `QueryService.convertToCalcitePlan` injects at the top of every analytics-engine plan, lowered to a Substrait `Fetch`. End-to-end validation against `:integ-test:integTestRemote --tests 'org.opensearch.sql.calcite.remote.CalciteMultisearchCommandIT'` with `-Dtests.analytics.force_routing=true -Dtests.analytics.parquet_indices=true`: 21 tests, **5 pass** (was 0/21 before this change). The remaining 16 are blocked on orthogonal issues that are out of scope here: - `Field [...] not found.` analyzer errors (8 tests) — pre-existing parquet-backed-index field-resolution gap. - TIMESTAMP / SPAN scalar functions unsupported (4 tests). - AssertionError on error-message format (2 tests). - One residual `Names list must match exactly to nested schema, but found 2 uses for 6 names` on `testMultisearchWithThreeSubsearches` — likely a different code path (PARTIAL/FINAL split via `OpenSearchAggregateSplitRule`) that the convertor unit tests don't exercise; tracked for follow-up. - Two timeouts/long-running on the largest queries. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [QA] Add MultisearchCommandIT for the analytics-engine REST path Lands a self-contained QA IT covering PPL `multisearch` so the analytics-route fix in this PR is exercised inside core without cross-plugin dependencies on the SQL plugin. Three tests, scoped to the surface analytics already supports end-to-end: | Test | Shape | |---|---| | `testMultisearchTwoBranchesByCategory` | Basic 2-way Union over int0 buckets — `Union(Filter+Eval+Project, Filter+Eval+Project)` followed by `Aggregate(count by) | sort`. Exercises the same convertReduceFragment chain (`attachFragmentOnTop(Sort, attachFragmentOnTop(Aggregate, convertFinalAggFragment(Union)))`) that the rewire fix targets. | | `testMultisearchThreeBranchesByStr0` | 3-way Union — the exact `Union(ER, ER, ER)` shape that surfaced the residual "2 uses for 6 names" failure I'd flagged as a follow-up in an earlier draft of the PR description; the rewire fix already covers it on a fresh cluster. | | `testMultisearchSingleSubsearchRejected` | Arity check — pinned at the parser layer (AstBuilder.visitMultisearchCommand rejects <2 subsearches with `SyntaxCheckException`). Regression-pin against accidental relaxation of that guard. | Each branch projects to a scalar-only field set (`fields int0, class` / `fields str0, bucket`) so the union row type sidesteps the calcs dataset's date/time/datetime columns — `ArrowSchemaFromCalcite.toArrowType` doesn't yet handle TIMESTAMP, tracked separately. Bumps `test-ppl-frontend`'s `unified-query-*` dependency from 3.6.0.0-SNAPSHOT to 3.7.0.0-SNAPSHOT so the bundled PPL grammar exposes the `multisearch` keyword (along with table/regex/rex/convert added since 3.6). The SQL Snapshots repo (already declared in the build) carries the published 3.7 artifacts; for local sql-repo HEAD development, run `./gradlew :ppl:publishUnifiedQueryPublicationToMavenLocal` from the sql repo. The version bump is independent of the in-flight test-ppl-frontend UnifiedQueryService.setting() change in opensearch-project#21526 — different files, no conflict. Validates: 3/3 MultisearchCommandIT pass; full `:sandbox:qa:analytics-engine-rest:integTest` suite still green (110 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Spotless reformat of testFinalAggInnerStageScanRowType Single-line spotless reformat in DataFusionFragmentConvertorTests: the OpenSearchStageInputScan constructor call now fits on one line (it was previously broken across multiple lines). Originally bumped sandbox/plugins/analytics-backend-datafusion's sqlUnifiedQueryVersion 3.6 -> 3.7 to align with test-ppl-frontend, but the entire internalClusterTest classpath block (including that pin) was removed upstream by opensearch-project#21555 (dbe4a42, "Enable Lucene Filter delegation from Datafusion for Correctness"). The build.gradle hunk dropped during rebase; only the spotless reformat survives. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Register CASE in project capabilities + QA IT Calcite emits SqlKind.CASE for any conditional expression — explicit `eval x = case(cond, val, …)` in PPL, plus the `count(eval(predicate))` conditional-count idiom (lowered to COUNT(CASE WHEN predicate THEN … END)) and several other shapes. Without CASE in `STANDARD_PROJECT_OPS`, the analytics planner rejected the operator with `No backend supports scalar function [CASE] among [datafusion]` before substrait emission. CASE doesn't need a backend adapter: isthmus translates SqlKind.CASE structurally to a Substrait IfThen rel, and DataFusion's substrait consumer handles IfThen natively. Just registering the capability is enough. Adds `testMultisearchEvalCaseProjection` to MultisearchCommandIT to pin the end-to-end path — multisearch + `eval bucket = case(cond, val else default)` + stats. Uses an explicit `else` arm so isthmus doesn't have to convert an untyped NULL literal; the implicit-else `count(eval(…))` shape that the v2-side testMultisearchSuccessRatePattern uses still hits a separate isthmus limitation (`Unable to convert the type NULL` from `TypeConverter` on a SqlTypeName.NULL literal — tracked separately, out of scope here). Validates: 4/4 MultisearchCommandIT pass; full :sandbox:qa:analytics-engine-rest:integTest suite still green (111 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Pre-isthmus untyped-NULL rewriter for CASE arms Calcite emits a `RexLiteral` with `SqlTypeName.NULL` for the implicit ELSE arm of `CASE WHEN cond THEN val END` — exactly the shape PPL `count(eval(predicate))` lowers to (`COUNT(CASE WHEN predicate THEN <projected> END)`). Isthmus' `TypeConverter.toSubstrait` rejects `SqlTypeName.NULL` with `Unable to convert the type NULL`, blocking the analytics path before substrait emission. Adds `UntypedNullPreprocessor` — a `RelHomogeneousShuttle` + `RexShuttle` pass applied in `convertToSubstrait` and `convertStandalone` *before* the SubstraitRelVisitor sees the plan. Walks every CASE call's value operands (THEN arms and the ELSE arm) and substitutes any `SqlTypeName.NULL` literal with a typed null literal matching the CASE's resolved return type. Calcite already widens the CASE's return type to the leastRestrictive of branches, so the substituted type is correct by construction. Scope is intentionally narrow: only CASE call operands are rewritten today. Other untyped-NULL contexts (function arguments, comparison RHS) are rare in PPL-generated plans and would need per-operator type inference to do safely; defer until a concrete test surfaces one. Test coverage: - `UntypedNullPreprocessorTests` (4 new): * `testCountEvalCaseRewritesElseNullToTypedNull` — the motivating shape `COUNT(CASE WHEN cond THEN 1 ELSE null END)`. * `testCaseWithThenNullIsAlsoRewritten` — null in the THEN arm. * `testCaseConditionOperandUnchanged` — even-index condition operands left alone. * `testCountOverRewrittenCaseProjectionTypechecks` — Aggregate(Project(CASE)) with the rewriter applied still type-checks end-to-end. - New `testMultisearchCountEvalConditionalCount` in MultisearchCommandIT — mirrors the v2-side `CalciteMultisearchCommandIT.testMultisearchSuccessRatePattern` shape (`count(eval(predicate))`) end-to-end on the analytics-engine REST path. Validates: 5/5 MultisearchCommandIT pass; 4/4 new + 12/12 existing FragmentConvertor unit tests; full :sandbox:qa:analytics-engine-rest:integTest suite still green (112 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Simplify testCaseConditionOperandUnchanged for spotless The original assertion wrapped a no-op single-iteration loop around an empty- body RexShuttle whose accept() result is just caseExpr.toString(). Spotless flagged the empty class body (`new RexShuttle() {}`) as a formatting violation and tried to wrap it across two lines, which read worse than the underlying intent — comparing the input CASE expression to the rewriter's output to prove no-op behavior when no untyped nulls are present. Replace the loop+shuttle with a direct `assertEquals(caseExpr.toString(), rewrittenCase.toString())` — same semantics, cleaner code, no awkward formatting. The test still asserts the rewriter doesn't touch CASE expressions whose operands are already typed. Sandbox check (`./gradlew check -p sandbox -Dsandbox.enabled=true`) now passes end-to-end. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>

…search-project#21555)

…ntyped-NULL fixes for multisearch (opensearch-project#21528) * [Analytics Backend / DataFusion] Fix Plan.Root.names mismatch for schema-reshaping wrappers `DataFusionFragmentConvertor.rewire` always populated the new `Plan.Root.names` list with the *inner* plan's names. For schema-preserving wrappers (Sort, Filter, Fetch) those happen to coincide with the wrapper's output schema, so the bug was hidden. For schema-reshaping wrappers (Aggregate, Project) the wrapper's output width differs from the inner's, and DataFusion's substrait consumer rejects the plan in `make_renamed_schema` with: Substrait error: Names list must match exactly to nested schema, but found {wrapper-width} uses for {inner-width} names This shape is hit by every PPL `multisearch` query whose coordinator stage is `Sort(Aggregate(Union(StageInputScan, StageInputScan)))` — the Aggregate narrows the wide Union row type, and the inner-names override surfaced the mismatch as a 500. Fix: derive the new `Plan.Root.names` from the wrapper RelNode's row type (`fragment.getRowType().getFieldList()`), not the inner plan. Both `attachFragmentOnTop` and `attachPartialAggOnTop` already have the wrapper RelNode in scope, so this is a local change with no signature ripple beyond adding a `List<String> wrapperNames` parameter to `rewire`. Test coverage: - `testAttachPartialAggOnTop_PlanRootNamesMatchWrapperOutput` — the partial-agg path with a 3-column inner scan and a 1-column wrapper aggregate; pins names to the wrapper's output. - `testAttachFragmentOnTop_AggregateOverMultiColumnInner_PlanRootNamesMatchWrapperOutput` — the multisearch coordinator-stage shape (`Aggregate(Union)`). - `testMultisearchShape_SortOverAggregateOverThreeWayUnion_PlanRootNamesMatchTopOutput` — full chain `Sort → Aggregate → Union(Sin × 3)` modeling the `testMultisearchWithThreeSubsearches` query plan. - `testMultisearchShape_SystemLimitOverSortOverAggregateOverUnion_NamesMatchTopOutput` — adds the implicit `LogicalSystemLimit` wrapper that `QueryService.convertToCalcitePlan` injects at the top of every analytics-engine plan, lowered to a Substrait `Fetch`. End-to-end validation against `:integ-test:integTestRemote --tests 'org.opensearch.sql.calcite.remote.CalciteMultisearchCommandIT'` with `-Dtests.analytics.force_routing=true -Dtests.analytics.parquet_indices=true`: 21 tests, **5 pass** (was 0/21 before this change). The remaining 16 are blocked on orthogonal issues that are out of scope here: - `Field [...] not found.` analyzer errors (8 tests) — pre-existing parquet-backed-index field-resolution gap. - TIMESTAMP / SPAN scalar functions unsupported (4 tests). - AssertionError on error-message format (2 tests). - One residual `Names list must match exactly to nested schema, but found 2 uses for 6 names` on `testMultisearchWithThreeSubsearches` — likely a different code path (PARTIAL/FINAL split via `OpenSearchAggregateSplitRule`) that the convertor unit tests don't exercise; tracked for follow-up. - Two timeouts/long-running on the largest queries. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [QA] Add MultisearchCommandIT for the analytics-engine REST path Lands a self-contained QA IT covering PPL `multisearch` so the analytics-route fix in this PR is exercised inside core without cross-plugin dependencies on the SQL plugin. Three tests, scoped to the surface analytics already supports end-to-end: | Test | Shape | |---|---| | `testMultisearchTwoBranchesByCategory` | Basic 2-way Union over int0 buckets — `Union(Filter+Eval+Project, Filter+Eval+Project)` followed by `Aggregate(count by) | sort`. Exercises the same convertReduceFragment chain (`attachFragmentOnTop(Sort, attachFragmentOnTop(Aggregate, convertFinalAggFragment(Union)))`) that the rewire fix targets. | | `testMultisearchThreeBranchesByStr0` | 3-way Union — the exact `Union(ER, ER, ER)` shape that surfaced the residual "2 uses for 6 names" failure I'd flagged as a follow-up in an earlier draft of the PR description; the rewire fix already covers it on a fresh cluster. | | `testMultisearchSingleSubsearchRejected` | Arity check — pinned at the parser layer (AstBuilder.visitMultisearchCommand rejects <2 subsearches with `SyntaxCheckException`). Regression-pin against accidental relaxation of that guard. | Each branch projects to a scalar-only field set (`fields int0, class` / `fields str0, bucket`) so the union row type sidesteps the calcs dataset's date/time/datetime columns — `ArrowSchemaFromCalcite.toArrowType` doesn't yet handle TIMESTAMP, tracked separately. Bumps `test-ppl-frontend`'s `unified-query-*` dependency from 3.6.0.0-SNAPSHOT to 3.7.0.0-SNAPSHOT so the bundled PPL grammar exposes the `multisearch` keyword (along with table/regex/rex/convert added since 3.6). The SQL Snapshots repo (already declared in the build) carries the published 3.7 artifacts; for local sql-repo HEAD development, run `./gradlew :ppl:publishUnifiedQueryPublicationToMavenLocal` from the sql repo. The version bump is independent of the in-flight test-ppl-frontend UnifiedQueryService.setting() change in opensearch-project#21526 — different files, no conflict. Validates: 3/3 MultisearchCommandIT pass; full `:sandbox:qa:analytics-engine-rest:integTest` suite still green (110 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Spotless reformat of testFinalAggInnerStageScanRowType Single-line spotless reformat in DataFusionFragmentConvertorTests: the OpenSearchStageInputScan constructor call now fits on one line (it was previously broken across multiple lines). Originally bumped sandbox/plugins/analytics-backend-datafusion's sqlUnifiedQueryVersion 3.6 -> 3.7 to align with test-ppl-frontend, but the entire internalClusterTest classpath block (including that pin) was removed upstream by opensearch-project#21555 (dbe4a42, "Enable Lucene Filter delegation from Datafusion for Correctness"). The build.gradle hunk dropped during rebase; only the spotless reformat survives. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Register CASE in project capabilities + QA IT Calcite emits SqlKind.CASE for any conditional expression — explicit `eval x = case(cond, val, …)` in PPL, plus the `count(eval(predicate))` conditional-count idiom (lowered to COUNT(CASE WHEN predicate THEN … END)) and several other shapes. Without CASE in `STANDARD_PROJECT_OPS`, the analytics planner rejected the operator with `No backend supports scalar function [CASE] among [datafusion]` before substrait emission. CASE doesn't need a backend adapter: isthmus translates SqlKind.CASE structurally to a Substrait IfThen rel, and DataFusion's substrait consumer handles IfThen natively. Just registering the capability is enough. Adds `testMultisearchEvalCaseProjection` to MultisearchCommandIT to pin the end-to-end path — multisearch + `eval bucket = case(cond, val else default)` + stats. Uses an explicit `else` arm so isthmus doesn't have to convert an untyped NULL literal; the implicit-else `count(eval(…))` shape that the v2-side testMultisearchSuccessRatePattern uses still hits a separate isthmus limitation (`Unable to convert the type NULL` from `TypeConverter` on a SqlTypeName.NULL literal — tracked separately, out of scope here). Validates: 4/4 MultisearchCommandIT pass; full :sandbox:qa:analytics-engine-rest:integTest suite still green (111 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Pre-isthmus untyped-NULL rewriter for CASE arms Calcite emits a `RexLiteral` with `SqlTypeName.NULL` for the implicit ELSE arm of `CASE WHEN cond THEN val END` — exactly the shape PPL `count(eval(predicate))` lowers to (`COUNT(CASE WHEN predicate THEN <projected> END)`). Isthmus' `TypeConverter.toSubstrait` rejects `SqlTypeName.NULL` with `Unable to convert the type NULL`, blocking the analytics path before substrait emission. Adds `UntypedNullPreprocessor` — a `RelHomogeneousShuttle` + `RexShuttle` pass applied in `convertToSubstrait` and `convertStandalone` *before* the SubstraitRelVisitor sees the plan. Walks every CASE call's value operands (THEN arms and the ELSE arm) and substitutes any `SqlTypeName.NULL` literal with a typed null literal matching the CASE's resolved return type. Calcite already widens the CASE's return type to the leastRestrictive of branches, so the substituted type is correct by construction. Scope is intentionally narrow: only CASE call operands are rewritten today. Other untyped-NULL contexts (function arguments, comparison RHS) are rare in PPL-generated plans and would need per-operator type inference to do safely; defer until a concrete test surfaces one. Test coverage: - `UntypedNullPreprocessorTests` (4 new): * `testCountEvalCaseRewritesElseNullToTypedNull` — the motivating shape `COUNT(CASE WHEN cond THEN 1 ELSE null END)`. * `testCaseWithThenNullIsAlsoRewritten` — null in the THEN arm. * `testCaseConditionOperandUnchanged` — even-index condition operands left alone. * `testCountOverRewrittenCaseProjectionTypechecks` — Aggregate(Project(CASE)) with the rewriter applied still type-checks end-to-end. - New `testMultisearchCountEvalConditionalCount` in MultisearchCommandIT — mirrors the v2-side `CalciteMultisearchCommandIT.testMultisearchSuccessRatePattern` shape (`count(eval(predicate))`) end-to-end on the analytics-engine REST path. Validates: 5/5 MultisearchCommandIT pass; 4/4 new + 12/12 existing FragmentConvertor unit tests; full :sandbox:qa:analytics-engine-rest:integTest suite still green (112 tests across 14 ITs). Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Simplify testCaseConditionOperandUnchanged for spotless The original assertion wrapped a no-op single-iteration loop around an empty- body RexShuttle whose accept() result is just caseExpr.toString(). Spotless flagged the empty class body (`new RexShuttle() {}`) as a formatting violation and tried to wrap it across two lines, which read worse than the underlying intent — comparing the input CASE expression to the rewriter's output to prove no-op behavior when no untyped nulls are present. Replace the loop+shuttle with a direct `assertEquals(caseExpr.toString(), rewrittenCase.toString())` — same semantics, cleaner code, no awkward formatting. The test still asserts the rewriter doesn't touch CASE expressions whose operands are already typed. Sandbox check (`./gradlew check -p sandbox -Dsandbox.enabled=true`) now passes end-to-end. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>

expani requested a review from a team as a code owner May 7, 2026 23:32

expani and others added 6 commits May 7, 2026 18:08

Integrated InstructionHandler and introduce delegation handle for dat…

5b801e8

…a node execution Signed-off-by: expani <anijainc@amazon.com>

Added Filter delegation E2E from Datafusion into Lucene

af60995

Signed-off-by: expani <anijainc@amazon.com>

Cherry picked commits from older branch and fixed compile issues

e9b8877

Signed-off-by: expani <anijainc@amazon.com>

Cherry picked from 14e05ff and changed ndefault numberOfNodes to 1

c90ccb0

Signed-off-by: expani <anijainc@amazon.com>

fixes for handling session context

3d4bcd7

Signed-off-by: bharath-techie <bharath78910@gmail.com> Signed-off-by: expani <anijainc@amazon.com>

Fix logger usage checks and add analytics-framework to composite-engi…

d931545

…ne test classpath Signed-off-by: expani <anijainc@amazon.com>

expani force-pushed the ka_filter_delegation branch from e80143b to d931545 Compare May 8, 2026 01:11

mch2 added skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels May 8, 2026

mch2 reviewed May 8, 2026

View reviewed changes

Comment thread sandbox/plugins/analytics-backend-datafusion/rust/src/session_context.rs

mch2 reviewed May 8, 2026

View reviewed changes

Comment thread sandbox/qa/analytics-engine-rest/build.gradle

mch2 approved these changes May 8, 2026

View reviewed changes

mch2 merged commit dbe4a42 into opensearch-project:main May 8, 2026
39 of 41 checks passed

imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026

Enable Lucene Filter delegation from Datafusion for Correctness (open…

912fdf3

…search-project#21555)

Bukhtawar pushed a commit to Bukhtawar/OpenSearch that referenced this pull request May 10, 2026

Enable Lucene Filter delegation from Datafusion for Correctness (open…

eeb411e

…search-project#21555)

sandeshkr419 mentioned this pull request May 11, 2026

[draft] LBP initial interface #20917

Closed

3 tasks

Navigation Menu

Uh oh!

Enable Lucene Filter delegation from Datafusion for Correctness - #21555

Enable Lucene Filter delegation from Datafusion for Correctness#21555
mch2 merged 6 commits into
opensearch-project:mainfrom
expani:ka_filter_delegation

expani commented May 7, 2026

Uh oh!

github-actions Bot commented May 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

codecov Bot commented May 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

expani commented May 7, 2026

Description

What this enables

Key changes

Credits

Testing

Uh oh!

github-actions Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Analyzer ❗

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

codecov Bot commented May 8, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented May 7, 2026 •

edited

Loading