Repository: GoogleCloudPlatform/pgadapter
Pull request #604 (2023-01-20 10:08:49)

feat: allow unsupported OIDs as param types
PGAdapter would refuse any parameter value that used an OID that was not currently known to PGAdapter. This meant that if an application used for example Oid.CHAR as the parameter type, PGAdapter would refuse it. This change allows those parameters and automatically encodes these as untyped parameter values using a string value when sending them to Cloud Spanner. This is not guaranteed to work for all types, but it will work for any type in Cloud Spanner that uses string encoding (meaning most of them).

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/604?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#604](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/604?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (b702b6f) into [postgresql-dialect](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/b0b7e0eab9de9c25d24d382d61297a898554a387?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (b0b7e0e) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.

```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect     #604      +/-   ##
========================================================
- Coverage                 88.69%   88.67%   -0.02%     
+ Complexity                 2136     2135       -1     
========================================================
  Files                       124      124              
  Lines                      7101     7100       -1     
  Branches                   1011     1011              
========================================================
- Hits                       6298     6296       -2     
  Misses                      567      567              
- Partials                    236      237       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `88.67% <100.00%> (-0.02%)` | :arrow_down: |

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/604?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...google/cloud/spanner/pgadapter/parsers/Parser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/604?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3BhcnNlcnMvUGFyc2VyLmphdmE=) | `88.00% <100.00%> (-0.12%)` | :arrow_down: |
| [...ava/com/google/cloud/spanner/pgadapter/Server.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/604?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL1NlcnZlci5qYXZh) | `58.06% <0.00%> (-3.23%)` | :arrow_down: |

: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/Parser.java b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/Parser.java
index 16141cec..4bbb1b67 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/Parser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/Parser.java
@@ -97,9 +97,12 @@ public abstract class Parser<T> {
       case Oid.JSONB:
         return new JsonbParser(item, formatCode);
       case Oid.UNSPECIFIED:
-        return new UnspecifiedParser(item, formatCode);
       default:
-        throw new IllegalArgumentException("Unsupported parameter type: " + oidType);
+        // Use the UnspecifiedParser for unknown types. This will encode the parameter value as a
+        // string and send it to Spanner without any type information. This will ensure that clients
+        // that for example send char instead of varchar as the type code for a parameter would
+        // still work.
+        return new UnspecifiedParser(item, formatCode);
     }
   }
 


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
index f8f0cab2..6405081a 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -93,6 +93,7 @@ import org.postgresql.PGConnection;
 import org.postgresql.PGStatement;
 import org.postgresql.core.Oid;
 import org.postgresql.jdbc.PgStatement;
+import org.postgresql.util.PGobject;
 import org.postgresql.util.PSQLException;
 
 @RunWith(Parameterized.class)
@@ -637,6 +638,41 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
     }
   }
 
+  @Test
+  public void testCharParam() throws SQLException {
+    String sql = "insert into foo values ($1)";
+    mockSpanner.putStatementResult(StatementResult.update(Statement.of(sql), 1L));
+    String jdbcSql = "insert into foo values (?)";
+    try (Connection connection = DriverManager.getConnection(createUrl())) {
+      try (PreparedStatement preparedStatement = connection.prepareStatement(jdbcSql)) {
+        PGobject pgObject = new PGobject();
+        pgObject.setType("char");
+        pgObject.setValue("a");
+        preparedStatement.setObject(1, pgObject);
+        assertEquals(1, preparedStatement.executeUpdate());
+      }
+    }
+
+    List<ParseMessage> parseMessages =
+        pgServer.getDebugMessages().stream()
+            .filter(message -> message instanceof ParseMessage)
+            .map(message -> (ParseMessage) message)
+            .collect(Collectors.toList());
+    assertFalse(parseMessages.isEmpty());
+    ParseMessage parseMessage = parseMessages.get(parseMessages.size() - 1);
+    assertEquals(1, parseMessage.getStatement().getGivenParameterDataTypes().length);
+    assertEquals(Oid.CHAR, parseMessage.getStatement().getGivenParameterDataTypes()[0]);
+
+    List<ExecuteSqlRequest> executeSqlRequests =
+        mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
+    assertEquals(1, executeSqlRequests.size());
+    ExecuteSqlRequest request = executeSqlRequests.get(0);
+    // Oid.CHAR is not a recognized type in PGAdapter.
+    assertEquals(0, request.getParamTypesCount());
+    assertEquals(1, request.getParams().getFieldsCount());
+    assertEquals("a", request.getParams().getFieldsMap().get("p1").getStringValue());
+  }
+
   @Test
   public void testAutoDescribedStatementsAreReused() throws SQLException {
     String jdbcSql = "select col_date from all_types where col_date=?";
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
index 19b87e5b..571d33dc 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/statements/StatementTest.java
@@ -39,6 +39,7 @@ import com.google.cloud.spanner.ResultSet;
 import com.google.cloud.spanner.SpannerException;
 import com.google.cloud.spanner.SpannerExceptionFactory;
 import com.google.cloud.spanner.Statement;
+import com.google.cloud.spanner.Value;
 import com.google.cloud.spanner.connection.AbstractStatementParser;
 import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
 import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
@@ -371,8 +372,10 @@ public class StatementTest {
 
     IntermediatePortalStatement portalStatement =
         intermediateStatement.createPortal("", parameters, new ArrayList<>(), new ArrayList<>());
-    assertThrows(
-        IllegalArgumentException.class, () -> portalStatement.bind(Statement.of(sqlStatement)));
+    Statement boundStatement = portalStatement.bind(Statement.of(sqlStatement));
+    assertEquals(
+        Value.untyped(com.google.protobuf.Value.newBuilder().setStringValue("{}").build()),
+        boundStatement.getParameters().get("p1"));
   }
 
   @Test