Support pushdown physical sort operator to speedup SortMergeJoin #3864

LantaoJin · 2025-07-10T10:12:14Z

Description

In Calcite + OpenSearch, the default physical Join operator is EnumerableMergeJoin:

EnumerableMergeJoin
\-EnumerableSort
  \-CalciteEnumerableIndexScan
\-EnumerableSort
  \-CalciteEnumerableIndexScan

After this patching, the EnumerableSort operators could be pushed down to DSL:

EnumerableMergeJoin
\-CalciteEnumerableIndexScan(PushDownContext=[SORT->"join_key"])
\-CalciteEnumerableIndexScan(PushDownContext=[SORT->"join_key"])

In v3, a string field can enable sort pushdown if

the field type is KEYWORD
or the field type is TEXT with keyword subfield

Related Issues

Resolves #3863

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Lantao Jin <ltjin@amazon.com>

…ta=true Signed-off-by: Lantao Jin <ltjin@amazon.com>

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java

Signed-off-by: Lantao Jin <ltjin@amazon.com>

opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchTextType.java

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin · 2025-07-14T09:53:22Z

...h/java/org/opensearch/sql/expression/operator/predicate/MergeArrayAndObjectMapBenchmark.java

    Map<String, OpenSearchDataType> finalResult = new HashMap<>();
    for (Map<String, OpenSearchDataType> map : candidateMaps) {
-      OpenSearchDescribeIndexRequest.mergeObjectAndArrayInsideMap(finalResult, map);
+      MergeRuleHelper.merge(finalResult, map);


not related. just fix a compile error in benchmarks module.

LantaoJin · 2025-07-14T09:55:53Z

integ-test/src/test/resources/indexDefinitions/account_index_mapping.json

+        "fielddata": true,
+        "fields": {
+          "keyword": {
+            "type": "keyword",
+            "ignore_above": 256
+          }
+        }


gender in test sample data should contain keyword subfield. If else, we cannot sorting, aggregate on it. Sorting on a fielddata field would get unstable result. ref link

LantaoJin · 2025-07-14T09:57:14Z

integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java

-          rows(1, 20, "f", "VA"),
-          rows(1, 30, "f", "IN"),
-          rows(1, 30, "f", "PA"),
-          rows(1, 30, "m", "IL"),
-          rows(1, 30, "m", "MD"),
-          rows(1, 30, "m", "TN"),
-          rows(1, 30, "m", "WA"));


this lower case values in v2 were caused by grouping by a fielddata field.

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin · 2025-07-15T08:48:33Z

integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java

        explainQueryToString(
            "source=opensearch-sql_test_index_account "
                + "| sort account_number, firstname, address, balance "
-                + "| sort - balance, - gender, address "


We do not sort on text type without keyword subfield in v3. Even sorting on text type with fielddata=true will get unexpected order:
For example, if gender is a text type field with fielddata=true.

Check following queries and their outputs in v2.

index=test1:

gender

M a 1

M b 1

F c 2

F d d 1

source = test1 | sort gender returns

gender

M a 1

M b 1

F d d 1

F c 2

source = test1 | sort - gender returns

gender

M a 1

M b 1

F c 2

F d d 1

index=test2:

gender

F d d 1

M a 1

F c 2

M b 1

source=test2 | sort gender returns

gender

F d d 1

M a 1

M b 1

F c 2

source = test2 | sort - gender returns

gender

M a 1

M b 1

F d d 1

F c 2

The logic of sorting a fielddata is 1) tokenization 2) internal sort by token 3) sort by first token
ASC: "F d d 1" =tokenization=> "f", "d", "d", "1" =internal sort by token=> "1", "f", "d", "d" =sort by first token=> "1"
DESC: "F d d 1" =tokenization=> "f", "d", "d", "1" =internal sort by token=> "f", "d", "d", "1" =sort by first token=> "f"

LantaoJin · 2025-07-15T14:02:10Z

Could you review this again?@qianheng-aws @penghuo

Signed-off-by: Lantao Jin <ltjin@amazon.com>

qianheng-aws · 2025-07-16T03:57:50Z

integ-test/src/test/java/org/opensearch/sql/ppl/RareCommandIT.java

          rows("F", "KS", 7),
          rows("F", "CO", 7),
-          rows("F", "NV", 8),
+          isPushdownEnabled() ? rows("F", "AR", 8) : rows("F", "NV", 8),


How do we introduce this disparity between with&without pushdown?

I added "keyword" subfield for the gender, agg pushdown by gender works in caclite.

Why pushdown enable impact results?

Why pushdown enable impact results?

This result difference is not introduced by code changes in this PR. You can reproduce it in main branch when change the schema from

"gender": { "type": "text", "fielddata": true },

to

"gender": { "type": "text", "fields": { "keyword": { "type": "keyword", "ignore_above": 256 } } },

The three results (v2 non-pushdown, v3 non-pushdown, v3 pushdown) are different.

v2 non-pushdown v3 non-pushdown v3 pushdown

"F", "VA", 8 "F", "NV", 8 "F", "AR", 8

But they are all correct. The rare command is not a deterministic command, it result depends on the order of return from OpenSearch and implementation (for v2). When the bucket pushdown works (by gender), the fetched data order is different with data order in non-pushdown.

penghuo · 2025-07-16T16:50:42Z

integ-test/src/test/java/org/opensearch/sql/ppl/RareCommandIT.java

          rows("F", "KS", 7),
          rows("F", "CO", 7),
-          rows("F", "NV", 8),
+          isPushdownEnabled() ? rows("F", "AR", 8) : rows("F", "NV", 8),


Why pushdown enable impact results?

penghuo · 2025-07-16T18:05:06Z

...rc/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchSortIndexScanRule.java

  public void onMatch(RelOptRuleCall call) {
-    final LogicalSort sort = call.rel(0);
-    final CalciteLogicalIndexScan scan = call.rel(1);
+    final Sort sort = call.rel(0);


@LantaoJin @qianheng-aws couple questions?

Is OpenSearchSortIndexScanRule intended to be a physical tweak rule for EnumerableRel?

Do we still need Logical plan optimization rule and CalciteLogicalIndexScan? Can all existing rules be expressed as physical tweak rules for EnumerableRel?

EnumerableSort can generate from logical sort operator and logical join operator with physical SortMerge implementation. So I changed the OpenSearchSortIndexScanRule rule from logical layer to physical layer. It equals keeping the OpenSearchSortIndexScanRule in logical layer and adding a new rule for EnumerableSort + SortMergeJoin in physical layer.

Almost pushdown rules can move to physical layer but I didn't see any benefit to do that. We need logical plan rules. some optimization should happen in logical layer instead of physical. For example a LogicalJoin operator may generate multiple physical Join operators. rule applying in logical should be more efficient.

opensearch-trigger-bot · 2025-07-17T03:55:49Z

The backport to 2.19-dev failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-3864-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 71aa9ba0490305b81284eb369f212a8fc6e3936f
# Push it to GitHub
git push --set-upstream origin backport/backport-3864-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-dev

Then, create a pull request where the base branch is 2.19-dev and the compare/head branch is backport/backport-3864-to-2.19-dev.

…nsearch-project#3864) * Support pushdown physical sort operator to speedup SortMergeJoin Signed-off-by: Lantao Jin <ltjin@amazon.com> * Enable sort pushdown when the type is TEXT without fields and fielddata=true Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix UT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Add OpenSearchTextType.toKeywordSubField() Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Only push down sort when order is ASC for fielddata text Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert CalciteNoPushdownIT Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert the fielddata logic Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * 'gender' in test data should contain keyword subfield Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert the typo change in test Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix no pushdown IT Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit 71aa9ba)

…p SortMergeJoin (#3864) (#3889) * Support pushdown physical sort operator to speedup SortMergeJoin (#3864) * Support pushdown physical sort operator to speedup SortMergeJoin Signed-off-by: Lantao Jin <ltjin@amazon.com> * Enable sort pushdown when the type is TEXT without fields and fielddata=true Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix UT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Add OpenSearchTextType.toKeywordSubField() Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * Only push down sort when order is ASC for fielddata text Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert CalciteNoPushdownIT Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert the fielddata logic Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * 'gender' in test data should contain keyword subfield Signed-off-by: Lantao Jin <ltjin@amazon.com> * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert the typo change in test Signed-off-by: Lantao Jin <ltjin@amazon.com> * fix no pushdown IT Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit 71aa9ba) * Fix IT Signed-off-by: Lantao Jin <ltjin@amazon.com> * revert changes in build.gradle Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com>

Support pushdown physical sort operator to speedup SortMergeJoin

24a0488

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin added enhancement New feature or request calcite calcite migration releated pushdown pushdown related issues labels Jul 10, 2025

Enable sort pushdown when the type is TEXT without fields and fieldda…

9ebdb92

…ta=true Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin marked this pull request as ready for review July 10, 2025 15:38

LantaoJin requested review from GumpacG, MaxKsyunz, Swiddis, YANG-DB, Yury-Fridlyand, acarbonetto, anirudha, dai-chen, derek-ho, forestmvey, joshuali925, kavithacm, mengweieric, noCharger, penghuo, ps48, qianheng-aws, seankao-az and ykmr1224 as code owners July 10, 2025 15:38

LantaoJin commented Jul 10, 2025

View reviewed changes

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java Outdated Show resolved Hide resolved

Fix UT

7142b9a

Signed-off-by: Lantao Jin <ltjin@amazon.com>

qianheng-aws reviewed Jul 11, 2025

View reviewed changes

opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchTextType.java Outdated Show resolved Hide resolved

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java Outdated Show resolved Hide resolved

LantaoJin added 2 commits July 11, 2025 15:52

Add OpenSearchTextType.toKeywordSubField()

d6b1b67

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Merge remote-tracking branch 'upstream/main' into issues/3863

1edb107

LantaoJin added 4 commits July 11, 2025 18:29

Fix IT

dd8334d

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Fix IT

7773d04

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Only push down sort when order is ASC for fielddata text

7c4717e

Signed-off-by: Lantao Jin <ltjin@amazon.com>

revert CalciteNoPushdownIT

d5ae3ab

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin requested a review from qianheng-aws July 11, 2025 15:05

LantaoJin added the backport 2.19-dev label Jul 11, 2025

LantaoJin added 3 commits July 14, 2025 15:41

revert the fielddata logic

fa0fb77

Signed-off-by: Lantao Jin <ltjin@amazon.com>

Fix IT

23e179e

Signed-off-by: Lantao Jin <ltjin@amazon.com>

'gender' in test data should contain keyword subfield

3e9e60b

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin commented Jul 14, 2025

View reviewed changes

Fix IT

2d0b7c7

Signed-off-by: Lantao Jin <ltjin@amazon.com>

LantaoJin commented Jul 15, 2025

View reviewed changes

LantaoJin added 2 commits July 15, 2025 22:05

revert the typo change in test

4ecd58f

Signed-off-by: Lantao Jin <ltjin@amazon.com>

fix no pushdown IT

511400f

Signed-off-by: Lantao Jin <ltjin@amazon.com>

qianheng-aws reviewed Jul 16, 2025

View reviewed changes

qianheng-aws approved these changes Jul 16, 2025

View reviewed changes

penghuo reviewed Jul 16, 2025

View reviewed changes

LantaoJin requested a review from penghuo July 17, 2025 03:19

penghuo approved these changes Jul 17, 2025

View reviewed changes

penghuo merged commit 71aa9ba into opensearch-project:main Jul 17, 2025
23 checks passed

opensearch-trigger-bot bot added the backport-failed label Jul 17, 2025

LantaoJin mentioned this pull request Jul 17, 2025

[Backport 2.19-dev] Support pushdown physical sort operator to speedup SortMergeJoin (#3864) #3889

Merged

LantaoJin added the backport-manually Filed a PR to backport manually. label Jul 22, 2025

opensearch-project deleted a comment from opensearch-trigger-bot bot Aug 13, 2025

Support pushdown physical sort operator to speedup SortMergeJoin #3864

Support pushdown physical sort operator to speedup SortMergeJoin #3864

Uh oh!

Conversation

LantaoJin commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Jul 14, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Jul 15, 2025 • 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!

LantaoJin commented Jul 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin Jul 17, 2025 • 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!

Uh oh!

opensearch-trigger-bot bot commented Jul 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LantaoJin commented Jul 10, 2025 •

edited

Loading

LantaoJin Jul 14, 2025 •

edited

Loading

LantaoJin Jul 15, 2025 •

edited

Loading

LantaoJin Jul 17, 2025 •

edited

Loading