Repository: GoogleCloudPlatform/pgadapter
Pull request #706 (2023-03-03 11:39:55)

fix: allow unquoted text values in arrays
PostgreSQL allows (text) values in array literals to be unquoted. Quotes are only required if the literal contains commas.

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#706](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (5ac44aa) into [postgresql-dialect](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/6ef7240017e7fee9786132c92ac32b3aecc6f63c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (6ef7240) will **increase** coverage by `3.01%`.
> The diff coverage is `96.08%`.

```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect     #706      +/-   ##
========================================================
+ Coverage                 86.32%   89.34%   +3.01%     
- Complexity                 1914     2301     +387     
========================================================
  Files                       122      126       +4     
  Lines                      6216     7583    +1367     
  Branches                    849     1089     +240     
========================================================
+ Hits                       5366     6775    +1409     
+ Misses                      605      560      -45     
- Partials                    245      248       +3     
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `89.34% <96.08%> (+3.01%)` | :arrow_up: |

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/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...om/google/cloud/spanner/pgadapter/ProxyServer.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL1Byb3h5U2VydmVyLmphdmE=) | `81.39% <ø> (+0.58%)` | :arrow_up: |
| [...e/cloud/spanner/pgadapter/utils/PgJdbcCatalog.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3V0aWxzL1BnSmRiY0NhdGFsb2cuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...spanner/pgadapter/wireprotocol/ControlMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9Db250cm9sTWVzc2FnZS5qYXZh) | `89.80% <ø> (+3.26%)` | :arrow_up: |
| [...panner/pgadapter/wireprotocol/CopyDataMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9Db3B5RGF0YU1lc3NhZ2UuamF2YQ==) | `85.00% <ø> (ø)` | |
| [...panner/pgadapter/wireprotocol/CopyFailMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9Db3B5RmFpbE1lc3NhZ2UuamF2YQ==) | `75.00% <ø> (ø)` | |
| [...panner/pgadapter/wireprotocol/DescribeMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9EZXNjcmliZU1lc3NhZ2UuamF2YQ==) | `85.50% <ø> (-0.61%)` | :arrow_down: |
| [...d/spanner/pgadapter/wireprotocol/ParseMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9QYXJzZU1lc3NhZ2UuamF2YQ==) | `92.06% <ø> (+6.34%)` | :arrow_up: |
| [...panner/pgadapter/wireprotocol/PasswordMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9QYXNzd29yZE1lc3NhZ2UuamF2YQ==) | `67.34% <ø> (ø)` | |
| [...d/spanner/pgadapter/wireprotocol/QueryMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9RdWVyeU1lc3NhZ2UuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...spanner/pgadapter/wireprotocol/StartupMessage.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3dpcmVwcm90b2NvbC9TdGFydHVwTWVzc2FnZS5qYXZh) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| ... and [93 more](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/706?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | |

: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/parsers/ArrayParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParser.java
index 404b180a..460c68cb 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParser.java
@@ -89,11 +89,7 @@ public class ArrayParser extends Parser<List<?>> {
         case TEXT:
           this.item =
               stringArrayToList(
-                  new String(item, StandardCharsets.UTF_8),
-                  elementOid,
-                  this.isStringEquivalent,
-                  this.sessionState,
-                  false);
+                  new String(item, StandardCharsets.UTF_8), elementOid, this.sessionState, false);
           break;
         case BINARY:
           this.item = binaryArrayToList(item, false);
@@ -138,14 +134,12 @@ public class ArrayParser extends Parser<List<?>> {
   public static List<?> stringArrayToList(
       @Nullable String value,
       int elementOid,
-      boolean isStringEquivalent,
       SessionState sessionState,
       boolean convertToValidSpannerElements) {
     if (value == null) {
       return null;
     }
-    List<String> values =
-        SimpleParser.readArrayLiteral(value, isStringEquivalent, elementOid == Oid.BYTEA);
+    List<String> values = SimpleParser.readArrayLiteral(value, elementOid == Oid.BYTEA);
     ArrayList<Object> result = new ArrayList<>(values.size());
     for (String element : values) {
       if (element == null) {
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 506aa827..1171e968 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
@@ -275,8 +275,7 @@ public class SimpleParser {
     return new SimpleParser(query).peekKeyword(command);
   }
 
-  public static List<String> readArrayLiteral(
-      String expression, boolean mustBeQuoted, boolean returnRawHexValue) {
+  public static List<String> readArrayLiteral(String expression, boolean returnRawHexValue) {
     List<String> result = new ArrayList<>();
     SimpleParser parser = new SimpleParser(expression);
     if (!parser.eatToken("{")) {
@@ -288,7 +287,7 @@ public class SimpleParser {
         break;
       } else if (parser.eatKeyword("null")) {
         result.add(null);
-      } else if (mustBeQuoted || parser.peekToken("\"")) {
+      } else if (parser.peekToken("\"")) {
         QuotedString quotedString = parser.readQuotedString('"', true);
         if (quotedString == null) {
           throw PGExceptionFactory.newPGException(
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/utils/CsvCopyParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/utils/CsvCopyParser.java
index 5d83144b..9d307c20 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/utils/CsvCopyParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/utils/CsvCopyParser.java
@@ -170,49 +170,39 @@ class CsvCopyParser implements CopyInParser {
             switch (type.getArrayElementType().getCode()) {
               case STRING:
                 return Value.stringArray(
-                    cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.TEXT, false, sessionState, true)));
+                    cast(ArrayParser.stringArrayToList(recordValue, Oid.TEXT, sessionState, true)));
               case PG_JSONB:
                 return Value.pgJsonbArray(
                     cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.JSONB, false, sessionState, true)));
+                        ArrayParser.stringArrayToList(recordValue, Oid.JSONB, sessionState, true)));
               case BOOL:
                 return Value.boolArray(
-                    cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.BOOL, false, sessionState, true)));
+                    cast(ArrayParser.stringArrayToList(recordValue, Oid.BOOL, sessionState, true)));
               case INT64:
                 return Value.int64Array(
-                    cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.INT8, false, sessionState, true)));
+                    cast(ArrayParser.stringArrayToList(recordValue, Oid.INT8, sessionState, true)));
               case FLOAT64:
                 return Value.float64Array(
                     cast(
                         ArrayParser.stringArrayToList(
-                            recordValue, Oid.FLOAT8, false, sessionState, true)));
+                            recordValue, Oid.FLOAT8, sessionState, true)));
               case PG_NUMERIC:
                 return Value.pgNumericArray(
                     cast(
                         ArrayParser.stringArrayToList(
-                            recordValue, Oid.NUMERIC, false, sessionState, true)));
+                            recordValue, Oid.NUMERIC, sessionState, true)));
               case BYTES:
                 return Value.bytesArray(
                     cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.BYTEA, false, sessionState, true)));
+                        ArrayParser.stringArrayToList(recordValue, Oid.BYTEA, sessionState, true)));
               case DATE:
                 return Value.dateArray(
-                    cast(
-                        ArrayParser.stringArrayToList(
-                            recordValue, Oid.DATE, false, sessionState, true)));
+                    cast(ArrayParser.stringArrayToList(recordValue, Oid.DATE, sessionState, true)));
               case TIMESTAMP:
                 return Value.timestampArray(
                     cast(
                         ArrayParser.stringArrayToList(
-                            recordValue, Oid.TIMESTAMPTZ, false, sessionState, true)));
+                            recordValue, Oid.TIMESTAMPTZ, sessionState, true)));
             }
           default:
             SpannerException spannerException =


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParserTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParserTest.java
index 8d6ef1dc..f858fba8 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParserTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/ArrayParserTest.java
@@ -189,6 +189,19 @@ public class ArrayParserTest {
                 Type.timestamp(),
                 Oid.TIMESTAMPTZ)
             .item);
+    assertEquals(
+        Arrays.asList(
+            Timestamp.parseTimestamp("2023-02-13T19:15:00.123456Z"),
+            null,
+            Timestamp.parseTimestamp("2000-01-01T00:00:00Z")),
+        new ArrayParser(
+                "{2023-02-13T19:15:00.123456+00:00,null,2000-01-01T00:00:00+00}"
+                    .getBytes(StandardCharsets.UTF_8),
+                FormatCode.TEXT,
+                mock(SessionState.class),
+                Type.timestamp(),
+                Oid.TIMESTAMPTZ)
+            .item);
   }
 
   @Test
@@ -202,6 +215,15 @@ public class ArrayParserTest {
                 Type.date(),
                 Oid.DATE)
             .item);
+    assertEquals(
+        Arrays.asList(Date.parseDate("2023-02-13"), null, Date.parseDate("2000-01-01")),
+        new ArrayParser(
+                "{2023-02-13,null,2000-01-01}".getBytes(StandardCharsets.UTF_8),
+                FormatCode.TEXT,
+                mock(SessionState.class),
+                Type.date(),
+                Oid.DATE)
+            .item);
   }
 
   @Test
@@ -451,16 +473,13 @@ public class ArrayParserTest {
   @Test
   public void testNullTextArray() {
     assertNull(
-        ArrayParser.stringArrayToList(
-            null, Oid.UNSPECIFIED, false, mock(SessionState.class), false));
+        ArrayParser.stringArrayToList(null, Oid.UNSPECIFIED, mock(SessionState.class), false));
     assertNull(
-        ArrayParser.stringArrayToList(
-            null, Oid.UNSPECIFIED, false, mock(SessionState.class), true));
+        ArrayParser.stringArrayToList(null, Oid.UNSPECIFIED, mock(SessionState.class), true));
     assertNull(
-        ArrayParser.stringArrayToList(
-            null, Oid.UNSPECIFIED, true, mock(SessionState.class), false));
+        ArrayParser.stringArrayToList(null, Oid.UNSPECIFIED, mock(SessionState.class), false));
     assertNull(
-        ArrayParser.stringArrayToList(null, Oid.UNSPECIFIED, true, mock(SessionState.class), true));
+        ArrayParser.stringArrayToList(null, Oid.UNSPECIFIED, mock(SessionState.class), true));
   }
 
   static ResultSet createArrayResultSet(Type arrayElementType, Value value) {
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/statements/SimpleParserTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/statements/SimpleParserTest.java
index 70e0f3d3..e602a815 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/statements/SimpleParserTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/statements/SimpleParserTest.java
@@ -475,28 +475,27 @@ public class SimpleParserTest {
 
   @Test
   public void testReadArrayLiteral() {
-    assertEquals(ImmutableList.of(), SimpleParser.readArrayLiteral("{}", true, true));
-    assertEquals(ImmutableList.of("foo"), SimpleParser.readArrayLiteral("{\"foo\"}", true, true));
+    assertEquals(ImmutableList.of(), SimpleParser.readArrayLiteral("{}", true));
+    assertEquals(ImmutableList.of("foo"), SimpleParser.readArrayLiteral("{\"foo\"}", true));
     assertEquals(
-        ImmutableList.of("foo", "bar"),
-        SimpleParser.readArrayLiteral("{\"foo\", \"bar\"}", true, true));
+        ImmutableList.of("foo", "bar"), SimpleParser.readArrayLiteral("{\"foo\", \"bar\"}", true));
     assertEquals(
         Arrays.asList("foo", "bar", null),
-        SimpleParser.readArrayLiteral("{\"foo\", \"bar\", null}", true, true));
-    assertEquals(ImmutableList.of("1", "2"), SimpleParser.readArrayLiteral("{1, 2}", false, true));
-    assertEquals(
-        ImmutableList.of("1", "2"), SimpleParser.readArrayLiteral("{\"1\", \"2\"}", false, true));
+        SimpleParser.readArrayLiteral("{\"foo\", \"bar\", null}", true));
+    assertEquals(ImmutableList.of("1", "2"), SimpleParser.readArrayLiteral("{1, 2}", true));
+    assertEquals(ImmutableList.of("1", "2"), SimpleParser.readArrayLiteral("{\"1\", \"2\"}", true));
     assertEquals(
         ImmutableList.of("{\"foo\": \"bar\"}"),
-        SimpleParser.readArrayLiteral("{\"{\\\"foo\\\": \\\"bar\\\"}\"}", true, true));
+        SimpleParser.readArrayLiteral("{\"{\\\"foo\\\": \\\"bar\\\"}\"}", true));
 
-    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("1, 2", false, true));
-    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("{1, 2", false, true));
-    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("1, 2}", false, true));
-    assertThrows(
-        PGException.class, () -> SimpleParser.readArrayLiteral("{1, 2} extra token", false, true));
+    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("1, 2", true));
+    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("{1, 2", true));
+    assertThrows(PGException.class, () -> SimpleParser.readArrayLiteral("1, 2}", true));
     assertThrows(
-        PGException.class,
-        () -> SimpleParser.readArrayLiteral("{foo, bar}", /* mustBeQuoted = */ true, true));
+        PGException.class, () -> SimpleParser.readArrayLiteral("{1, 2} extra token", true));
+
+    assertEquals(ImmutableList.of("foo", "bar"), SimpleParser.readArrayLiteral("{foo, bar}", true));
+    assertEquals(
+        ImmutableList.of("foo 1", "bar 2"), SimpleParser.readArrayLiteral("{foo 1, bar 2}", true));
   }
 }