Skip to content

Commit 8f2d058

Browse files
authored
[Analytics Engine] Wire window-function support for PPL top / rare (#21593)
* [Analytics Engine] Wire window-function support for PPL top / rare PPL `top` and `rare` lower (via `CalciteRelNodeVisitor#visitRareTopN`) to a `LogicalProject` containing `ROW_NUMBER() OVER (PARTITION BY ... ORDER BY ...)`. On the analytics-engine route, `OpenSearchProjectRule.annotateExpr` treated the `RexOver` as an ordinary `RexCall` and fell into the scalar-function viability path. That path keys on `ScalarFunction.fromSqlOperatorWithFallback`, which returns null for window-only aggregates like `ROW_NUMBER`, so every `top` / `rare` query died with "No backend supports scalar function [ROW_NUMBER] among [datafusion]" before substrait emission. Add `EngineCapability.WINDOW` and detect `RexOver` ahead of the standard RexCall branch in `OpenSearchProjectRule#annotateExpr`. When the child's viable backends include a WINDOW-capable backend, wrap the call in an `AnnotatedProjectExpression` exactly like other annotated calls — the existing strip path's `OperatorAnnotation::unwrap` returns the original `RexOver` unchanged, so isthmus's `RexExpressionConverter#visitOver` emits an inline substrait `WindowFunctionInvocation` (the substrait standard catalog already binds `ROW_NUMBER`, so DataFusion's substrait consumer decodes it natively — no separate substrait Window rel needed). WINDOW is intentionally coarse (one boolean per backend rather than a per-window-function `WindowCapability`). The substrait standard catalog already constrains which window aggregates the backend's substrait consumer can decode; a runtime decode failure is a clearer error than a duplicated registry split between SPI and backend. The existing `AggregateFunction` / `AggregateCapability` model remains right for ordinary aggregates; once a second backend with different window support lands, or a window-only function carries a non-standard call shape, WINDOW can be promoted to a per-function class without disturbing the planner-side annotation flow. Result on `CalciteTopCommandIT` / `CalciteRareCommandIT` against the force-routed analytics-engine path: - `CalciteTopCommandIT`: 0/6 → 5/6 (only legacy-preferred test out-of-scope) - `CalciteRareCommandIT`: 0/5 → 3/5 (legacy-preferred + sort-tie out-of-scope) - combined: 0/11 → 8/11 Tests added in `ProjectRuleTests`: - `testRowNumberOverPartitionByOrderByMarksAsWindow` — happy path - `testRowNumberWithoutWindowCapabilityErrors` — capability-gap message names the window function - `testStripAnnotationsPreservesRexOver` — strip path returns plain `LogicalProject(RexOver)` so isthmus can decode it Signed-off-by: Kai Huang <ahkcs@amazon.com> * [QA] Add self-contained Top + Rare CommandITs Self-contained integration tests under sandbox/qa/analytics-engine-rest mirroring CalciteTopCommandIT / CalciteRareCommandIT (and their PPL bases TopCommandIT / RareCommandIT) from the opensearch-project/sql repository. Each test sends a PPL query through POST /_analytics/ppl (exposed by test-ppl-frontend), exercising the same UnifiedQueryPlanner → CalciteRelNodeVisitor#visitRareTopN → ROW_NUMBER() OVER (...) → Substrait WindowFunctionInvocation → DataFusion pipeline that this PR enables via EngineCapability.WINDOW. TopCommandIT (5 tests on the calcs dataset): - testTopWithoutGroup: top str0 → 3 rows with distinct counts (deterministic) - testTopNWithoutGroup: top 1 str0 → 1 row - testTopNWithGroup: top 1 str3 by str0 → 3 rows (one per str0 group) - testTopCommandUseNull: top int0 → 8 buckets (7 non-null + 1 null) - testTopCommandUseNullFalse: top usenull=false int0 → 7 non-null buckets RareCommandIT (4 tests): - testRareWithoutGroup: rare str0 → 3 rows ASC by count (deterministic) - testRareWithGroup: rare str3 by str0 → 5 distinct (str0, str3) pairs - testRareCommandUseNull: rare int0 → 8 buckets - testRareCommandUseNullFalse: rare usenull=false int0 → 7 non-null buckets The legacy-preferred=false variants from CalciteTopCommandIT / CalciteRareCommandIT are not mirrored — they require a cluster-level PPL syntax setting toggle that the QA harness doesn't expose. The default usenull behaviour (PPL_SYNTAX_LEGACY_PREFERRED=true) is the production-default code path and is covered by the four usenull variants above. All 9 pass against the QA test cluster (`./gradlew :sandbox:qa:analytics-engine-rest:integTest --tests "*TopCommandIT" --tests "*RareCommandIT"`). Signed-off-by: Kai Huang <ahkcs@amazon.com> --------- Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent bc96138 commit 8f2d058

4 files changed

Lines changed: 439 additions & 1 deletion

File tree

sandbox/plugins/analytics-engine/src/test/java/org/opensearch/analytics/planner/MockDataFusionBackend.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,16 @@ protected Set<JoinCapability> joinCapabilities() {
140140
protected Set<WindowCapability> windowCapabilities() {
141141
return Set.of(
142142
new WindowCapability(
143-
Set.of(WindowFunction.SUM, WindowFunction.AVG, WindowFunction.COUNT, WindowFunction.MIN, WindowFunction.MAX),
143+
Set.of(
144+
WindowFunction.SUM,
145+
WindowFunction.AVG,
146+
WindowFunction.COUNT,
147+
WindowFunction.MIN,
148+
WindowFunction.MAX,
149+
// ROW_NUMBER backs PPL `top` / `rare` / `dedup` lowering
150+
// (ROW_NUMBER() OVER (PARTITION BY ... ORDER BY ...)).
151+
WindowFunction.ROW_NUMBER
152+
),
144153
Set.of(PARQUET_DATA_FORMAT)
145154
)
146155
);

sandbox/plugins/analytics-engine/src/test/java/org/opensearch/analytics/planner/ProjectRuleTests.java

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,18 @@
88

99
package org.opensearch.analytics.planner;
1010

11+
import com.google.common.collect.ImmutableList;
1112
import org.apache.calcite.plan.RelOptTable;
1213
import org.apache.calcite.plan.RelOptUtil;
1314
import org.apache.calcite.rel.RelNode;
1415
import org.apache.calcite.rel.logical.LogicalProject;
1516
import org.apache.calcite.rex.RexCall;
17+
import org.apache.calcite.rex.RexFieldCollation;
18+
import org.apache.calcite.rex.RexInputRef;
1619
import org.apache.calcite.rex.RexNode;
20+
import org.apache.calcite.rex.RexOver;
21+
import org.apache.calcite.rex.RexWindowBounds;
22+
import org.apache.calcite.rex.RexWindowExclusion;
1723
import org.apache.calcite.sql.SqlFunction;
1824
import org.apache.calcite.sql.SqlFunctionCategory;
1925
import org.apache.calcite.sql.SqlKind;
@@ -323,6 +329,135 @@ private static void assertNoAnnotationInTree(RexNode node) {
323329
}
324330
}
325331

332+
// ---- Window functions ----
333+
334+
/**
335+
* PPL's {@code top}/{@code rare}/{@code streamstats} commands lower to
336+
* {@code ROW_NUMBER() OVER (PARTITION BY ... ORDER BY ...)} inside a {@code LogicalProject}.
337+
* The project rule narrows viable backends via {@link
338+
* org.opensearch.analytics.spi.BackendCapabilityProvider#windowCapabilities()} on the rule's
339+
* window-narrowing pass; the {@link RexOver} itself is left unannotated so that
340+
* strip-annotations / isthmus's {@code RexExpressionConverter#visitOver} can decode it
341+
* directly into a substrait {@code WindowFunctionInvocation}.
342+
*/
343+
public void testRowNumberOverPartitionByOrderByMarksAsWindow() {
344+
RexNode rowNumber = makeRowNumberOver(/*partitionField*/ 0, /*orderField*/ 1);
345+
OpenSearchProject result = runProject(
346+
rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.VARCHAR), 0),
347+
rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.INTEGER), 1),
348+
rowNumber
349+
);
350+
assertTrue(result.getViableBackends().contains(MockDataFusionBackend.NAME));
351+
// RexOver and pass-through field refs must NOT be annotated — RexOver dispatches through
352+
// RexExpressionConverter#visitOver downstream, which doesn't recognize the wrapper.
353+
assertFalse("Field ref must not be annotated", result.getProjects().get(0) instanceof AnnotatedProjectExpression);
354+
assertFalse("Field ref must not be annotated", result.getProjects().get(1) instanceof AnnotatedProjectExpression);
355+
assertFalse("RexOver must not be annotated", result.getProjects().get(2) instanceof AnnotatedProjectExpression);
356+
assertTrue(
357+
"Third project expression must remain a RexOver, was " + result.getProjects().get(2).getClass().getSimpleName(),
358+
result.getProjects().get(2) instanceof RexOver
359+
);
360+
}
361+
362+
/**
363+
* When no viable backend declares a {@link org.opensearch.analytics.spi.WindowCapability}
364+
* covering {@code ROW_NUMBER}, the planner must surface a capability-gap error at plan
365+
* time rather than failing later in substrait emission.
366+
*/
367+
public void testRowNumberWithoutWindowCapabilityErrors() {
368+
MockDataFusionBackend dfNoWindow = new MockDataFusionBackend() {
369+
@Override
370+
protected Set<org.opensearch.analytics.spi.WindowCapability> windowCapabilities() {
371+
return Set.of();
372+
}
373+
};
374+
RelOptTable table = mockTable(
375+
"test_index",
376+
new String[] { "name", "value" },
377+
new SqlTypeName[] { SqlTypeName.VARCHAR, SqlTypeName.INTEGER }
378+
);
379+
RexNode rowNumber = makeRowNumberOver(/*partitionField*/ 0, /*orderField*/ 1);
380+
LogicalProject project = LogicalProject.create(
381+
stubScan(table),
382+
List.of(),
383+
List.of(rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.VARCHAR), 0), rowNumber),
384+
List.of("name", "rn")
385+
);
386+
PlannerContext context = buildContext("parquet", nameValueFields(), List.of(dfNoWindow, LUCENE));
387+
IllegalStateException exception = expectThrows(IllegalStateException.class, () -> runPlanner(project, context));
388+
assertTrue(
389+
"Expected planner to surface window-function capability gap, got: " + exception.getMessage(),
390+
exception.getMessage().contains("No backend supports window functions") && exception.getMessage().contains("ROW_NUMBER")
391+
);
392+
}
393+
394+
/**
395+
* Strip-annotations on a project containing a {@link RexOver} hoists each unique window
396+
* call into a child {@link LogicalProject} (see {@code OpenSearchProject#liftNestedRexOver})
397+
* and rewrites the outer expression to a {@link RexInputRef} into the hoisted slot. The
398+
* outer project must carry no annotation wrappers, and the inner project must preserve the
399+
* {@link RexOver} verbatim so isthmus's {@code RexExpressionConverter#visitOver} can decode
400+
* it into a substrait {@code WindowFunctionInvocation}.
401+
*/
402+
public void testStripAnnotationsPreservesRexOver() {
403+
RexNode rowNumber = makeRowNumberOver(/*partitionField*/ 0, /*orderField*/ 1);
404+
OpenSearchProject annotated = runProject(rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.VARCHAR), 0), rowNumber);
405+
RelNode stripped = annotated.stripAnnotations(annotated.getInputs());
406+
assertTrue("Stripped plan must be a plain LogicalProject", stripped instanceof LogicalProject);
407+
List<RexNode> outerExprs = ((LogicalProject) stripped).getProjects();
408+
for (RexNode expr : outerExprs) {
409+
assertNoAnnotationInTree(expr);
410+
}
411+
assertTrue(
412+
"Hoisted outer expr must be a RexInputRef into the inner Project, was " + outerExprs.get(1).getClass().getSimpleName(),
413+
outerExprs.get(1) instanceof RexInputRef
414+
);
415+
416+
// The inner Project (hoisted from liftNestedRexOver) must hold the original RexOver
417+
// at the appended slot so substrait emission sees the WindowFunction at top level.
418+
RelNode innerInput = stripped.getInputs().get(0);
419+
assertTrue("Inner plan must be a LogicalProject carrying the hoisted RexOver", innerInput instanceof LogicalProject);
420+
List<RexNode> innerExprs = ((LogicalProject) innerInput).getProjects();
421+
boolean foundRexOver = false;
422+
for (RexNode innerExpr : innerExprs) {
423+
if (innerExpr instanceof RexOver) {
424+
foundRexOver = true;
425+
break;
426+
}
427+
}
428+
assertTrue("Inner Project must contain the hoisted RexOver, exprs=" + innerExprs, foundRexOver);
429+
}
430+
431+
/**
432+
* Builds a {@code ROW_NUMBER() OVER (PARTITION BY $partitionField ORDER BY $orderField)}
433+
* RexOver against the (VARCHAR, INTEGER) stub-scan schema.
434+
*/
435+
private RexNode makeRowNumberOver(int partitionField, int orderField) {
436+
RexNode partition = rexBuilder.makeInputRef(
437+
typeFactory.createSqlType(partitionField == 0 ? SqlTypeName.VARCHAR : SqlTypeName.INTEGER),
438+
partitionField
439+
);
440+
RexFieldCollation order = new RexFieldCollation(
441+
rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.INTEGER), orderField),
442+
Set.of()
443+
);
444+
return rexBuilder.makeOver(
445+
typeFactory.createSqlType(SqlTypeName.BIGINT),
446+
SqlStdOperatorTable.ROW_NUMBER,
447+
List.of(),
448+
List.of(partition),
449+
ImmutableList.of(order),
450+
RexWindowBounds.UNBOUNDED_PRECEDING,
451+
RexWindowBounds.CURRENT_ROW,
452+
RexWindowExclusion.EXCLUDE_NO_OTHER,
453+
true,
454+
true,
455+
false,
456+
false,
457+
false
458+
);
459+
}
460+
326461
// ---- Mixed backends in one projection ----
327462

328463
public void testMixedBackendsInProjection() {
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.analytics.qa;
10+
11+
import org.opensearch.client.Request;
12+
import org.opensearch.client.Response;
13+
14+
import java.io.IOException;
15+
import java.util.Arrays;
16+
import java.util.List;
17+
import java.util.Map;
18+
19+
/**
20+
* Self-contained integration test for PPL {@code rare} on the analytics-engine route.
21+
*
22+
* <p>Mirrors {@code CalciteRareCommandIT} / {@code RareCommandIT} from the
23+
* {@code opensearch-project/sql} repository. {@code rare} lowers the same way as
24+
* {@code top} via {@code CalciteRelNodeVisitor#visitRareTopN} — a
25+
* {@code LogicalProject} containing {@code ROW_NUMBER() OVER (PARTITION BY ... ORDER BY count ASC)} —
26+
* so it exercises the same {@code EngineCapability.WINDOW} flag in
27+
* {@code OpenSearchProjectRule} that this PR introduces.
28+
*/
29+
public class RareCommandIT extends AnalyticsRestTestCase {
30+
31+
private static final Dataset DATASET = new Dataset("calcs", "calcs");
32+
33+
private static boolean dataProvisioned = false;
34+
35+
private void ensureDataProvisioned() throws IOException {
36+
if (dataProvisioned == false) {
37+
DatasetProvisioner.provision(client(), DATASET);
38+
dataProvisioned = true;
39+
}
40+
}
41+
42+
// ── rare without group ─────────────────────────────────────────────────────
43+
44+
public void testRareWithoutGroup() throws IOException {
45+
// calcs.str0 has 3 distinct values with distinct counts → rare order is
46+
// deterministic ASC by count: {FURNITURE: 2, OFFICE SUPPLIES: 6, TECHNOLOGY: 9}.
47+
assertRowsEqual(
48+
"source=" + DATASET.indexName + " | rare str0",
49+
row("FURNITURE", 2),
50+
row("OFFICE SUPPLIES", 6),
51+
row("TECHNOLOGY", 9)
52+
);
53+
}
54+
55+
public void testRareWithGroup() throws IOException {
56+
// rare str3 per str0 group. calcs.str3 distribution by str0:
57+
// FURNITURE → {e: 2} → 1 row
58+
// OFFICE SUPPLIES → {e: 4, null: 2} → 2 rows
59+
// TECHNOLOGY → {null: 5, e: 4} → 2 rows
60+
// Total: 5 distinct (str0, str3) pairs under the default rare limit.
61+
assertNumOfRows("source=" + DATASET.indexName + " | rare str3 by str0", 5);
62+
}
63+
64+
// ── usenull behaviour ──────────────────────────────────────────────────────
65+
66+
public void testRareCommandUseNull() throws IOException {
67+
// calcs.int0 distribution: 6 nulls; 7 distinct non-null values {1, 3, 4 (×3),
68+
// 7, 8 (×3), 10, 11}. Default usenull=true → 8 bucket categories (7 + null).
69+
assertNumOfRows("source=" + DATASET.indexName + " | rare int0", 8);
70+
}
71+
72+
public void testRareCommandUseNullFalse() throws IOException {
73+
// usenull=false drops the null bucket → 7 result rows.
74+
assertNumOfRows("source=" + DATASET.indexName + " | rare usenull=false int0", 7);
75+
}
76+
77+
// ── helpers ─────────────────────────────────────────────────────────────────
78+
79+
private static List<Object> row(Object... values) {
80+
return Arrays.asList(values);
81+
}
82+
83+
@SafeVarargs
84+
@SuppressWarnings("varargs")
85+
private final void assertRowsEqual(String ppl, List<Object>... expected) throws IOException {
86+
Map<String, Object> response = executePpl(ppl);
87+
@SuppressWarnings("unchecked")
88+
List<List<Object>> actualRows = (List<List<Object>>) response.get("rows");
89+
assertNotNull("Response missing 'rows' for query: " + ppl, actualRows);
90+
assertEquals("Row count mismatch for query: " + ppl, expected.length, actualRows.size());
91+
for (int i = 0; i < expected.length; i++) {
92+
List<Object> want = expected[i];
93+
List<Object> got = actualRows.get(i);
94+
assertEquals(
95+
"Column count mismatch at row " + i + " for query: " + ppl,
96+
want.size(),
97+
got.size()
98+
);
99+
for (int j = 0; j < want.size(); j++) {
100+
assertCellEquals(
101+
"Cell mismatch at row " + i + ", col " + j + " for query: " + ppl,
102+
want.get(j),
103+
got.get(j)
104+
);
105+
}
106+
}
107+
}
108+
109+
private void assertNumOfRows(String ppl, int expectedRows) throws IOException {
110+
Map<String, Object> response = executePpl(ppl);
111+
@SuppressWarnings("unchecked")
112+
List<List<Object>> actualRows = (List<List<Object>>) response.get("rows");
113+
assertNotNull("Response missing 'rows' for query: " + ppl, actualRows);
114+
assertEquals("Row count mismatch for query: " + ppl, expectedRows, actualRows.size());
115+
}
116+
117+
/** Numeric-tolerant cell comparator (Jackson returns Integer/Long/Double interchangeably). */
118+
private static void assertCellEquals(String message, Object expected, Object actual) {
119+
if (expected == null || actual == null) {
120+
assertEquals(message, expected, actual);
121+
return;
122+
}
123+
if (expected instanceof Number && actual instanceof Number) {
124+
double e = ((Number) expected).doubleValue();
125+
double a = ((Number) actual).doubleValue();
126+
if (Double.compare(e, a) != 0) {
127+
fail(message + ": expected <" + expected + "> but was <" + actual + ">");
128+
}
129+
return;
130+
}
131+
assertEquals(message, expected, actual);
132+
}
133+
134+
private Map<String, Object> executePpl(String ppl) throws IOException {
135+
ensureDataProvisioned();
136+
Request request = new Request("POST", "/_analytics/ppl");
137+
request.setJsonEntity("{\"query\": \"" + escapeJson(ppl) + "\"}");
138+
Response response = client().performRequest(request);
139+
return assertOkAndParse(response, "PPL: " + ppl);
140+
}
141+
}

0 commit comments

Comments
 (0)