Enable PPL fillnull on the analytics-engine route via DataFusion COALESCE - #21472
Conversation
DataFusion natively supports CEIL; declaring it as a project capability lets the analytics-engine planner route Project nodes containing CEIL through DataFusion. Same pattern as the COALESCE addition: one entry in STANDARD_PROJECT_OPS, no convertor changes needed (Substrait default extensions handle the conversion). Exercised by CalciteFillNullCommandIT.testFillNullWithFunctionOnOtherField, which calls 'fillnull with ceil(num1) in num0' — the COALESCE replacement is a CEIL of another field. Without CEIL declared project-capable, the planner would fail with 'No backend supports scalar function [CEIL] among [datafusion]' even after COALESCE is wired. CEIL is also one of the ~100 Bucket-1 functions in the PPL→DataFusion audit (direct DataFusion mapping, S0). This commit is the templated shape for any other Bucket-1 scalar — add the constant to STANDARD_PROJECT_OPS, no other changes. Signed-off-by: Kai Huang <ahkcs@amazon.com>
PR Reviewer Guide 🔍(Review updated until commit dc560ab)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to dc560ab Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit cd8e6e2
Suggestions up to commit f4188dd
Suggestions up to commit 350b67d
Suggestions up to commit 349c68f
|
…ecursively OpenSearchProjectRule.annotateExpr recurses into a project call's operands, wrapping every sub-call in an AnnotatedProjectExpression so the planner can track viable backends per sub-expression. The previous OpenSearchProject.stripAnnotations only unwrapped the top-level wrapper — for a project like PLUS(CEIL(value), value), the strip left an inner Annotated(CEIL(...)) sitting inside the outer call. Substrait isthmus has no converter for ANNOTATED_PROJECT_EXPR, so conversion failed with 'Unable to convert call ANNOTATED_PROJECT_EXPR'. Walk each project expression with a RexShuttle that recursively unwraps every AnnotatedProjectExpression it encounters before handing the plan off to the backend FragmentConvertor. The shuttle's super.visitCall keeps the operand-traversal default; the override intercepts wrappers and re-feeds the unwrapped result through the shuttle so deeply-nested wrappers also get stripped. Adds ProjectRuleTests#testStripAnnotationsRecursivelyUnwrapsNestedExpressions covering the nested-call shape. The bug is independent of which scalar functions any backend declares project-capable; it would fire for any project with a nested scalar call once both the outer and inner operators are project-capable on the same backend. Signed-off-by: Kai Huang <ahkcs@amazon.com>
…ies for PPL fillnull PPL fillnull lowers via CalciteRelNodeVisitor.visitFillNull into a LogicalProject(COALESCE(field, replacement)) tree. The analytics-engine planner's OpenSearchProjectRule rejects any project-list scalar call unless at least one viable backend declares it project-capable, with 'No backend supports scalar function [COALESCE] among [datafusion]'. DataFusion natively supports COALESCE, but the backend plugin had not declared any projectCapabilities, so every fillnull query failed the planner check before reaching Substrait conversion. Add a STANDARD_PROJECT_OPS set mirroring the existing STANDARD_FILTER_OPS pattern, seeded with COALESCE, and override projectCapabilities() to fan it out across the same field-types and formats the backend already advertises for filters. No Rust-side or convertor changes are needed — the default Substrait extension catalog loaded by DataFusionPlugin.loadSubstraitExtensions handles COALESCE natively, and DataFusion's runtime executes it directly. Lifts CalciteFillNullCommandIT on the force-routed analytics-engine path from 2/13 passing (only the SQL-plugin preflight type-check tests) to 12/13 passing. The remaining failure is testFillNullWithFunctionOnOtherField, which uses a nested CEIL inside the COALESCE replacement — handled in follow-up commits. Signed-off-by: Kai Huang <ahkcs@amazon.com>
|
Persistent review updated to latest commit 350b67d |
|
lets pls add a test in /sandbox/qa to test this as well - we're going to have a lot of functions added quickly and we'll need those kinds of tests running on os core builds to prevent regression. |
|
❌ Gradle check result for 350b67d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Persistent review updated to latest commit f4188dd |
Done — added |
|
Persistent review updated to latest commit cd8e6e2 |
|
❌ Gradle check result for cd8e6e2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Self-contained integration test for PPL fillnull on the analytics-engine
route, mirroring CalciteFillNullCommandIT from opensearch-project/sql so
the analytics-engine path can be verified inside core without
cross-plugin dependencies on the SQL plugin.
Each test sends a PPL query through POST /_analytics/ppl (exposed by
the test-ppl-frontend plugin) which runs the same UnifiedQueryPlanner
→ CalciteRelNodeVisitor → Substrait → DataFusion pipeline as the SQL
plugin's force-routed analytics path. Covers all 13 fillnull surface
forms:
- with X in fields — single value, named fields
- with X in f1,f2 — single value, multiple fields
- using f1=X, f2=Y — per-field replacement
- using f0=f1 — replacement is a field reference
- with ceil(num1) in num0 — replacement contains a nested call
(exercises recursive AnnotatedProjectExpression strip)
- chained fillnulls — multiple commands
- value=X (all fields) — Calcite-specific syntax
- value=X f1 — Calcite-specific, named fields
- value='N/A' str2 — Calcite-specific, string value
- using int0=8589934592 — BIGINT into INTEGER (numeric coercion)
- mixed-type errors — preflight type-incompatibility validation
Provisions the 'calcs' dataset (parquet-backed) once per class via
DatasetProvisioner (which uses refresh=true, sidestepping the
LuceneCommitter.getSafeCommitInfo TODO that hangs refresh=wait_for in
the SQL plugin's IT path).
Result: 13/13 pass against the QA-module's testcluster — no SQL plugin,
no force_routing setting, no parquet-IT system properties needed.
Signed-off-by: Kai Huang <ahkcs@amazon.com>
|
Persistent review updated to latest commit dc560ab |
|
❌ Gradle check result for dc560ab: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
mch2
added
skip-diff-analyzer
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21472 +/- ##
=========================================
Coverage 73.41% 73.42%
- Complexity 74420 74440 +20
=========================================
Files 5967 5967
Lines 338227 338230 +3
Branches 48754 48755 +1
=========================================
+ Hits 248315 248346 +31
+ Misses 70105 70061 -44
- Partials 19807 19823 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…/regexp_replace() functions Onboards the PPL `replace` command and the `replace()` / `regexp_replace()` eval functions to the analytics-engine route by mapping their two Calcite lowering targets — `SqlStdOperatorTable.REPLACE` and `SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's native `replace` and `regexp_replace` UDFs. Same templated shape as the `fillnull` POC (opensearch-project#21472): ScalarFunction enum constant + STANDARD_PROJECT_OPS membership + opensearch_scalar_functions.yaml extension entry + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge = onboarded to the analytics route. Two scalar functions added: REPLACE (literal substring replace) and REGEXP_REPLACE (regex replace). Both project-side only; the comparison result of a replaced field is filtered via the existing EQUALS capability, so no STANDARD_FILTER_OPS additions are needed. PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()` to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need translation before substrait serialization, because DataFusion uses Rust's `regex` crate which has different parsing rules: * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized escape sequence. The adapter expands each block to per-character escaped literals (semantics-preserving). * `$N` numeric backreferences in the replacement — Rust's replacement parser is identifier-greedy, so `$1_$2` is parsed as a reference to group named `1_` followed by `$2` (Java parses it as group 1 + literal underscore + group 2). The adapter wraps every numeric backreference in braces (`${N}`) for unambiguous Rust parsing. Both transforms are in `RegexpReplaceAdapter` and registered against `ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls without `\Q` in the pattern AND without bare `$N` in the replacement pass through unchanged. * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite integration: 3). * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10. Covers literal command (single + multi-pair = nested REPLACE), wildcard command (prefix + suffix), `replace()` and `regexp_replace()` in eval, full-row content checks, no-match passthrough, multi-field IN clause. * SQL plugin's `CalciteReplaceCommandIT` force-routed through the analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true` — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run. (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1 error-message assertion order-agnostic, mirroring the rename precedent from opensearch-project#5413.) Unlike `fillnull`/`regex` where the bridge was a single one-line capability addition, `replace`'s wildcard form exposes Java↔Rust regex syntax divergence. The adapter is reusable for any future Calcite operator whose PPL lowering goes through `WildcardUtils` (e.g. potential future patterns in `like`-with-escape, custom regex lowerings). Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…/regexp_replace() functions (#21527) Onboards the PPL `replace` command and the `replace()` / `regexp_replace()` eval functions to the analytics-engine route by mapping their two Calcite lowering targets — `SqlStdOperatorTable.REPLACE` and `SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's native `replace` and `regexp_replace` UDFs. Same templated shape as the `fillnull` POC (#21472): ScalarFunction enum constant + STANDARD_PROJECT_OPS membership + opensearch_scalar_functions.yaml extension entry + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge = onboarded to the analytics route. Two scalar functions added: REPLACE (literal substring replace) and REGEXP_REPLACE (regex replace). Both project-side only; the comparison result of a replaced field is filtered via the existing EQUALS capability, so no STANDARD_FILTER_OPS additions are needed. PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()` to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need translation before substrait serialization, because DataFusion uses Rust's `regex` crate which has different parsing rules: * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized escape sequence. The adapter expands each block to per-character escaped literals (semantics-preserving). * `$N` numeric backreferences in the replacement — Rust's replacement parser is identifier-greedy, so `$1_$2` is parsed as a reference to group named `1_` followed by `$2` (Java parses it as group 1 + literal underscore + group 2). The adapter wraps every numeric backreference in braces (`${N}`) for unambiguous Rust parsing. Both transforms are in `RegexpReplaceAdapter` and registered against `ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls without `\Q` in the pattern AND without bare `$N` in the replacement pass through unchanged. * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite integration: 3). * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10. Covers literal command (single + multi-pair = nested REPLACE), wildcard command (prefix + suffix), `replace()` and `regexp_replace()` in eval, full-row content checks, no-match passthrough, multi-field IN clause. * SQL plugin's `CalciteReplaceCommandIT` force-routed through the analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true` — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run. (Companion SQL plugin PR #5415 makes 4 column-order assertions and 1 error-message assertion order-agnostic, mirroring the rename precedent from #5413.) Unlike `fillnull`/`regex` where the bridge was a single one-line capability addition, `replace`'s wildcard form exposes Java↔Rust regex syntax divergence. The adapter is reusable for any future Calcite operator whose PPL lowering goes through `WildcardUtils` (e.g. potential future patterns in `like`-with-escape, custom regex lowerings). Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…ESCE (opensearch-project#21472) * [Analytics Backend / DataFusion] Wire CEIL into project capabilities DataFusion natively supports CEIL; declaring it as a project capability lets the analytics-engine planner route Project nodes containing CEIL through DataFusion. Same pattern as the COALESCE addition: one entry in STANDARD_PROJECT_OPS, no convertor changes needed (Substrait default extensions handle the conversion). Exercised by CalciteFillNullCommandIT.testFillNullWithFunctionOnOtherField, which calls 'fillnull with ceil(num1) in num0' — the COALESCE replacement is a CEIL of another field. Without CEIL declared project-capable, the planner would fail with 'No backend supports scalar function [CEIL] among [datafusion]' even after COALESCE is wired. CEIL is also one of the ~100 Bucket-1 functions in the PPL→DataFusion audit (direct DataFusion mapping, S0). This commit is the templated shape for any other Bucket-1 scalar — add the constant to STANDARD_PROJECT_OPS, no other changes. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Engine] Strip nested AnnotatedProjectExpression wrappers recursively OpenSearchProjectRule.annotateExpr recurses into a project call's operands, wrapping every sub-call in an AnnotatedProjectExpression so the planner can track viable backends per sub-expression. The previous OpenSearchProject.stripAnnotations only unwrapped the top-level wrapper — for a project like PLUS(CEIL(value), value), the strip left an inner Annotated(CEIL(...)) sitting inside the outer call. Substrait isthmus has no converter for ANNOTATED_PROJECT_EXPR, so conversion failed with 'Unable to convert call ANNOTATED_PROJECT_EXPR'. Walk each project expression with a RexShuttle that recursively unwraps every AnnotatedProjectExpression it encounters before handing the plan off to the backend FragmentConvertor. The shuttle's super.visitCall keeps the operand-traversal default; the override intercepts wrappers and re-feeds the unwrapped result through the shuttle so deeply-nested wrappers also get stripped. Adds ProjectRuleTests#testStripAnnotationsRecursivelyUnwrapsNestedExpressions covering the nested-call shape. The bug is independent of which scalar functions any backend declares project-capable; it would fire for any project with a nested scalar call once both the outer and inner operators are project-capable on the same backend. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [Analytics Backend / DataFusion] Wire COALESCE into project capabilities for PPL fillnull PPL fillnull lowers via CalciteRelNodeVisitor.visitFillNull into a LogicalProject(COALESCE(field, replacement)) tree. The analytics-engine planner's OpenSearchProjectRule rejects any project-list scalar call unless at least one viable backend declares it project-capable, with 'No backend supports scalar function [COALESCE] among [datafusion]'. DataFusion natively supports COALESCE, but the backend plugin had not declared any projectCapabilities, so every fillnull query failed the planner check before reaching Substrait conversion. Add a STANDARD_PROJECT_OPS set mirroring the existing STANDARD_FILTER_OPS pattern, seeded with COALESCE, and override projectCapabilities() to fan it out across the same field-types and formats the backend already advertises for filters. No Rust-side or convertor changes are needed — the default Substrait extension catalog loaded by DataFusionPlugin.loadSubstraitExtensions handles COALESCE natively, and DataFusion's runtime executes it directly. Lifts CalciteFillNullCommandIT on the force-routed analytics-engine path from 2/13 passing (only the SQL-plugin preflight type-check tests) to 12/13 passing. The remaining failure is testFillNullWithFunctionOnOtherField, which uses a nested CEIL inside the COALESCE replacement — handled in follow-up commits. Signed-off-by: Kai Huang <ahkcs@amazon.com> * [QA] Add FillNullCommandIT for the analytics-engine REST path Self-contained integration test for PPL fillnull on the analytics-engine route, mirroring CalciteFillNullCommandIT from opensearch-project/sql so the analytics-engine path can be verified inside core without cross-plugin dependencies on the SQL plugin. Each test sends a PPL query through POST /_analytics/ppl (exposed by the test-ppl-frontend plugin) which runs the same UnifiedQueryPlanner → CalciteRelNodeVisitor → Substrait → DataFusion pipeline as the SQL plugin's force-routed analytics path. Covers all 13 fillnull surface forms: - with X in fields — single value, named fields - with X in f1,f2 — single value, multiple fields - using f1=X, f2=Y — per-field replacement - using f0=f1 — replacement is a field reference - with ceil(num1) in num0 — replacement contains a nested call (exercises recursive AnnotatedProjectExpression strip) - chained fillnulls — multiple commands - value=X (all fields) — Calcite-specific syntax - value=X f1 — Calcite-specific, named fields - value='N/A' str2 — Calcite-specific, string value - using int0=8589934592 — BIGINT into INTEGER (numeric coercion) - mixed-type errors — preflight type-incompatibility validation Provisions the 'calcs' dataset (parquet-backed) once per class via DatasetProvisioner (which uses refresh=true, sidestepping the LuceneCommitter.getSafeCommitInfo TODO that hangs refresh=wait_for in the SQL plugin's IT path). Result: 13/13 pass against the QA-module's testcluster — no SQL plugin, no force_routing setting, no parquet-IT system properties needed. Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
…/regexp_replace() functions (opensearch-project#21527) Onboards the PPL `replace` command and the `replace()` / `regexp_replace()` eval functions to the analytics-engine route by mapping their two Calcite lowering targets — `SqlStdOperatorTable.REPLACE` and `SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's native `replace` and `regexp_replace` UDFs. Same templated shape as the `fillnull` POC (opensearch-project#21472): ScalarFunction enum constant + STANDARD_PROJECT_OPS membership + opensearch_scalar_functions.yaml extension entry + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge = onboarded to the analytics route. Two scalar functions added: REPLACE (literal substring replace) and REGEXP_REPLACE (regex replace). Both project-side only; the comparison result of a replaced field is filtered via the existing EQUALS capability, so no STANDARD_FILTER_OPS additions are needed. PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()` to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need translation before substrait serialization, because DataFusion uses Rust's `regex` crate which has different parsing rules: * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized escape sequence. The adapter expands each block to per-character escaped literals (semantics-preserving). * `$N` numeric backreferences in the replacement — Rust's replacement parser is identifier-greedy, so `$1_$2` is parsed as a reference to group named `1_` followed by `$2` (Java parses it as group 1 + literal underscore + group 2). The adapter wraps every numeric backreference in braces (`${N}`) for unambiguous Rust parsing. Both transforms are in `RegexpReplaceAdapter` and registered against `ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls without `\Q` in the pattern AND without bare `$N` in the replacement pass through unchanged. * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite integration: 3). * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10. Covers literal command (single + multi-pair = nested REPLACE), wildcard command (prefix + suffix), `replace()` and `regexp_replace()` in eval, full-row content checks, no-match passthrough, multi-field IN clause. * SQL plugin's `CalciteReplaceCommandIT` force-routed through the analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true` — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run. (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1 error-message assertion order-agnostic, mirroring the rename precedent from opensearch-project#5413.) Unlike `fillnull`/`regex` where the bridge was a single one-line capability addition, `replace`'s wildcard form exposes Java↔Rust regex syntax divergence. The adapter is reusable for any future Calcite operator whose PPL lowering goes through `WildcardUtils` (e.g. potential future patterns in `like`-with-escape, custom regex lowerings). Signed-off-by: Jialiang Liang <jiallian@amazon.com>
…/regexp_replace() functions (opensearch-project#21527) Onboards the PPL `replace` command and the `replace()` / `regexp_replace()` eval functions to the analytics-engine route by mapping their two Calcite lowering targets — `SqlStdOperatorTable.REPLACE` and `SqlLibraryOperators.REGEXP_REPLACE_3` — through Substrait to DataFusion's native `replace` and `regexp_replace` UDFs. Same templated shape as the `fillnull` POC (opensearch-project#21472): ScalarFunction enum constant + STANDARD_PROJECT_OPS membership + opensearch_scalar_functions.yaml extension entry + ADDITIONAL_SCALAR_SIGS Calcite-op→Substrait-name bridge = onboarded to the analytics route. Two scalar functions added: REPLACE (literal substring replace) and REGEXP_REPLACE (regex replace). Both project-side only; the comparison result of a replaced field is filtered via the existing EQUALS capability, so no STANDARD_FILTER_OPS additions are needed. PPL's wildcard `replace` form lowers via `WildcardUtils.convertWildcardPatternToRegex()` to a Java-`Pattern`-compatible regex. Two flavors of Java syntax need translation before substrait serialization, because DataFusion uses Rust's `regex` crate which has different parsing rules: * `\Q…\E` quoted-literal blocks — Rust rejects `\Q` as an unrecognized escape sequence. The adapter expands each block to per-character escaped literals (semantics-preserving). * `$N` numeric backreferences in the replacement — Rust's replacement parser is identifier-greedy, so `$1_$2` is parsed as a reference to group named `1_` followed by `$2` (Java parses it as group 1 + literal underscore + group 2). The adapter wraps every numeric backreference in braces (`${N}`) for unambiguous Rust parsing. Both transforms are in `RegexpReplaceAdapter` and registered against `ScalarFunction.REGEXP_REPLACE` in `scalarFunctionAdapters()`. Calls without `\Q` in the pattern AND without bare `$N` in the replacement pass through unchanged. * `RegexpReplaceAdapterTests` — 19/19 (unquote: 9, brace: 7, dual-rewrite integration: 3). * `ReplaceCommandIT` (new self-contained QA IT, calcs dataset) — 10/10. Covers literal command (single + multi-pair = nested REPLACE), wildcard command (prefix + suffix), `replace()` and `regexp_replace()` in eval, full-row content checks, no-match passthrough, multi-field IN clause. * SQL plugin's `CalciteReplaceCommandIT` force-routed through the analytics-engine route via `-Dtests.analytics.{force_routing,parquet_indices}=true` — 21/21 in both the direct suite and the `CalciteNoPushdownIT` re-run. (Companion SQL plugin PR opensearch-project#5415 makes 4 column-order assertions and 1 error-message assertion order-agnostic, mirroring the rename precedent from opensearch-project#5413.) Unlike `fillnull`/`regex` where the bridge was a single one-line capability addition, `replace`'s wildcard form exposes Java↔Rust regex syntax divergence. The adapter is reusable for any future Calcite operator whose PPL lowering goes through `WildcardUtils` (e.g. potential future patterns in `like`-with-escape, custom regex lowerings). Signed-off-by: Jialiang Liang <jiallian@amazon.com>
3 participants
Description
Enables PPL
fillnullon the analytics-engine query route by wiringCOALESCE(andCEIL, used inside one fillnull-with-function test) into the DataFusion backend's project-side scalar capabilities. Bundled with one analytics-engine planner fix that's a prerequisite for any nested project call to round-trip through Substrait.This is the proof-of-concept template for the remaining ~99 Bucket-1 PPL functions: every direct DataFusion mapping is a one-line addition to
DataFusionAnalyticsBackendPlugin.STANDARD_PROJECT_OPS(no Rust-side, convertor, or planner changes).How it failed before
CalciteRelNodeVisitor.visitFillNullalready lowersfillnullinto a cleanLogicalProject(COALESCE(field, replacement))— verified byCalcitePPLFillnullTest. But runningCalciteFillNullCommandITagainst a force-routed analytics-engine cluster yielded:The DataFusion backend declared
filterCapabilities,scanCapabilities, andaggregateCapabilities, butprojectCapabilities()defaulted to empty — soOpenSearchProjectRulerejected everyCOALESCEit saw. Once COALESCE was declared, the fillnull-with-function test (fillnull with ceil(num1) in num0) hit a second issue:OpenSearchProjectRule.annotateExprrecurses into operands and wraps every sub-call.OpenSearchProject.stripAnnotationsonly unwrapped the top-level wrapper, leaving an innerAnnotatedProjectExpression(CEIL)that Substrait isthmus has no converter for.Changes
OpenSearchProject.stripAnnotations— replace single-level unwrap with aRexShuttlethat recursively stripsAnnotatedProjectExpressionat every depth before handing the plan to the backendFragmentConvertor. AddsProjectRuleTests#testStripAnnotationsRecursivelyUnwrapsNestedExpressionscovering the nested-call shape. This is a general fix — any project containing a nested scalar call would hit it once both operators are project-capable; fillnull-with-function just happened to expose it first.DataFusionAnalyticsBackendPlugin.projectCapabilities()— declareCOALESCE(PPL fillnull's lowering target) on all supported field types and formats. Mirrors the existingSTANDARD_FILTER_OPSpattern.DataFusionAnalyticsBackendPlugin.STANDARD_PROJECT_OPS— addCEIL. Exercised byCalciteFillNullCommandIT.testFillNullWithFunctionOnOtherField(fillnull with ceil(num1) in num0). Same one-line shape future Bucket-1 functions will use.No Rust-side, convertor, or Substrait-extension changes — DataFusion's native runtime executes
COALESCE/CEILdirectly via the default Substrait extension catalog already loaded byDataFusionPlugin.loadSubstraitExtensions.Test results
CalciteFillNullCommandITagainsttests.analytics.force_routing=truewith parquet-backed indices:No backend supports scalar function [COALESCE]error aboveCalcitePPLFillnullIT: 3/3 pass (the secondary fillnull IT coveringfillnull with X in ...,fillnull using f=X, andfillnull with Xall-fields).ProjectRuleTests: all existing tests pass + new nested-strip test passes.POC framing for future Bucket-1 functions
The PPL function audit identifies ~100 Bucket-1 functions where DataFusion has a direct native mapping. After this PR's groundwork lands, each subsequent function follows the templated commit shape demonstrated by the
CEILcommit:No other code changes needed — the planner picks up the new capability via
BackendCapabilityProvider, isthmus emits the right Substrait, and DataFusion executes natively.Check List