Fix pagination for many columns (#2440)

[andreaskulicke]

Description

Fixes deserialization of OpenSeachIndexScan.

Issues Resolved

[BUG] no such index[]: Pagination fails for many columns

Check List

• [x] New functionality includes testing.
  • [x] 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
• [x] 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.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 95.38%. Comparing base (0d8341f) to head (df6da01).

:exclamation: Current head df6da01 differs from pull request most recent head 0a4e539. Consider uploading reports for the commit 0a4e539 to get more accurate results

Files	Patch %	Lines
...l/opensearch/storage/scan/OpenSearchIndexScan.java	75.00%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2441      +/-   ##
============================================
+ Coverage     95.36%   95.38%   +0.01%     
+ Complexity     5131     4991     -140     
============================================
  Files           490      478      -12     
  Lines         14426    13915     -511     
  Branches        969      934      -35     
============================================
- Hits          13758    13273     -485     
+ Misses          643      619      -24     
+ Partials         25       23       -2     

Flag	Coverage Δ
sql-engine	95.38% <75.00%> (+0.01%)	:arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[andreaskulicke]

Hi @MaxKsyunz, sorry for pulling you in explicitly, but do you mind having a look at these changes? I fixed the issues reported above. Thanks!

[andreaskulicke]

@penghuo, @dai-chen, @MaxKsyunz: sorry for bothering you, but do you mind having a look at the pull request? We are planning to use opentelemetry, dataprepper, opensearch, PowerBI to visualize logging data. On the way the default column names that get created are quite long so that after having ~30 columns (i.e. log attributes) we run into the problem described in #2440. So this is currently a blocker for us and instead of keeping on patching versions on our own it would be nice if the fix could be included upstream. Thanks!

[andreaskulicke]

@dai-chen very welcome and thanks for reviewing! Any idea how to get another maintainer for a review? Did I miss something?

[andreaskulicke]

@dai-chen, @acarbonetto: I fixed the import (review comment), rebased and pushed. It seems that another review is required. Would you mind having a look again? Thanks!

[andreaskulicke]

Any update here?

[andreaskulicke]

@dai-chen, @acarbonetto: Would you mind having a look again? There was the second OpenSearch release by now. It would really be nice to include this bug fix upstream. Thanks!

[dai-chen]

@andreaskulicke Thanks for the fix! The PR is already approved.

Because this is long pending, could you merge latest main branch to make sure all UT/IT can pass? Thanks!

[andreaskulicke]

Sure, thanks for the review! I merged the latest main into the branch, ran .\gradlew test and no test failed. Then I ran ./gradlew :integ-test:integTest and several tests failed. The test failures are unrelated to my changes and look like a setup problem on my machine (running on German Windows, just cloned sql repo, ...?), e.g.:

Tests with failures:
 - org.opensearch.sql.ppl.DateTimeFunctionIT.testMakeTime
 - org.opensearch.sql.ppl.DateTimeFunctionIT.testDayName
 - org.opensearch.sql.sql.DateTimeFunctionIT.testDayName
 - org.opensearch.sql.sql.DateTimeFunctionIT.testMakeTime
 - org.opensearch.sql.sql.SQLCorrectnessIT.runAllTests

1827 tests completed, 5 failed, 56 skipped

Example output:

org.opensearch.sql.ppl.DateTimeFunctionIT > testMakeTime FAILED
    org.opensearch.client.ResponseException: method [POST], host [http://127.0.0.1:52491], URI [/_plugins/_ppl], status line [HTTP/1.1 500 Internal Server Error]
    {
      "error": {
        "reason": "There was internal problem at backend",
        "details": "Text '20:30:40,000000000' could not be parsed, unparsed text found at index 8",
        "type": "DateTimeParseException"
      },
      "status": 500
    }
        at __randomizedtesting.SeedInfo.seed([65AED68CBB4AACD4:7A6828BF2C8BAE82]:0)
        at app//org.opensearch.client.RestClient.convertResponse(RestClient.java:385)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:355)
        at app//org.opensearch.client.RestClient.performRequest(RestClient.java:330)
        at app//org.opensearch.sql.ppl.PPLIntegTestCase.executeQueryToString(PPLIntegTestCase.java:30)
        at app//org.opensearch.sql.ppl.PPLIntegTestCase.executeQuery(PPLIntegTestCase.java:26)
        at app//org.opensearch.sql.ppl.DateTimeFunctionIT.testMakeTime(DateTimeFunctionIT.java:1238)

  2> java.lang.AssertionError:
    Expected: iterable with items [[Wednesday]] in any order
         but: not matched: <["Mittwoch"]>

or:

...
 ERROR][o.o.s.l.p.RestSqlAction  ] [integTest-0] 70bcb58b-b5a1-48fc-b821-4c1315c202a1 Client side error during query execution
  org.opensearch.sql.exception.SemanticCheckException: timestamp:17:30:00 in unsupported format, please use 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'

I also got some unrelated modifications in g4-files:

        modified:   spark/src/main/antlr/FlintSparkSqlExtensions.g4
        modified:   spark/src/main/antlr/SparkSqlBase.g4
        modified:   spark/src/main/antlr/SqlBaseLexer.g4
        modified:   spark/src/main/antlr/SqlBaseParser.g4

In detail:

diff --git a/spark/src/main/antlr/SqlBaseLexer.g4 b/spark/src/main/antlr/SqlBaseLexer.g4
index e2b178d34..83e40c4a2 100644
--- a/spark/src/main/antlr/SqlBaseLexer.g4
+++ b/spark/src/main/antlr/SqlBaseLexer.g4
@@ -182,6 +182,7 @@ ELSE: 'ELSE';
 END: 'END';
 ESCAPE: 'ESCAPE';
 ESCAPED: 'ESCAPED';
+EVOLUTION: 'EVOLUTION';
 EXCEPT: 'EXCEPT';
 EXCHANGE: 'EXCHANGE';
 EXCLUDE: 'EXCLUDE';
diff --git a/spark/src/main/antlr/SqlBaseParser.g4 b/spark/src/main/antlr/SqlBaseParser.g4
index 3d0085165..60b67b080 100644
--- a/spark/src/main/antlr/SqlBaseParser.g4
+++ b/spark/src/main/antlr/SqlBaseParser.g4
@@ -480,7 +480,7 @@ dmlStatementNoWith
     | fromClause multiInsertQueryBody+                                             #multiInsertQuery
     | DELETE FROM identifierReference tableAlias whereClause?                      #deleteFromTable
     | UPDATE identifierReference tableAlias setClause whereClause?                 #updateTable
-    | MERGE INTO target=identifierReference targetAlias=tableAlias
+    | MERGE (WITH SCHEMA EVOLUTION)? INTO target=identifierReference targetAlias=tableAlias
         USING (source=identifierReference |
           LEFT_PAREN sourceQuery=query RIGHT_PAREN) sourceAlias=tableAlias
         ON mergeCondition=booleanExpression
@@ -1399,6 +1399,7 @@ ansiNonReserved
     | DOUBLE
     | DROP
     | ESCAPED
+    | EVOLUTION
     | EXCHANGE
     | EXCLUDE
     | EXISTS
@@ -1715,6 +1716,7 @@ nonReserved
     | END
     | ESCAPE
     | ESCAPED
+    | EVOLUTION
     | EXCHANGE
     | EXCLUDE
     | EXECUTE

Shall I investigate further and try to get the integ-tests running successfully?

[dai-chen]

@andreaskulicke No problem! Will wait for CI complete and merge if failure unrelated. Thank you!

[dai-chen]

CI is failing on BWC test. Same as main branch.