Summary by CodeRabbit

• Tests

  • Added integration test verifying random-sort + head caps results to 5.
  • Added explain test ensuring nondeterministic sort keys are not pushed down.
  • Added test verifying system query size limits are respected for sort expressions.

• Bug Fixes

  • Tightened validation for sort pushdown to reject nondeterministic expressions and preserve correct limit/offset behavior.

Walkthrough

Adds an integration YAML test for issue 5114 and two new integration tests; tightens SortExprIndexScanRule to prevent pushing non-deterministic sort expressions and restrict limit/offset pushdown to LogicalSort-backed scans.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Support sort expression pushdown for SortMergeJoin #4830 — related changes to sort-expression pushdown and planner rules.

Suggested labels

testing

Suggested reviewers

• qianheng-aws
• ps48
• kavithacm
• dai-chen
• anirudha
• MaxKsyunz
• Yury-Fridlyand
• Swiddis

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

• 📝 Generate docstrings

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java (1)

1070-1078: Good regression test for the non-deterministic sort pushdown fix.

The test correctly validates both that SORT_EXPR is not pushed down and that LogicalSort remains in the plan when sorting by a non-deterministic expression (rand()).

Minor style nit: this test uses Assert.assertFalse/Assert.assertTrue (lines 1076-1077) while most other tests in this class use the unqualified assertTrue/assertFalse inherited from the base class. Consider aligning for consistency.

Style alignment

-    Assert.assertFalse(result.contains("SORT_EXPR->["));
-    Assert.assertTrue(result.contains("LogicalSort("));
+    assertFalse(result.contains("SORT_EXPR->["));
+    assertTrue(result.contains("LogicalSort("));

• X
• Mastodon
• Reddit
• LinkedIn