Add position() to V2 engine #177

margarit-h · 2022-11-24T03:39:16Z

Signed-off-by: Margarit Hakobyan [email protected]

Description

Usage: The syntax POSITION(substr IN str) returns the position of the first occurrence of substring substr in string str. Returns 0 if substr is not in str. Returns NULL if any argument is NULL.

Argument type: STRING, STRING, INTEGER

Return type integer:

(STRING IN STRING) -> INTEGER

Example::

os> SELECT POSITION('world' IN 'helloworld')
fetched rows / total rows = 1/1
+-------------------------------------+
| POSITION('world' IN 'helloworld')   |
|-------------------------------------|
| 6                                   |
+-------------------------------------+

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

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: Margarit Hakobyan <[email protected]>

codecov · 2022-11-24T03:50:32Z

Codecov Report

Merging #177 (9a6b7b5) into integ-add-position-to-v2 (cfb2650) will decrease coverage by 2.50%.
The diff coverage is 100.00%.

@@                      Coverage Diff                       @@
##             integ-add-position-to-v2     #177      +/-   ##
==============================================================
- Coverage                       98.29%   95.79%   -2.51%     
- Complexity                       3465     3470       +5     
==============================================================
  Files                             347      357      +10     
  Lines                            8645     9314     +669     
  Branches                          550      669     +119     
==============================================================
+ Hits                             8498     8922     +424     
- Misses                            142      334     +192     
- Partials                            5       58      +53

Flag	Coverage Δ
query-workbench	`62.76% <ø> (?)`
sql-engine	`98.30% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java	`100.00% <100.00%> (ø)`
...c/main/java/org/opensearch/sql/expression/DSL.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/BuiltinFunctionName.java	`100.00% <100.00%> (ø)`
...g/opensearch/sql/expression/text/TextFunction.java	`100.00% <100.00%> (ø)`
...pensearch/sql/sql/parser/AstExpressionBuilder.java	`100.00% <100.00%> (ø)`
workbench/public/application.tsx	`0.00% <0.00%> (ø)`
workbench/public/utils/PanelWrapper.tsx	`100.00% <0.00%> (ø)`
workbench/public/components/PPLPage/PPLPage.tsx	`56.52% <0.00%> (ø)`
workbench/public/components/app.tsx	`0.00% <0.00%> (ø)`
...h/public/components/QueryLanguageSwitch/Switch.tsx	`85.71% <0.00%> (ø)`
... and 5 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java

Yury-Fridlyand · 2022-11-24T20:47:04Z

core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java

@@ -241,6 +242,15 @@ private DefaultFunctionResolver locate() {
                TextFunction::exprLocate), INTEGER, STRING, STRING, INTEGER));
  }

+  /**
+   * POSITION(substr IN str) returns the position of the first occurrence of a substring in a string.
+   * If the substring is not found within the original string, this function returns 0.


Maybe: reference LOCATE function and signature (see other functions' docs).

sql/src/main/antlr/OpenSearchSQLParser.g4

Signed-off-by: Margarit Hakobyan <[email protected]>

integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java

Signed-off-by: Margarit Hakobyan <[email protected]>

core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java

acarbonetto · 2022-11-28T22:09:49Z

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

@@ -288,6 +289,10 @@ public UnresolvedExpression highlight(UnresolvedExpression fieldName,
    return new HighlightFunction(fieldName, arguments);
  }

+  public UnresolvedExpression position(UnresolvedExpression left, UnresolvedExpression right) {
+    return new PositionFunction(left, right);


Same comment as above: can we used a better description than left and right?

Addressed in 9a6b7b5

core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java

acarbonetto · 2022-11-28T22:14:45Z

core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java

+@ToString
+public class PositionFunction extends UnresolvedExpression {
+  @Getter
+  private UnresolvedExpression left;


Same comment as above:
Can we used a better description than left and right?

Addressed in 9a6b7b5

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

core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java

acarbonetto · 2022-11-28T22:22:10Z

core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java

+  void position() {
+    FunctionExpression expression = DSL.position(
+        DSL.literal("world"),
+        DSL.literal("helloworld"));


can you change this to "helloworldworld"? Then it will also cover the duplicate case.

docs/user/dql/functions.rst

integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java

sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java

forestmvey · 2022-11-28T23:17:44Z

core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java

+    assertEquals(INTEGER, expression.type());
+    assertEquals(6, eval(expression).integerValue());
+
+    when(nullRef.type()).thenReturn(STRING);


Can we split these out into null and missing tests.

Signed-off-by: Margarit Hakobyan <[email protected]>

core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java

Signed-off-by: Margarit Hakobyan <[email protected]>

Signed-off-by: Margarit Hakobyan <[email protected]> Rebased

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased

This reverts commit eee57aa.

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased

This reverts commit eee57aa.

Signed-off-by: Margarit Hakobyan <[email protected]> Rebased Signed-off-by: Margarit Hakobyan <[email protected]>

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased Signed-off-by: Margarit Hakobyan <[email protected]>

This reverts commit eee57aa. Signed-off-by: Margarit Hakobyan <[email protected]>

Optional is not required since api returns list instead of object. Signed-off-by: Vijayan Balasubramanian <[email protected]>

Add position() to V2 engine

441b997

Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h requested a review from a team November 24, 2022 03:40

Yury-Fridlyand reviewed Nov 24, 2022

View reviewed changes

margarit-h added 3 commits November 24, 2022 15:04

Add integration tests

8025fdb

Signed-off-by: Margarit Hakobyan <[email protected]>

Address PR review comment

0950e7d

Signed-off-by: Margarit Hakobyan <[email protected]>

Address another PR review comment

8609e8b

Signed-off-by: Margarit Hakobyan <[email protected]>

Yury-Fridlyand approved these changes Nov 25, 2022

View reviewed changes

Add a unit test

e254462

Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h mentioned this pull request Nov 25, 2022

[FEATURE] Support for position in the v2 engine opensearch-project/sql#1106

Closed

Add unit tests and docs

07637e9

Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h changed the title ~~[WIP:] Add position() to V2 engine~~ Add position() to V2 engine Nov 25, 2022

GumpacG reviewed Nov 28, 2022

View reviewed changes

integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java Show resolved Hide resolved

GumpacG approved these changes Nov 28, 2022

View reviewed changes

Added a test case

c8cd1bb

Signed-off-by: Margarit Hakobyan <[email protected]>

acarbonetto requested changes Nov 28, 2022

View reviewed changes

forestmvey reviewed Nov 28, 2022

View reviewed changes

forestmvey approved these changes Nov 28, 2022

View reviewed changes

Address PR review comments

9a6b7b5

Signed-off-by: Margarit Hakobyan <[email protected]>

acarbonetto approved these changes Nov 30, 2022

View reviewed changes

core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java Show resolved Hide resolved

margarit-h merged commit 2964853 into integ-add-position-to-v2 Nov 30, 2022

margarit-h deleted the dev-add-position-to-v2 branch November 30, 2022 19:07

margarit-h added a commit that referenced this pull request Nov 30, 2022

Add position() to V2 engine (#177)

1e61486

Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h added a commit that referenced this pull request Dec 5, 2022

Add position() to V2 engine (#177)

57ae38b

Signed-off-by: Margarit Hakobyan <[email protected]> Rebased

margarit-h added a commit that referenced this pull request Dec 5, 2022

Add position() to V2 engine (#177)

fc1bccf

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased

margarit-h added a commit that referenced this pull request Dec 5, 2022

Add position() to V2 engine (#177)

eee57aa

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased

margarit-h added a commit that referenced this pull request Dec 6, 2022

Revert "Add position() to V2 engine (#177)"

b0d0014

This reverts commit eee57aa.

margarit-h added a commit that referenced this pull request Dec 6, 2022

Add position() to V2 engine (#177)

93e4b9f

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased

margarit-h added a commit that referenced this pull request Dec 6, 2022

Revert "Add position() to V2 engine (#177)"

c276efc

This reverts commit eee57aa.

margarit-h added a commit that referenced this pull request Dec 6, 2022

Add position() to V2 engine (#177)

8609f96

Signed-off-by: Margarit Hakobyan <[email protected]> Rebased Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h added a commit that referenced this pull request Dec 6, 2022

Add position() to V2 engine (#177)

cab64ae

Signed-off-by: Margarit Hakobyan <[email protected]> Rbased Signed-off-by: Margarit Hakobyan <[email protected]>

margarit-h added a commit that referenced this pull request Dec 6, 2022

Revert "Add position() to V2 engine (#177)"

35ac23b

This reverts commit eee57aa. Signed-off-by: Margarit Hakobyan <[email protected]>

andy-k-improving pushed a commit that referenced this pull request Nov 16, 2024

Remove optional to get features (#177)

4465452

Optional is not required since api returns list instead of object. Signed-off-by: Vijayan Balasubramanian <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add position() to V2 engine #177

Add position() to V2 engine #177

margarit-h commented Nov 24, 2022 •

edited

Loading

codecov bot commented Nov 24, 2022 •

edited

Loading

Yury-Fridlyand Nov 24, 2022

acarbonetto Nov 28, 2022

margarit-h Nov 29, 2022

acarbonetto Nov 28, 2022

margarit-h Nov 29, 2022

acarbonetto Nov 28, 2022

forestmvey Nov 28, 2022

Add position() to V2 engine #177

Add position() to V2 engine #177

Conversation

margarit-h commented Nov 24, 2022 • edited Loading

Description

Issues Resolved

Check List

codecov bot commented Nov 24, 2022 • edited Loading

Codecov Report

Yury-Fridlyand Nov 24, 2022

Choose a reason for hiding this comment

acarbonetto Nov 28, 2022

Choose a reason for hiding this comment

margarit-h Nov 29, 2022

Choose a reason for hiding this comment

acarbonetto Nov 28, 2022

Choose a reason for hiding this comment

margarit-h Nov 29, 2022

Choose a reason for hiding this comment

acarbonetto Nov 28, 2022

Choose a reason for hiding this comment

forestmvey Nov 28, 2022

Choose a reason for hiding this comment

margarit-h commented Nov 24, 2022 •

edited

Loading

codecov bot commented Nov 24, 2022 •

edited

Loading