[analytics-engine] Fix CHECKED_LONG_SUM conversion by ahkcs · Pull Request #22611 · opensearch-project/OpenSearch

ahkcs · 2026-07-30T17:52:31Z

Description

SQL PR opensearch-project/sql#5612 introduced the reflective CHECKED_LONG_SUM aggregate. Isthmus resolves Substrait functions by exact Calcite operator identity, so the analytics path could not bind that marker directly.

This change canonicalizes aggregate and window forms of CHECKED_LONG_SUM to SqlStdOperatorTable.SUM in the RBO planner, before OpenSearchAggregateRule runs. Aggregate splitting, TopK, Lucene, Isthmus, and DataFusion therefore all see native SUM. When Calcite's logical return type differs, a result projection restores the original BIGINT schema after aggregation.

The earlier local operator, Substrait YAML bindings, and delegating Rust UDAF are removed. DataFusion resolves the standard Substrait function to its native sum_udaf.

Testing

AggregatePlanShapeTests and WindowPlanShapeTests
DataFusionFragmentConvertorTests
StatsCommandIT.testStatsIntegralSum
analytics-engine and analytics-backend-datafusion precommit
cargo fmt --all --check

github-actions · 2026-07-30T17:53:31Z

PR Reviewer Guide 🔍

(Review updated until commit `7ebc621`)

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 Possible Infinite Loop `onMatch` always calls `transformTo` when the rule matches, even after the rewrite has already happened. Since `isCheckedLongSum` returns true only for non-`SUM` operators with `SqlKind.SUM` named `CHECKED_LONG_SUM`, subsequent matches should not fire; however, if the result-type projection causes the HEP planner to revisit the aggregate before the rewritten calls are recognized, or if the aggregate is not structurally changed (same row type, so no projection wrapper), the rule could re-fire and loop. Consider guarding `onMatch`/`matches` to ensure at least one call is actually being rewritten and verifying HEP won't repeatedly match the transformed aggregate. public void onMatch(RelOptRuleCall ruleCall) { LogicalAggregate aggregate = ruleCall.rel(0); List<AggregateCall> rewritten = new ArrayList<>(aggregate.getAggCallList().size()); for (AggregateCall call : aggregate.getAggCallList()) { rewritten.add(isCheckedLongSum(call.getAggregation()) ? rewrite(call, aggregate) : call); } LogicalAggregate replacement = aggregate.copy( aggregate.getTraitSet(), aggregate.getInput(), aggregate.getGroupSet(), aggregate.getGroupSets(), rewritten ); ruleCall.transformTo(projectToOriginalRowType(ruleCall, aggregate, replacement)); } Type Cast Nullability When the replacement aggregate's output type differs from the original (e.g., Calcite's SUM returns a wider/nullable type while CHECKED_LONG_SUM was declared BIGINT non-null-forced), the cast to `field.getType()` may fail at validation or produce runtime NPE when the SUM yields null (empty group) but is cast to a non-nullable BIGINT. Consider preserving nullability compatible with the source SUM or verifying `field.getType()` is nullable before casting. for (RelDataTypeField field : original.getRowType().getFieldList()) { RexNode ref = rexBuilder.makeInputRef(replacement, field.getIndex()); RelDataType replacementType = replacement.getRowType().getFieldList().get(field.getIndex()).getType(); if (!replacementType.equals(field.getType())) { ref = rexBuilder.makeCast(field.getType(), ref); } projects.add(ref); names.add(field.getName()); }

github-actions · 2026-07-30T17:53:53Z

PR Code Suggestions ✨

Latest suggestions up to 7ebc621

Explore these optional code suggestions:

Category Suggestion Impact

General

Recurse fully when detecting nested matches

The RexShuttle.visitOver override does not recurse into the RexOver's operands or
window expressions, so nested CHECKED_LONG_SUM occurrences (e.g., inside another
window's operands) may be missed. Call super.visitOver(over) to ensure full
traversal before returning.

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumWindowRule.java [49-61]

 private static boolean containsCheckedLongSum(RexNode expression) {
     boolean[] found = new boolean[1];
     expression.accept(new RexShuttle() {
         @Override
         public RexNode visitOver(RexOver over) {
             if (OpenSearchCheckedLongSumRule.isCheckedLongSum(over.getAggOperator())) {
                 found[0] = true;
             }
-            return over;
+            return super.visitOver(over);
         }
     });
     return found[0];
 }

Suggestion importance[1-10]: 5

__

Why: Valid observation: without calling super.visitOver, nested RexOver operands would not be traversed. However, nested window functions inside another window's operands are rare, so the practical impact is limited.

Low

Preserve original aggregate return type

Passing null as the return type forces Calcite to re-infer it from SUM, which may
differ from CHECKED_LONG_SUM's original type (e.g., nullability or precision),
causing downstream type mismatches. Pass call.getType() explicitly to preserve the
original row-type contract and simplify the projectToOriginalRowType casting logic.

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumRule.java [70-84]

 return AggregateCall.create(
     SqlStdOperatorTable.SUM,
     call.isDistinct(),
     call.isApproximate(),
     call.ignoreNulls(),
     call.rexList,
     call.getArgList(),
     call.filterArg,
     call.distinctKeys,
     call.collation,
     aggregate.getGroupSet().cardinality(),
     aggregate.getInput(),
-    null,
+    call.getType(),
     call.getName()
 );

Suggestion importance[1-10]: 4

__

Why: Passing call.getType() could preserve the original type contract, but the current code intentionally handles type differences via projectToOriginalRowType. The tests show the CAST behavior is expected, so this change could conflict with the design intent.

even
Human: Continue

</details></details></td><td align=center>Low

</td></tr></tr></tbody></table>

___

#### Previous suggestions


<details><summary>Suggestions up to commit 47401b0</summary>
<br><table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>General</td>
<td>



<details><summary>Recurse into nested window expressions</summary>

___


**<code>RexShuttle.visitOver</code> does not recurse into the <code>RexOver</code>'s operands/window by default <br>beyond what <code>super</code> does, but the current override returns <code>over</code> without calling <br><code>super.visitOver(over)</code>, so nested <code>RexOver</code> inside operands would not be inspected. <br>Delegate to <code>super.visitOver</code> after setting the flag to ensure full traversal.**

[sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumWindowRule.java [49-61]](https://github.com/opensearch-project/OpenSearch/pull/22611/files#diff-6811c08f2f0478f8d26d203fb0ae3c05a470862d5a72eec2c679fe4fc0a592deR49-R61)

```diff
 private static boolean containsCheckedLongSum(RexNode expression) {
     boolean[] found = new boolean[1];
     expression.accept(new RexShuttle() {
         @Override
         public RexNode visitOver(RexOver over) {
             if (OpenSearchCheckedLongSumRule.isCheckedLongSum(over.getAggOperator())) {
                 found[0] = true;
             }
-            return over;
+            return super.visitOver(over);
         }
     });
     return found[0];
 }

Suggestion importance[1-10]: 5

__

Why: Valid observation - the current override returns over without calling super.visitOver, which prevents recursion into nested RexOver in operands. However, nested window functions are uncommon, making this a minor robustness improvement.

Low

Suggestions up to commit af8fb51

Category	Suggestion	Impact
General	Recurse into nested window expressions during detection `RexShuttle.visitOver` does not recurse into the `RexOver` operands, so a `CHECKED_LONG_SUM` window nested inside a wrapping expression (e.g. `CAST(CHECKED_LONG_SUM(...) OVER ...)`) will be detected only at the top level, but detection here relies on shuttle traversal. Call `super.visitOver(over)` before checking to ensure nested `RexOver` nodes are also visited, keeping this consistent with the rewriter. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumWindowRule.java [49-61] private static boolean containsCheckedLongSum(RexNode expression) { boolean[] found = new boolean[1]; expression.accept(new RexShuttle() { @Override public RexNode visitOver(RexOver over) { + RexNode visited = super.visitOver(over); if (OpenSearchCheckedLongSumRule.isCheckedLongSum(over.getAggOperator())) { found[0] = true; } - return over; + return visited; } }); return found[0]; } Suggestion importance[1-10]: 6 __ Why: Valid observation: `RexShuttle.visitOver` in the detection path doesn't recurse into operands, so nested `RexOver` inside wrapping expressions could be missed. Calling `super.visitOver` aligns detection with the rewriter behavior, though impact depends on whether such nested cases occur in practice.	Low
General	Make marker detection less fragile Relying on `getKind() == SqlKind.SUM` plus a name match is fragile; if the marker's kind ever changes, matching silently breaks. Since the name is the authoritative marker, drop the kind check (or guard it defensively) so the rebind remains robust across marker evolutions. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumRule.java [63-67] static boolean isCheckedLongSum(SqlAggFunction operator) { return operator != SqlStdOperatorTable.SUM - && operator.getKind() == SqlKind.SUM && "CHECKED_LONG_SUM".equalsIgnoreCase(operator.getName()); } Suggestion importance[1-10]: 3 __ Why: The current kind check adds a defensive guard that ensures the marker is a SUM-kind aggregate; removing it may actually reduce safety by matching unrelated operators with the same name. The suggestion's rationale is debatable and impact is marginal.	Low

Suggestions up to commit e7f94f6

Category	Suggestion	Impact
General	Preserve original aggregate return type `rewrite` calls `AggregateCall.create` which re-derives the return type from `SUM` over the input, potentially producing a different (non-nullable or wider) type than the original `CHECKED_LONG_SUM` return type. This is why `projectToOriginalRowType` needs to insert casts. Consider using the `AggregateCall.create` overload that accepts an explicit `type` (i.e., pass `call.getType()`) to keep the row type stable and avoid unnecessary CAST projections downstream. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumRule.java [47-52] +@Override +public void onMatch(RelOptRuleCall ruleCall) { + LogicalAggregate aggregate = ruleCall.rel(0); + List<AggregateCall> rewritten = new ArrayList<>(aggregate.getAggCallList().size()); + for (AggregateCall call : aggregate.getAggCallList()) { + rewritten.add(isCheckedLongSum(call.getAggregation()) ? rewrite(call, aggregate) : call); + } - Suggestion importance[1-10]: 5 __ Why: Valid observation that using the `AggregateCall.create` overload with an explicit type (`call.getType()`) could avoid the need for CAST projections and simplify `projectToOriginalRowType`. However, the `improved_code` is identical to `existing_code`, so it doesn't demonstrate the change.	Low
General	Preserve original window semantics on rewrite The `allowPartial` and `nullWhenCountZero` flags are hard-coded to `true` and `false`, discarding the original window's semantics. Preserve the source `RexOver`'s properties (e.g., pass `visited`-derived values where available) so the rewritten window behaves identically to the original `CHECKED_LONG_SUM OVER (...)`. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumWindowRule.java [77-91] return rexBuilder.makeOver( visited.getType(), SqlStdOperatorTable.SUM, visited.getOperands(), window.partitionKeys, window.orderKeys, window.getLowerBound(), window.getUpperBound(), window.getExclude(), window.isRows(), - true, + window.isRows() \|\| !window.orderKeys.isEmpty(), false, visited.isDistinct(), visited.ignoreNulls() ); Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern that hard-coded `allowPartial=true` and `nullWhenCountZero=false` flags may not match the original `RexOver` semantics, but the proposed replacement `window.isRows() \|\| !window.orderKeys.isEmpty()` is a heuristic guess rather than a definitive fix, and `RexOver` doesn't directly expose these flags.	Low

Suggestions up to commit 0416b8d

Category	Suggestion	Impact
Possible issue	Preserve original aggregate return type Passing `null` for the return type lets Calcite infer SUM's type from the input, which may not match `call.getType()` (e.g., CHECKED_LONG_SUM forces BIGINT nullable while SUM's inferred type over an INTEGER column is INTEGER). This is why the row type diverges and a cast projection is needed. Consider passing `call.getType()` explicitly to preserve the original result type and avoid the extra projection/cast, or ensure the cast projection preserves nullability semantics exactly. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumRule.java [70-84] return AggregateCall.create( SqlStdOperatorTable.SUM, call.isDistinct(), call.isApproximate(), call.ignoreNulls(), call.rexList, call.getArgList(), call.filterArg, call.distinctKeys, call.collation, aggregate.getGroupSet().cardinality(), aggregate.getInput(), - null, + call.getType(), call.getName() ); Suggestion importance[1-10]: 5 __ Why: Passing `call.getType()` would preserve the original result type and avoid the extra cast projection. However, the current implementation intentionally re-infers the type and handles casting via `projectToOriginalRowType`, and the tests verify this behavior, so this change may break tests.	Low
General	Guard against HEP rule re-firing loop The rule matches any `LogicalProject` containing CHECKED_LONG_SUM, transforms it, and the transformed project — still a `LogicalProject` — will match again in a bottom-up HEP traversal unless `matches()` returns false after transform. Since `containsCheckedLongSum` should return false post-rewrite this may be safe, but confirm HEP doesn't loop; alternatively, short-circuit by checking equality before calling `transformTo`. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumWindowRule.java [42-47] @Override public void onMatch(RelOptRuleCall ruleCall) { LogicalProject project = ruleCall.rel(0); RexBuilder rexBuilder = project.getCluster().getRexBuilder(); List<RexNode> rewritten = project.getProjects().stream().map(expr -> expr.accept(new Rewriter(rexBuilder))).toList(); + if (rewritten.equals(project.getProjects())) { + return; + } ruleCall.transformTo(project.copy(project.getTraitSet(), project.getInput(), rewritten, project.getRowType())); } Suggestion importance[1-10]: 4 __ Why: Adding a short-circuit guard is a reasonable defensive measure to prevent potential HEP re-firing, though `matches()` already checks for CHECKED_LONG_SUM which is removed post-rewrite, making this largely redundant.	Low
General	Use equals instead of identity check The identity check `operator != SqlStdOperatorTable.SUM` is fragile because the rule may re-fire on the rewritten aggregate — the newly created SUM will not be the singleton if compared by identity in some class loaders, but conversely any user-defined SUM-kind operator named "CHECKED_LONG_SUM" other than the singleton triggers the rewrite. Consider a positive identity check (e.g., `SqlStdOperatorTable.SUM.equals(operator)` short-circuit) and also guard `matches()` to avoid re-firing infinitely on the transformed node in HEP. sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchCheckedLongSumRule.java [64-66] static boolean isCheckedLongSum(SqlAggFunction operator) { - return operator != SqlStdOperatorTable.SUM + return !SqlStdOperatorTable.SUM.equals(operator) && operator.getKind() == SqlKind.SUM && "CHECKED_LONG_SUM".equalsIgnoreCase(operator.getName()); } Suggestion importance[1-10]: 3 __ Why: Using `.equals()` instead of `!=` is a minor defensive improvement; `SqlStdOperatorTable.SUM` is a singleton so identity comparison is typically reliable. The concern about infinite re-firing is speculative.	Low

Suggestions up to commit d0cc903

Category	Suggestion	Impact
General	Use rebound call for aggregation checks The post-bind literal-rewrite logic uses the original `call` to check for `LocalAggOp`, but after `bindCheckedLongSum` the aggregation may have changed to `LOCAL_CHECKED_LONG_SUM_OP`. Use `substraitCall.getAggregation()` instead to ensure consistent behavior for rebound calls. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/DataFusionFragmentConvertor.java [772-778] - RexNode toConvert = call.getAggregation() instanceof LocalAggOp localOp + RexNode toConvert = substraitCall.getAggregation() instanceof LocalAggOp localOp ? localOp.normaliseLiteralArg(i, rexLit, rexBuilder, typeFactory) : rexLit; rewritten.set(i, rexConverter.apply(toConvert)); } if (rewritten == null) return bound; return Optional.of(ImmutableAggregateFunctionInvocation.builder().from(fn).arguments(rewritten).build()); } Suggestion importance[1-10]: 4 __ Why: The suggestion correctly notes that after `bindCheckedLongSum`, the `substraitCall` should be used consistently. However, since `LOCAL_CHECKED_LONG_SUM_OP` is not a `LocalAggOp`, the branch behavior is the same (falls through to `rexLit`). It's a minor consistency improvement with limited practical impact.	Low

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

github-actions · 2026-07-30T17:58:22Z

Persistent review updated to latest commit 2d8457e

github-actions · 2026-07-30T18:45:28Z

Persistent review updated to latest commit b798c51

github-actions · 2026-07-30T19:09:16Z

❌ Gradle check result for b798c51: 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?

github-actions · 2026-07-30T20:15:26Z

Persistent review updated to latest commit 6d8cc59

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

github-actions · 2026-07-30T20:41:28Z

Persistent review updated to latest commit a01e3bd

github-actions · 2026-07-30T21:08:17Z

❌ Gradle check result for a01e3bd: 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?

…m-substrait

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

github-actions · 2026-07-31T16:43:58Z

Persistent review updated to latest commit d0cc903

github-actions · 2026-07-31T17:40:22Z

✅ Gradle check result for d0cc903: SUCCESS

codecov · 2026-07-31T17:40:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.49%. Comparing base (3eed58b) to head (7ebc621).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22611      +/-   ##
============================================
- Coverage     71.55%   71.49%   -0.07%     
+ Complexity    77036    76984      -52     
============================================
  Files          6156     6156              
  Lines        358413   358413              
  Branches      52243    52243              
============================================
- Hits         256476   256247     -229     
- Misses        81581    81776     +195     
- Partials      20356    20390      +34

☔ 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.

sandeshkr419

Thanks @ahkcs for working on this. I have an alternative proposal which I suspect should simplify the changes.

The operator rewrite should belong in the RBO planner layer (a new OpenSearchCheckedLongSumRule modeled on OpenSearchDistinctCountRule), not inside DataFusionFragmentConvertor. Moving it earlier, i.e., before OpenSearchAggregateRule runs means the entire stack sees SqlStdOperatorTable.SUM by operator identity: AggregateSplitRule, TopK, the Lucene backend, and Isthmus all resolve it correctly without any special-casing.

The custom LOCAL_CHECKED_LONG_SUM_OP, YAML bindings, and checked_long_sum.rs Rust UDAF are only needed in this PR because the current rewrite happens too late (inside the fragment convertor, after Isthmus dispatch). With an early planner rewrite, DataFusion receives sum in the Substrait proto and resolves to its own native sum_udaf — the delegation wrapper in checked_long_sum.rs is pure boilerplate with zero custom logic and could be deleted entirely.

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

github-actions · 2026-08-04T04:10:12Z

Persistent review updated to latest commit 0416b8d

github-actions · 2026-08-04T04:44:46Z

Persistent review updated to latest commit f98b766

github-actions · 2026-08-04T05:42:10Z

✅ Gradle check result for f98b766: SUCCESS

ahkcs · 2026-08-04T06:19:41Z

The sandbox-check failure is caused by an upstream SQL snapshot regression introduced by opensearch-project/sql#5633: aggregate signature validation rejects equivalent analytics-api date/time/IP/binary types. The fix is opensearch-project/sql#5674.

I reproduced the failure with seed DECD078A88A50739 and verified the patched plugin against the failing datetime test plus the affected list/multi-shard aggregation suites. No additional OpenSearch code change is needed; I will rerun this check after the fixed SQL snapshot is published.

ahkcs · 2026-08-04T17:30:31Z

`sandbox-check` failures are pre-existing and unrelated to this PR

All 11 failures come from a SQL-plugin bug, not from the CHECKED_LONG_SUM change here.

Why they aren't from this PR:

This PR's diff is 8 files, all CHECKED_LONG_SUM-specific — nothing touches LIST/VALUES.
The identical 11 failures appear on unrelated branches, e.g. runs 30894498598 (fuzzy-dsl-support), 30887626220 (range-dsl-support), and 30827742242.
sandbox/qa/analytics-engine-rest/build.gradle consumes opensearch-sql-plugin:3.8.0.0-SNAPSHOT@zip, a floating snapshot, so a SQL-repo regression turns every branch red at once. sandbox-check was clean on main as of Jul 21 and broke by Aug 2.

Root cause. PPLTypeChecker.typesMatch rejected a type pair whenever only one side extended AbstractExprRelDataType. PPLOperandTypes.SCALAR_TYPES declares DATE/TIME/TIMESTAMP/IP/BINARY as UDTs, but the analytics engine builds row types from plain Calcite types (date → TIMESTAMP(3), ip/binary → VARBINARY), so those operands always failed. The error message contradicted itself because the "expected" and "actual" strings are rendered by two different code paths that agree while typesMatch does not:

Aggregation function LIST expects field type
{...|[DATE]|[TIME]|[TIMESTAMP]|[IP]|[BINARY]}, but got [TIMESTAMP]

Introduced by opensearch-project/sql#5633.

Fix: opensearch-project/sql#5675.

Verified by building that branch's plugin zip and pointing this QA suite at it, on this PR's head commit (f98b7665d2d): ListAggregateMultiTypeIT goes 4 failed → 0 failed, and all five affected classes pass (96 tests, 0 failures). Details in this comment.

sandbox-check here will stay red until #5675 merges and a new snapshot publishes. No change is needed on this PR. Worth noting the job is continue-on-error: true, so it does not block merge.

github-actions · 2026-08-04T21:18:16Z

Persistent review updated to latest commit e7f94f6

github-actions · 2026-08-04T22:14:24Z

✅ Gradle check result for e7f94f6: SUCCESS

alchemist51 · 2026-08-05T06:46:52Z

* What went wrong:
Could not determine the dependencies of task ':sandbox:plugins:test-ppl-frontend:forbiddenApisMain'.
> Could not resolve all dependencies for configuration ':sandbox:plugins:test-ppl-frontend:runtimeClasspath'.
   > Could not find org.opensearch.query:ppl-rest-spi:3.8.0.0-SNAPSHOT.
     Searched in the following locations:
       - file:/home/runner/.m2/repository/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/maven-metadata.xml
       - file:/home/runner/.m2/repository/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/ppl-rest-spi-3.8.0.0-SNAPSHOT.pom
       - https://ci.opensearch.org/ci/dbc/snapshots/maven/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/maven-metadata.xml
       - https://ci.opensearch.org/ci/dbc/snapshots/maven/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/ppl-rest-spi-3.8.0.0-SNAPSHOT.pom
       - https://ci.opensearch.org/maven2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/maven-metadata.xml
       - https://ci.opensearch.org/maven2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/ppl-rest-spi-3.8.0.0-SNAPSHOT.pom
       - https://ci.opensearch.org/m2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/maven-metadata.xml
       - https://ci.opensearch.org/m2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/ppl-rest-spi-3.8.0.0-SNAPSHOT.pom
       - https://repo.maven.apache.org/maven2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/maven-metadata.xml
       - https://repo.maven.apache.org/maven2/org/opensearch/query/ppl-rest-spi/3.8.0.0-SNAPSHOT/ppl-rest-spi-3.8.0.0-SNAPSHOT.pom
     Required by:
         project ':sandbox:plugins:test-ppl-frontend' > org.opensearch.query:unified-query-ppl:3.8.0.0-SNAPSHOT:20260804.211640-79 > org.opensearch.query:unified-query-protocol:3.8.0.0-SNAPSHOT:20260804.211640-79 > org.opensearch.query:unified-query-opensearch:3.8.0.0-SNAPSHOT:20260804.211640-79

github-actions · 2026-08-05T15:52:15Z

Persistent review updated to latest commit af8fb51

github-actions · 2026-08-05T16:55:42Z

✅ Gradle check result for af8fb51: SUCCESS

github-actions · 2026-08-06T05:52:34Z

Persistent review updated to latest commit 47401b0

github-actions · 2026-08-06T06:49:05Z

✅ Gradle check result for 47401b0: SUCCESS

mch2 · 2026-08-06T21:27:46Z

This is more reason for us to remove this hacky circular dependency on sql plugin. Ideally these rules would be registered with us or applied entirely in sql side before we receive the relNode, that is validated against what AE can support.

I suggest we take this to unblock our sandbox check and other changes, and then aggressively refactor this dependency.

sandeshkr419

Thanks @ahkcs for working on this. This looks super neat now with the logic moved to planing side.

github-actions · 2026-08-06T23:39:34Z

Persistent review updated to latest commit 7ebc621

github-actions · 2026-08-07T00:38:31Z

✅ Gradle check result for 7ebc621: SUCCESS

ahkcs requested a review from a team as a code owner July 30, 2026 17:52

Fix checked long sum analytics conversion

2d8457e

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

ahkcs force-pushed the fix/checked-long-sum-substrait branch from dba238c to 2d8457e Compare July 30, 2026 17:56

alchemist51 approved these changes Jul 30, 2026

View reviewed changes

Merge branch 'main' into fix/checked-long-sum-substrait

b798c51

sandeshkr419 changed the title ~~Fix CHECKED_LONG_SUM conversion in analytics engine~~ [analytics engine] Fix CHECKED_LONG_SUM conversion Jul 30, 2026

sandeshkr419 changed the title ~~[analytics engine] Fix CHECKED_LONG_SUM conversion~~ [analytics-engine] Fix CHECKED_LONG_SUM conversion Jul 30, 2026

Fix checked long sum schema collisions

a01e3bd

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

ahkcs force-pushed the fix/checked-long-sum-substrait branch from 6d8cc59 to a01e3bd Compare July 30, 2026 20:39

ahkcs added 2 commits July 31, 2026 09:39

Merge remote-tracking branch 'upstream/main' into fix/checked-long-su…

8d9630e

…m-substrait

Limit checked sum operator rebinding

d0cc903

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

sandeshkr419 requested changes Jul 31, 2026

View reviewed changes

sandeshkr419 mentioned this pull request Aug 3, 2026

[analytics-backend-datafusion] Normalize CHAR/VARCHAR literals to Str in VirtualTable conversion #22554

Open

3 tasks

Rewrite checked sum in analytics planner

0416b8d

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

Merge branch 'main' into fix/checked-long-sum-substrait

f98b766

ahkcs mentioned this pull request Aug 4, 2026

Match analytics types in aggregate signatures opensearch-project/sql#5674

Open

Merge branch 'main' into fix/checked-long-sum-substrait

e7f94f6

Merge branch 'main' into fix/checked-long-sum-substrait

af8fb51

Merge branch 'main' into fix/checked-long-sum-substrait

47401b0

Merge branch 'main' into fix/checked-long-sum-substrait

7ebc621

sandeshkr419 self-requested a review August 6, 2026 21:26

sandeshkr419 approved these changes Aug 6, 2026

View reviewed changes

mch2 marked this pull request as draft August 6, 2026 23:36

mch2 marked this pull request as ready for review August 6, 2026 23:36

mch2 closed this Aug 6, 2026

mch2 reopened this Aug 6, 2026

mch2 merged commit b068dd3 into opensearch-project:main Aug 7, 2026
19 of 20 checks passed

Uh oh!

Conversation

ahkcs commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Uh oh!

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

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 7ebc621)

Uh oh!

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

Uh oh!

PR Code Suggestions ✨

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 31, 2026

Uh oh!

codecov Bot commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sandeshkr419 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

ahkcs commented Aug 4, 2026

Uh oh!

ahkcs commented Aug 4, 2026

sandbox-check failures are pre-existing and unrelated to this PR

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

alchemist51 commented Aug 5, 2026

Uh oh!

github-actions Bot commented Aug 5, 2026

Uh oh!

github-actions Bot commented Aug 5, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

mch2 commented Aug 6, 2026

Uh oh!

sandeshkr419 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

ahkcs commented Jul 30, 2026 •

edited

Loading

github-actions Bot commented Jul 30, 2026 •

edited

Loading

(Review updated until commit `7ebc621`)

github-actions Bot commented Jul 30, 2026 •

edited

Loading

codecov Bot commented Jul 31, 2026 •

edited

Loading

sandeshkr419 left a comment •

edited

Loading

`sandbox-check` failures are pre-existing and unrelated to this PR