Revert stream pattern method in V2 and implement SIMPLE_PATTERN in Calcite by songkant-aws · Pull Request #3553 · opensearch-project/sql

songkant-aws · 2025-04-16T03:37:12Z

Description

Revert stream pattern method in V2 and implement SIMPLE_PATTERN in Calcite

Related Issues

Resolves #3552

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: Songkan Tang <songkant@amazon.com>

qianheng-aws · 2025-04-16T08:46:26Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java

+                .toString()
+                .toLowerCase(Locale.ROOT);
+    if (patternMethod.equalsIgnoreCase(PatternMethod.SIMPLE_PATTERN.getName())) {
+      return new Parse(ParseMethod.PATTERNS, sourceField, pattern, arguments);


Shall we rename the Parse node and change its doc? It's weird to use a node which says /** AST node represent Parse with regex operation. */ in its doc for PatternCommand.

Looks like our definition of UnresolvedPlan is something between AST and LogicalPlan. Most are more like AST(e.g. Parse, Rename, Eval), while others are more like LogicalPlan(e.g. Window, Aggregation, Project, Filter).

Under current convention, seems we cannot figure out a proper name for it or determine which group it belong to.

I would prefer not doing this refactoring in this PR to introduce minimum change. I left comment to call out why do we keep that Parse plan for legacy patterns method. And call out there is opportunity of refactoring it into just Project operator.

qianheng-aws · 2025-04-16T09:10:33Z

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

    java.util.Map<String, Literal> arguments = node.getArguments();
    String pattern = (String) node.getPattern().getValue();
+    // TODO: Support Grok parse method
+    Pair<SqlOperator, String> operatorPatternPair =


Could this be registered as functions in PPLFuncImpTable？Seems we will have more method in future and it should be a true function in Calcite.

Done. Registered related calcite REGEXP functions into PPLFuncImpTable

Signed-off-by: Songkan Tang <songkant@amazon.com>

LantaoJin · 2025-04-18T08:10:53Z

core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java

+  REGEXP_EXTRACT(FunctionName.of("regexp_extract")),
+  REGEXP_REPLACE_2(FunctionName.of("regexp_replace_2")),


Are these functions all existing UDF in V2?

As discussed, change them to internal functions with additional boolean to describe them.

LantaoJin · 2025-04-18T08:15:43Z

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLPatternsTest.java

+    RelNode root = getRelNode(ppl);
+
+    String expectedLogical =
+        "LogicalProject(ENAME=[$1], patterns_field=[REGEXP_REPLACE($1, '[A-H]')])\n"


I do think the '[A-H]' here should change to '[A-H]':VARCHAR if #3558 merged first Just call out for reminding.

Done. In visitParse, it's changed to makeLiteral(pattern, VARCHAR).

LantaoJin · 2025-04-18T08:21:53Z

core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java

    ParseMethod parseMethod = node.getParseMethod();
    java.util.Map<String, Literal> arguments = node.getArguments();
-    String pattern = (String) node.getPattern().getValue();
+    String patternValue = (String) node.getPattern().getValue();


patterns is a command, I didn't see the implementation code visitPatterns here? How PatternsIT works?

It will be changed in part II. Part I just reverts previous changes to V2. V2's patterns representation is Parse.

Signed-off-by: Songkan Tang <songkant@amazon.com>

qianheng-aws

Could you please add tests for explain on patterns in ExplainIT as well? May need 2 tests for cases w/wo stats command after patterns.

As discussed offline, it only supports pushdown for patterns with stats command after it. It would help us verify the feature of pushing down patterns on Calcite in the future.

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws · 2025-04-23T03:17:03Z

Could you please add tests for explain on patterns in ExplainIT as well? May need 2 tests for cases w/wo stats command after patterns.

As discussed offline, it only supports pushdown for patterns with stats command after it. It would help us verify the feature of pushing down patterns on Calcite in the future.

Sure, I added suggested two cases. But I feel that IT is a bit strange. It's better to assert on actual plan change. The pushed down script is encoded string that is not readable.

Signed-off-by: Songkan Tang <songkant@amazon.com>

LantaoJin · 2025-04-28T07:50:38Z

Please update visitWindow to visitPatterns in Analyzer.

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws · 2025-04-28T09:51:22Z

Please update visitWindow to visitPatterns in Analyzer.

Done. And fixed failed IT.

LantaoJin

Could you add testPatterns in PPLQueryDataAnonymizerTest

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws · 2025-04-29T05:50:49Z

Could you add testPatterns in PPLQueryDataAnonymizerTest

Added testPatterns for SIMPLE_PATTERN method. The BRAIN method will be added in another PR because it needs some signature change of unresolved plan.

…lcite (#3553) * Revert simple_pattern window function change to recover pushdown ability Signed-off-by: Songkan Tang <songkant@amazon.com> * Add SIMPLE_PATTERN patterns command support based on parse command Signed-off-by: Songkan Tang <songkant@amazon.com> * Address minor comments Signed-off-by: Songkan Tang <songkant@amazon.com> * Address comments part 2 Signed-off-by: Songkan Tang <songkant@amazon.com> * Make allowCast for pattern VARCHAR literal Signed-off-by: Songkan Tang <songkant@amazon.com> * Fix spotless Signed-off-by: Songkan Tang <songkant@amazon.com> * Minor ut failure fix Signed-off-by: Songkan Tang <songkant@amazon.com> * Add patterns command push down cases in ExplainIT Signed-off-by: Songkan Tang <songkant@amazon.com> * Fix patterns ExplainIT failure Signed-off-by: Songkan Tang <songkant@amazon.com> * Rename visitWindow to visitPatterns Signed-off-by: Songkan Tang <songkant@amazon.com> * Correct ExplainIT expected plan for patterns Signed-off-by: Songkan Tang <songkant@amazon.com> * Add patterns anonymizer test for SIMPLE_PATTERN Signed-off-by: Songkan Tang <songkant@amazon.com> --------- Signed-off-by: Songkan Tang <songkant@amazon.com> Signed-off-by: xinyual <xinyual@amazon.com>

songkant-aws added 2 commits April 16, 2025 11:31

Revert simple_pattern window function change to recover pushdown ability

c06d3c3

Signed-off-by: Songkan Tang <songkant@amazon.com>

Add SIMPLE_PATTERN patterns command support based on parse command

39d99e6

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws requested review from GumpacG, LantaoJin, 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 April 16, 2025 03:37

qianheng-aws reviewed Apr 16, 2025

View reviewed changes

LantaoJin added the calcite calcite migration releated label Apr 16, 2025

Address minor comments

81d0606

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws requested a review from qianheng-aws April 17, 2025 08:41

LantaoJin reviewed Apr 18, 2025

View reviewed changes

LantaoJin added calcite calcite migration releated and removed calcite calcite migration releated labels Apr 18, 2025

LantaoJin reviewed Apr 18, 2025

View reviewed changes

songkant-aws added 4 commits April 21, 2025 10:33

Address comments part 2

8438c8e

Signed-off-by: Songkan Tang <songkant@amazon.com>

Make allowCast for pattern VARCHAR literal

f8e8945

Signed-off-by: Songkan Tang <songkant@amazon.com>

Fix spotless

48dcf64

Signed-off-by: Songkan Tang <songkant@amazon.com>

Minor ut failure fix

828685c

Signed-off-by: Songkan Tang <songkant@amazon.com>

qianheng-aws reviewed Apr 22, 2025

View reviewed changes

Add patterns command push down cases in ExplainIT

1743d16

Signed-off-by: Songkan Tang <songkant@amazon.com>

Fix patterns ExplainIT failure

b0cf346

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws added 3 commits April 28, 2025 16:11

Merge branch 'main' into calcite-patterns-command-revert-stream-pattern

1f3b035

Rename visitWindow to visitPatterns

6e715ea

Signed-off-by: Songkan Tang <songkant@amazon.com>

Correct ExplainIT expected plan for patterns

e71f354

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws requested review from LantaoJin and qianheng-aws April 28, 2025 09:51

LantaoJin approved these changes Apr 29, 2025

View reviewed changes

LantaoJin requested changes Apr 29, 2025

View reviewed changes

Add patterns anonymizer test for SIMPLE_PATTERN

07a0f37

Signed-off-by: Songkan Tang <songkant@amazon.com>

LantaoJin mentioned this pull request Apr 29, 2025

Support eventstats command with Calcite #3585

Merged

7 tasks

LantaoJin approved these changes Apr 29, 2025

View reviewed changes

qianheng-aws approved these changes Apr 30, 2025

View reviewed changes

LantaoJin merged commit 788da98 into opensearch-project:main Apr 30, 2025
22 checks passed

songkant-aws deleted the calcite-patterns-command-revert-stream-pattern branch June 6, 2025 05:48

qianheng-aws mentioned this pull request Mar 18, 2026

Add songkant-aws as maintainer #5244

Merged

		REGEXP_EXTRACT(FunctionName.of("regexp_extract")),
		REGEXP_REPLACE_2(FunctionName.of("regexp_replace_2")),

Conversation

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

Uh oh!

Description

Related Issues

Check List

Uh oh!

qianheng-aws Apr 16, 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!

songkant-aws Apr 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!

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 left a comment • 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!

songkant-aws commented Apr 23, 2025

Uh oh!

LantaoJin commented Apr 28, 2025

Uh oh!

songkant-aws commented Apr 28, 2025

Uh oh!

LantaoJin left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

songkant-aws commented Apr 16, 2025 •

edited

Loading

qianheng-aws Apr 16, 2025 •

edited

Loading

songkant-aws Apr 17, 2025 •

edited

Loading

qianheng-aws left a comment •

edited

Loading

songkant-aws commented Apr 29, 2025 •

edited

Loading