Enhance the cost computing mechanism and push down context #4353

qianheng-aws · 2025-09-23T09:44:08Z

Description

This PR includes changes:

Support cacheing OpenSearchRequestBuilder inner PushContext. Otherwise it will be built twice for the cheapest plan.
Ehance the cost computing mechanism by:
- Split method computeSelfCost from estimateRowCount. In computeSelfCost, we've implemented more proper cost computing mechanism rather than relying on rows count only.
- Support more accurate script cost by involving script count rather than the status of whether containing scripts.
Move part of our explain IT from json format to yaml format.

The new cost computing mechanism has advantages in several cases:

The cost among scans with the same OpenSearch request will get equivilant cost. For example, before this PR, Scan(PushDownContext[LIMIT, PROJECT]) will get smaller cost than Scan(PushDownContext[PROJECT, LIMIT]) although they produce the same request. With this PR, they two will get the equivilent cost. And then the optimizer will finally choose the plan with push down sequence more close to the original logical plan.
Before this PR, the optimizer will choose plan like Scan(PushDownContext[AGG, PROJECT]) - Aggregate although the 2 agg could be merged. With this PR, the optimizer will choose the scan with a merged agg pushed down.
Before this PR, the cases like https://github.com/opensearch-project/sql/pull/4279#discussion_r2354020132 with several scripts in our agg builder, the optimizer will still choose it since scirpt count won't affect the cost of the plan. This PR will address it.

Related Issues

Resolves #4312

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
New PPL command checklist all confirmed.
API changes companion pull request created.
Commits are signed per the DCO using --signoff or -s.
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: Heng Qian <[email protected]>

# Conflicts: # integ-test/src/test/resources/expectedOutput/calcite/explain_agg_with_sum_enhancement.json

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws · 2025-09-24T05:33:03Z

core/src/main/java/org/opensearch/sql/utils/YamlFormatter.java

        YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS); // Quote numeric strings
    yamlFactory.enable(YAMLGenerator.Feature.INDENT_ARRAYS_WITH_INDICATOR);
    YAML_MAPPER = new ObjectMapper(yamlFactory);
-    YAML_MAPPER.enable(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS);


Remove this config, otherwise for the plan of v2 which contains multi-children for one level, it will put the children at beginning and the operator name at the end

# Conflicts: # integ-test/src/test/resources/expectedOutput/calcite/explain_patterns_simple_pattern_agg_push.json # integ-test/src/test/resources/expectedOutput/calcite_no_pushdown/explain_patterns_simple_pattern_agg_push.json

Signed-off-by: Heng Qian <[email protected]>

LantaoJin · 2025-09-24T08:16:35Z

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/PushDownContext.java

+    return this.stream().anyMatch(action -> action.digest().equals(digest));
+  }
+
+  public synchronized OpenSearchRequestBuilder createOrGetRequestBuilder() {


add java doc

sync offline, let' move this method out

LantaoJin · 2025-09-24T08:41:50Z

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

+   * than non-pushdown operators.
+   */
+  @Override
+  public @Nullable RelOptCost computeSelfCost(RelOptPlanner planner, RelMetadataQuery mq) {


please add a UT for these two method to ensure the behaviour would be stable.

LantaoJin · 2025-09-24T08:49:50Z

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

+        case SORT -> dCpu += dRows;
+          // Refer the org.apache.calcite.rel.metadata.RelMdRowCount.getRowCount(Aggregate rel,...)
+        case COLLAPSE -> {
+          dRows = dRows / 10;


what the diff with https://github.com/opensearch-project/sql/pull/4353/files#diff-70fd6c948cefa04f6fa28ddd2ccf0e85d6cbce9fa8153b995628e186b1bd918dR109?

LantaoJin · 2025-09-24T08:50:39Z

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

+          // Try to reduce the rows count by 1 to make the cost cheaper slightly than non-push down.
+          // Because we'd like to push down LIMIT even when the fetch in LIMIT is greater than
+          // dRows.
+        case LIMIT -> dRows = Math.min(dRows, ((LimitDigest) operation.digest()).limit()) - 1;


could be negative with -1?

It could be but VolcanoPlanner in Clacite will replace it with TINY cost it's less than 0.

yuancu · 2025-09-24T12:31:16Z

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

+          // Calculate the cost of script filter by multiplying the selectivity of the filter and
+          // the factor amplified by script count.
+          dCpu +=
+              NumberUtil.multiply(
+                  NumberUtil.multiply(dRows, RelMdUtil.guessSelectivity(filterDigest.condition())),
+                  Math.pow(1.1, filterDigest.scriptCount()));


Will Math.pow(1.1, filterDigest.scriptCount() - 1) be better? It ensures that when there is only one script, the cost of pushed down script filter is smaller than that the same filter in memory ( dRow * guessedFactor * estimateRowCountFactor < dRow * guessedFactor, otherwise it's something like dRow * guessedFactor * 1.1 * estimateRowCountFactor, which is not necessarily smaller than dRow * guessedFactor)

The default value of estimateRowCountFactor is 0.9, so the cost of pushed-down filter is smaller.

On the other hand, the biggest reduction of cost from pushed-down filter is not from the cost reduction on filter iteself, but the final output rows count of the scan with filter push down.

Signed-off-by: Heng Qian <[email protected]>

# Conflicts: # integ-test/src/test/resources/expectedOutput/calcite/explain_agg_script_timestamp_push.json # integ-test/src/test/resources/expectedOutput/calcite/explain_limit_agg_pushdown2.json # opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java # opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/CalciteLogicalIndexScan.java

Signed-off-by: Heng Qian <[email protected]>

# Conflicts: # integ-test/src/test/resources/expectedOutput/ppl/explain_patterns_simple_pattern_agg_push.json

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws · 2025-09-26T09:50:45Z

@LantaoJin There are some enhancement on plan with this PR:
explain_agg_with_sum_enhancement.yaml
explain_timechart_count.yaml
explain_filter_then_limit_push.yaml
explain_agg_counts_by1
explain_agg_counts_by2
explain_filter_push.yaml
explain_filter_push_compare_date_string.yaml
explain_filter_push_compare_time_string.yaml
explain_filter_push_compare_timestamp_string.yaml

And #4377 is no longer needed after this PR. I've removed the code and keep the yaml test only.

opensearch-trigger-bot · 2025-09-28T03:30:11Z

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-4353-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 585a04c5de2dedcb3e7c2abf130846caac4dee50
# Push it to GitHub
git push --set-upstream origin backport/backport-4353-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-4353-to-2.19-dev.

…h-project#4353) * Make PushDownContext support caching the built request builder Signed-off-by: Heng Qian <[email protected]> * Move PushDownContext outside of the AbstractCalciteIndexScan Signed-off-by: Heng Qian <[email protected]> * Refactor cost computing Signed-off-by: Heng Qian <[email protected]> * migrate explain IT from json format to yaml format Signed-off-by: Heng Qian <[email protected]> * Fix opensearch request reuse by equivalent operator Signed-off-by: Heng Qian <[email protected]> * fix IT after merging main Signed-off-by: Heng Qian <[email protected]> * remove plans assert in execution IT Signed-off-by: Heng Qian <[email protected]> * Merging main Signed-off-by: Heng Qian <[email protected]> * Add UT for cost evaluation of AbstractCalciteIndexScan Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> (cherry picked from commit 585a04c) Signed-off-by: Heng Qian <[email protected]>

…n context #4353 (#4399) * Enhance the cost computing mechanism and push down context (#4353) * Make PushDownContext support caching the built request builder Signed-off-by: Heng Qian <[email protected]> * Move PushDownContext outside of the AbstractCalciteIndexScan Signed-off-by: Heng Qian <[email protected]> * Refactor cost computing Signed-off-by: Heng Qian <[email protected]> * migrate explain IT from json format to yaml format Signed-off-by: Heng Qian <[email protected]> * Fix opensearch request reuse by equivalent operator Signed-off-by: Heng Qian <[email protected]> * fix IT after merging main Signed-off-by: Heng Qian <[email protected]> * remove plans assert in execution IT Signed-off-by: Heng Qian <[email protected]> * Merging main Signed-off-by: Heng Qian <[email protected]> * Add UT for cost evaluation of AbstractCalciteIndexScan Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> * merging main Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> (cherry picked from commit 585a04c) Signed-off-by: Heng Qian <[email protected]> * fix compiling Signed-off-by: Heng Qian <[email protected]> * fix IT Signed-off-by: Heng Qian <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]>

qianheng-aws added 4 commits September 22, 2025 13:43

Make PushDownContext support caching the built request builder

da16493

Signed-off-by: Heng Qian <[email protected]>

Move PushDownContext outside of the AbstractCalciteIndexScan

061d74b

Signed-off-by: Heng Qian <[email protected]>

Refactor cost computing

c35062a

Signed-off-by: Heng Qian <[email protected]>

migrate explain IT from json format to yaml format

20981cf

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws added enhancement New feature or request calcite calcite migration releated pushdown pushdown related issues labels Sep 23, 2025

Fix opensearch request reuse by equivalent operator

493164e

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws added 3 commits September 24, 2025 10:50

Merge remote-tracking branch 'refs/remotes/origin/main' into costRefact

140af5c

# Conflicts: # integ-test/src/test/resources/expectedOutput/calcite/explain_agg_with_sum_enhancement.json

fix IT after merging main

5c83bb0

Signed-off-by: Heng Qian <[email protected]>

remove plans assert in execution IT

6544a62

Signed-off-by: Heng Qian <[email protected]>

qianheng-aws commented Sep 24, 2025

View reviewed changes

qianheng-aws added 2 commits September 24, 2025 15:11

Merging main

7ded561

Signed-off-by: Heng Qian <[email protected]>

LantaoJin reviewed Sep 24, 2025

View reviewed changes

yuancu reviewed Sep 24, 2025

View reviewed changes

qianheng-aws added 6 commits September 26, 2025 16:52

Add UT for cost evaluation of AbstractCalciteIndexScan

63d9774

Signed-off-by: Heng Qian <[email protected]>

merging main

820aee0

Signed-off-by: Heng Qian <[email protected]>

merging main

9728a5a

Signed-off-by: Heng Qian <[email protected]>

Merge remote-tracking branch 'refs/remotes/origin/main' into costRefact

a10714c

# Conflicts: # integ-test/src/test/resources/expectedOutput/ppl/explain_patterns_simple_pattern_agg_push.json

merging main

7f6c308

Signed-off-by: Heng Qian <[email protected]>

yuancu approved these changes Sep 26, 2025

View reviewed changes

LantaoJin approved these changes Sep 28, 2025

View reviewed changes

LantaoJin merged commit 585a04c into opensearch-project:main Sep 28, 2025
33 checks passed

LantaoJin added the backport 2.19-dev label Sep 28, 2025

opensearch-trigger-bot bot added the backport-failed label Sep 28, 2025

qianheng-aws mentioned this pull request Sep 28, 2025

[Backport 2.19-dev] Enhance the cost computing mechanism and push down context #4353 #4399

Merged

qianheng-aws added the backport-manually Filed a PR to backport manually. label Sep 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance the cost computing mechanism and push down context #4353

Enhance the cost computing mechanism and push down context #4353

Uh oh!

qianheng-aws commented Sep 23, 2025 •

edited

Loading

Uh oh!

qianheng-aws Sep 24, 2025

Uh oh!

LantaoJin Sep 24, 2025

Uh oh!

LantaoJin Sep 24, 2025

Uh oh!

LantaoJin Sep 24, 2025

Uh oh!

LantaoJin Sep 24, 2025

Uh oh!

LantaoJin Sep 24, 2025

Uh oh!

qianheng-aws Sep 26, 2025

Uh oh!

yuancu Sep 24, 2025

Uh oh!

qianheng-aws Sep 26, 2025

Uh oh!

qianheng-aws commented Sep 26, 2025

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Sep 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Enhance the cost computing mechanism and push down context #4353

Enhance the cost computing mechanism and push down context #4353

Uh oh!

Conversation

qianheng-aws commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

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!

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!

qianheng-aws commented Sep 26, 2025

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Sep 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qianheng-aws commented Sep 23, 2025 •

edited

Loading