Repository: GoogleCloudPlatform/pgadapter
Pull request #616 (2023-01-26 10:29:06)

fix: copy to for a query would fail with a column list
`copy (select a, b from foo) to stdout` would fail with a 'Missing closing parentheses' error, as the parsing of the query expression would stop at the first comma.

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/616?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> :exclamation: No coverage uploaded for pull request base (`postgresql-dialect@c576124`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#section-missing-base-commit).
> The diff coverage is `100.00%`.

```diff
@@                  Coverage Diff                  @@
##             postgresql-dialect     #616   +/-   ##
=====================================================
  Coverage                      ?   88.83%           
  Complexity                    ?     2151           
=====================================================
  Files                         ?      124           
  Lines                         ?     7123           
  Branches                      ?     1019           
=====================================================
  Hits                          ?     6328           
  Misses                        ?      558           
  Partials                      ?      237           
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `88.83% <100.00%> (?)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/616?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...ud/spanner/pgadapter/statements/CopyStatement.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/616?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvQ29weVN0YXRlbWVudC5qYXZh) | `94.84% <100.00%> (ø)` | |
| [...oud/spanner/pgadapter/statements/SimpleParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/616?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3N0YXRlbWVudHMvU2ltcGxlUGFyc2VyLmphdmE=) | `92.30% <100.00%> (ø)` | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform)


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyStatement.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyStatement.java
index fe950b4e..e615f107 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyStatement.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/CopyStatement.java
@@ -563,7 +563,7 @@ public class CopyStatement extends IntermediatePortalStatement {
     }
     ParsedCopyStatement.Builder builder = new ParsedCopyStatement.Builder();
     if (parser.eatToken("(")) {
-      builder.query = parser.parseExpressionUntilKeyword(ImmutableList.of(), true, true);
+      builder.query = parser.parseExpressionUntilKeyword(ImmutableList.of(), true, true, false);
       if (!parser.eatToken(")")) {
         throw PGExceptionFactory.newPGException(
             "missing closing parentheses after query", SQLState.SyntaxError);
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java
index ccbd8eb9..8918b341 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/statements/SimpleParser.java
@@ -354,6 +354,15 @@ public class SimpleParser {
       ImmutableList<String> keywords,
       boolean sameParensLevelAsStart,
       boolean stopAtEndOfExpression) {
+    return parseExpressionUntilKeyword(
+        keywords, sameParensLevelAsStart, stopAtEndOfExpression, true);
+  }
+
+  String parseExpressionUntilKeyword(
+      ImmutableList<String> keywords,
+      boolean sameParensLevelAsStart,
+      boolean stopAtEndOfExpression,
+      boolean stopAtComma) {
     skipWhitespaces();
     int start = pos;
     boolean valid;
@@ -366,7 +375,7 @@ public class SimpleParser {
         if (stopAtEndOfExpression && parens < 0) {
           break;
         }
-      } else if (stopAtEndOfExpression && parens == 0 && sql.charAt(pos) == ',') {
+      } else if (stopAtEndOfExpression && parens == 0 && stopAtComma && sql.charAt(pos) == ',') {
         break;
       }
       if ((!sameParensLevelAsStart || parens == 0)


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatementTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatementTest.java
index 3c32d859..f118d104 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatementTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/statements/CopyToStatementTest.java
@@ -15,6 +15,7 @@
 package com.google.cloud.spanner.pgadapter.statements;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.mockito.Mockito.when;
 
 import com.google.cloud.spanner.pgadapter.ConnectionHandler;
@@ -91,4 +92,14 @@ public class CopyToStatementTest {
         new CopyToStatement(connectionHandler, options, "", parsedCopyStatement);
     assertEquals("\n", statement.getCsvFormat().getRecordSeparator());
   }
+
+  @Test
+  public void testCopyQuery() {
+    String query =
+        "select executed_at, workload, threads, batch_size, operation_count, round(read_avg/1000) as read_avg, round(read_p95/1000) as read_p95 from run where true and workload='a' and threads=20 and deployment='java_uds' order by executed_at desc limit 100";
+    ParsedCopyStatement parsedCopyStatement =
+        CopyStatement.parse(String.format("copy (%s) to stdout\n", query));
+    assertNotNull(parsedCopyStatement);
+    assertEquals(parsedCopyStatement.query, query);
+  }
 }