Filter script pushdown with RelJson serialization in Calcite #3859

songkant-aws · 2025-07-09T10:29:53Z

Description

The work on top of preivous Calcite filter script pushdown: #3812.
The issue of previous PR is it sends unsafe arbitrary Java code string to OpenSearch DataNode to compile it, leading to security concern where malicious user could use this exposed script field to run any code to attack cluster.

In this PR, it leverages Calcite existing RexNode serialization mechanism to mitigate security issue. We're now transporting logical expression representation, aka RexNode that will not allow user to customize arbitrary programming logic. The registered function calls are limited to PPLBuiltin UDFs and Calcite standard UDFs. The pushdown now undergoes this way:

|-----------------Coordinator Node -----------------|
optimized RexNode -> encoded RexNode Json String

---transport call-->

|--------------------------------------------------------Data Node -----------------------------------------------------|
decoded RexNode Json String -> deserialize as RexNode -> translate to Linq4j expressions -> code compile expressions and run

Related Issues

Resolves #3379

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: Heng Qian <[email protected]>

…shDown

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

…string Signed-off-by: Songkan Tang <[email protected]>

Signed-off-by: Songkan Tang <[email protected]>

LantaoJin · 2025-07-09T12:32:57Z

@songkant-aws I remember you mentioned "Calcite has the RexNode serialization related implementation", can you point it out here?

songkant-aws · 2025-07-09T12:37:05Z

...rch/src/main/java/org/opensearch/sql/opensearch/storage/serialization/RelJsonSerializer.java

+      RexNode rexNode, RelDataType relDataType, Map<String, ExprType> fieldTypes) {
+    try {
+      // Serialize RexNode and RelDataType by JSON
+      JsonBuilder jsonBuilder = new JsonBuilder();


Calcite related RexNode and RelDataType serialization

songkant-aws · 2025-07-09T12:37:56Z

...rch/src/main/java/org/opensearch/sql/opensearch/storage/serialization/RelJsonSerializer.java

+      RelJson relJson = RelJson.create();
+      Map<String, Object> rowTypeMap = mapper.readValue((String) objectMap.get(ROW_TYPE), TYPE_REF);
+      RelDataType rowType = relJson.toType(cluster.getTypeFactory(), rowTypeMap);
+      OpenSearchRelInputTranslator inputTranslator = new OpenSearchRelInputTranslator(rowType);
+      relJson = relJson.withInputTranslator(inputTranslator).withOperatorTable(pplSqlOperatorTable);
+      Map<String, Object> exprMap = mapper.readValue((String) objectMap.get(EXPR), TYPE_REF);
+      RexNode rexNode = relJson.toRex(cluster, exprMap);


Calcite RexNode and RelDataType deserialization related logic

songkant-aws · 2025-07-09T12:38:38Z

...n/java/org/opensearch/sql/opensearch/storage/serialization/OpenSearchRelInputTranslator.java

+  }
+
+  @Override
+  public RexNode translateInput(


Customized handler to handle RexNode's RelInput

songkant-aws · 2025-07-09T12:41:04Z

@LantaoJin See comments inline for Calcite RexNode serialization usage by JSON.

LantaoJin · 2025-07-09T12:50:19Z

...rch/src/main/java/org/opensearch/sql/opensearch/storage/serialization/RelJsonSerializer.java

+  public static final String EXPR = "expr";
+  public static final String FIELD_TYPES = "fieldTypes";
+  public static final String ROW_TYPE = "rowType";
+  private static final ObjectMapper mapper = new ObjectMapper();


It brings me that we can leverage jackson-dataformat-msgpack which is a Jackson extension library to serialize the json to binary. (could be faster and memory saving)

private static final ObjectMapper mapper = new ObjectMapper(new MessagePackFactory());

Ref https://github.com/msgpack/msgpack-java/blob/main/msgpack-jackson/README.md

I checked the Calcite RelJson code. Unfortunately, it doesn't rely on ObjectMapper. It implements its own JsonBuilder to write RexNode to json string. So we cannot leverage this msgpack extension.

qianheng-aws · 2025-07-10T03:58:57Z

...n/java/org/opensearch/sql/opensearch/storage/serialization/OpenSearchRelInputTranslator.java

+    final RexBuilder rexBuilder = cluster.getRexBuilder();
+
+    // Check if it is a local ref.
+    if (map.containsKey("type")) {


We can remove this handling logic or throw exception if we won't really have local reference.

It's inappropriate to put input's RowType in local ref

Yes, we can remove it. I don't see we have such LocalRef case.

Signed-off-by: Songkan Tang <[email protected]>

songkant-aws · 2025-07-10T08:25:34Z

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

+            + " | STATS count(AcceptedAnswerId) as count"
+            + " | EVAL dateStr = makedate(2025, count)"
+            + " | WHERE simple_query_string(['dateStr'], 'taste')";


Change to more complex ppl query because now most of EVAL filters can be pushed down as script expression. Adding agg before filters will prevent them pushdown.

Signed-off-by: Songkan Tang <[email protected]>

LantaoJin · 2025-07-16T05:41:43Z

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

@@ -426,6 +427,28 @@ public void testMultiFieldsRelevanceQueryFunctionExplain() throws IOException {
                + " default_operator='or', analyzer=english)"));
  }

+  @Ignore("The serialized string is unstable because of function properties")


@songkant-aws can you add above test case in explain IT?

LantaoJin · 2025-07-16T06:13:00Z

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java

+  }
+
+  /** Expression script language name. */
+  public static final String CALCITE_LANG_NAME = "opensearch_calcite";


for more readable, change to:

public static final String EXPRESSION_LANG_NAME = "opensearch_calcite_expression";

Signed-off-by: Songkan Tang <[email protected]>

LantaoJin · 2025-07-18T10:25:58Z

@songkant-aws can you fix the conflicts, I think the enhancement has been blocked for too long. @penghuo @qianheng-aws any other concerns? can we merge it first?

qianheng-aws · 2025-07-21T02:21:46Z

LGTM. Please resolve the conflict @songkant-aws

Signed-off-by: Songkan Tang <[email protected]>

qianheng-aws · 2025-07-22T02:14:57Z

opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java

+    private Expression tryAnalyzeOperand(RexNode node) {
+      try {
+        Expression expr = node.accept(this);
+        if (expr instanceof NamedFieldExpression) {


[question] In which case will we go into this?

No, we don't have such case. So it's noop when we encounter it.

Seems will throw exception if invoking CompoundQueryExpression ::and later

sql/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java

Line 927 in 37536b8

for (QueryExpression expression : expressions) {

I'm thinking we do will meet this after merging this PR: #3849, since it support using a singleton field as a condition in filer. #3849 (review)

But it needs more change to support push down such NamedFieldExpression, better in a separate PR.

It should never happen. We could just throw exception here.

It's OK to me leaving this code here. Current behavior of throwing exception in CompoundQueryExpression ::and makes more sense to me comparing to ignoring it. Ignoring NamedFieldExpression seems will induce to correctness issue.

@LantaoJin Do you have any other concern on this?

@LantaoJin Do you have any other concern on this?

Looks good to me as long as the case where field1=10 or field2 can apply partial pushdown field1=10.

opensearch-trigger-bot · 2025-07-22T03:09:13Z

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

LantaoJin · 2025-07-22T03:31:20Z

@songkant-aws please manually backport it to 2.19-dev

…rch-project#3859) * Support filter script push down for calcite Signed-off-by: Heng Qian <[email protected]> * Fix UT Signed-off-by: Heng Qian <[email protected]> * Change CompoundedScriptEngine name Signed-off-by: Heng Qian <[email protected]> * Remove duplicated code after merging main Signed-off-by: Heng Qian <[email protected]> * Create RelJsonSerializer to serialize RexNode instead of unsafe code string Signed-off-by: Songkan Tang <[email protected]> * Fix some IT Signed-off-by: Songkan Tang <[email protected]> * Add validation logic before script serialization Signed-off-by: Songkan Tang <[email protected]> * Resolve compilation issue after merge Signed-off-by: Songkan Tang <[email protected]> * Comply with relevance query pushdown Signed-off-by: Songkan Tang <[email protected]> * Add cost computing for script pushdown to balance performance Signed-off-by: Songkan Tang <[email protected]> * Correct test case after merge Signed-off-by: Songkan Tang <[email protected]> * Add V2Expression vs RexNode serde benchmark Signed-off-by: Songkan Tang <[email protected]> * Fix spotless check Signed-off-by: Songkan Tang <[email protected]> * Address comments Signed-off-by: Songkan Tang <[email protected]> * Fix IT after change Signed-off-by: Songkan Tang <[email protected]> * Encode langType into script JSON blob Signed-off-by: Songkan Tang <[email protected]> * Fix issue after merge Signed-off-by: Songkan Tang <[email protected]> * Consolidate with partial pushdown logic after merge Signed-off-by: Songkan Tang <[email protected]> * Fix UT Signed-off-by: Songkan Tang <[email protected]> --------- Signed-off-by: Heng Qian <[email protected]> Signed-off-by: Songkan Tang <[email protected]> Co-authored-by: Heng Qian <[email protected]> (cherry picked from commit 0a07684)

qianheng-aws and others added 7 commits June 23, 2025 18:23

Support filter script push down for calcite

1894875

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

Fix UT

13acaf5

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

Change CompoundedScriptEngine name

b37dc92

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

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

1abf8aa

…shDown

Remove duplicated code after merging main

bc6ecac

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

Create RelJsonSerializer to serialize RexNode instead of unsafe code …

cb3a2fe

…string Signed-off-by: Songkan Tang <[email protected]>

Fix some IT

700fc29

Signed-off-by: Songkan Tang <[email protected]>

songkant-aws commented Jul 9, 2025

View reviewed changes

LantaoJin reviewed Jul 9, 2025

View reviewed changes

LantaoJin added enhancement New feature or request pushdown pushdown related issues labels Jul 9, 2025

qianheng-aws reviewed Jul 10, 2025

View reviewed changes

songkant-aws added 4 commits July 10, 2025 12:16

Add validation logic before script serialization

fc77e5e

Signed-off-by: Songkan Tang <[email protected]>

Merge branch 'main' into rexnode-json-serialization

aba4fed

Resolve compilation issue after merge

2cf247d

Signed-off-by: Songkan Tang <[email protected]>

Comply with relevance query pushdown

3049c0c

Signed-off-by: Songkan Tang <[email protected]>

songkant-aws changed the title ~~[DRAFT] Create RelJsonSerializer to serialize RexNode instead of unsafe code string~~ Filter script pushdown with RelJson serialization in Calcite Jul 10, 2025

songkant-aws commented Jul 10, 2025

View reviewed changes

songkant-aws marked this pull request as ready for review July 10, 2025 08:27

songkant-aws requested review from ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB and mengweieric as code owners July 10, 2025 08:27

yuancu mentioned this pull request Jul 16, 2025

Pull/3859 yuancu/sql-plugin#5

Open

Address comments

ee67876

Signed-off-by: Songkan Tang <[email protected]>

LantaoJin reviewed Jul 16, 2025

View reviewed changes

songkant-aws added 5 commits July 16, 2025 17:14

Fix IT after change

c18006a

Signed-off-by: Songkan Tang <[email protected]>

Encode langType into script JSON blob

ffd26a7

Signed-off-by: Songkan Tang <[email protected]>

Merge branch 'main' into rexnode-json-serialization

3ff2f50

Merge branch 'main' into rexnode-json-serialization

2fee6ee

Fix issue after merge

08cd417

Signed-off-by: Songkan Tang <[email protected]>

Merge branch 'main' into rexnode-json-serialization

fde5cf0

penghuo previously approved these changes Jul 18, 2025

View reviewed changes

LantaoJin previously approved these changes Jul 19, 2025

View reviewed changes

qianheng-aws previously approved these changes Jul 21, 2025

View reviewed changes

songkant-aws added 2 commits July 21, 2025 13:55

Merge branch 'main' into rexnode-json-serialization

d2a95ea

Consolidate with partial pushdown logic after merge

1dd6752

Signed-off-by: Songkan Tang <[email protected]>

songkant-aws dismissed stale reviews from qianheng-aws, LantaoJin, and penghuo via 1dd6752 July 21, 2025 09:20

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

Fix UT

573552b

Signed-off-by: Songkan Tang <[email protected]>

penghuo approved these changes Jul 21, 2025

View reviewed changes

qianheng-aws reviewed Jul 22, 2025

View reviewed changes

qianheng-aws approved these changes Jul 22, 2025

View reviewed changes

LantaoJin approved these changes Jul 22, 2025

View reviewed changes

LantaoJin merged commit 0a07684 into opensearch-project:main Jul 22, 2025
23 checks passed

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

Filter script pushdown with RelJson serialization in Calcite #3859

Filter script pushdown with RelJson serialization in Calcite #3859

Uh oh!

Conversation

songkant-aws commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

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

Uh oh!

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!

songkant-aws commented Jul 9, 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!

songkant-aws Jul 10, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LantaoJin commented Jul 18, 2025

Uh oh!

qianheng-aws commented Jul 21, 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!

qianheng-aws Jul 22, 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!

qianheng-aws Jul 22, 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 22, 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 22, 2025

Uh oh!

LantaoJin commented Jul 22, 2025

Uh oh!

Uh oh!

songkant-aws commented Jul 9, 2025 •

edited

Loading

LantaoJin commented Jul 9, 2025 •

edited

Loading

songkant-aws Jul 10, 2025 •

edited

Loading

qianheng-aws Jul 22, 2025 •

edited

Loading

qianheng-aws Jul 22, 2025 •

edited

Loading

LantaoJin Jul 22, 2025 •

edited

Loading