Repository: GoogleCloudPlatform/pgadapter (java)

Issue:
Timestamps with nanosecond precision breaks SQLAlchemy
Timestamps were returned with nanosecond precision in the text format. This could break clients that do not expect this, such as SQLAlchemy.

Pull request:
fix: always return timestamp in microsecond precision
PGAdapter would return timestamps with nanosecond precision when the text format was used for the timestamp. This could break clients that do not expect this.

Fixes #512

Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParser.java
index 7b5961c5..b013abcf 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParser.java
@@ -64,7 +64,9 @@ public class TimestampParser extends Parser<Timestamp> {
           .parseLenient()
           .parseCaseInsensitive()
           .appendPattern("yyyy-MM-dd HH:mm:ss")
-          .appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true)
+          // We should never return more than microsecond precision, as some PG clients, such as
+          // SQLAlchemy will fail if they receive more.
+          .appendFraction(ChronoField.NANO_OF_SECOND, 0, 6, true)
           // Java 8 does not support seconds in timezone offset.
           .appendOffset(OptionsMetadata.isJava8() ? "+HH:mm" : "+HH:mm:ss", "+00")
           .toFormatter();


Test patch:
diff --git a/src/test/golang/pgadapter_pgx_tests/pgx.go b/src/test/golang/pgadapter_pgx_tests/pgx.go
index cee4fcd7..30179ba1 100644
--- a/src/test/golang/pgadapter_pgx_tests/pgx.go
+++ b/src/test/golang/pgadapter_pgx_tests/pgx.go
@@ -174,12 +174,7 @@ func TestQueryAllDataTypes(connString string, oid, format int16) *C.char {
 	if g, w := dateValue, wantDateValue; !reflect.DeepEqual(g, w) {
 		return C.CString(fmt.Sprintf("value mismatch\n Got: %v\nWant: %v", g, w))
 	}
-	// Encoding the timestamp values as a parameter will truncate it to microsecond precision.
 	wantTimestamptzValue, _ := time.Parse(time.RFC3339Nano, "2022-02-16T13:18:02.123456+00:00")
-	if strings.Contains(connString, "prefer_simple_protocol=true") || (oid == pgtype.TimestamptzOID && format == 0) {
-		// Simple protocol writes the timestamp as a string and preserves nanosecond precision.
-		wantTimestamptzValue, _ = time.Parse(time.RFC3339Nano, "2022-02-16T13:18:02.123456789+00:00")
-	}
 	if g, w := timestamptzValue.UTC().String(), wantTimestamptzValue.UTC().String(); g != w {
 		return C.CString(fmt.Sprintf("value mismatch\n Got: %v\nWant: %v", g, w))
 	}
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
index 8d18de14..bc811aea 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/AbortedMockServerTest.java
@@ -319,13 +319,8 @@ public class AbortedMockServerTest extends AbstractMockServerTest {
             assertEquals(3.14d, resultSet.getDouble(++index), 0.0d);
             assertEquals(100, resultSet.getInt(++index));
             assertEquals(new BigDecimal("6.626"), resultSet.getBigDecimal(++index));
-            if (preparedThreshold < 0) {
-              // The binary format will truncate the timestamp value to microseconds.
-              assertEquals(
-                  truncatedOffsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
-            } else {
-              assertEquals(offsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
-            }
+            assertEquals(
+                truncatedOffsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
             assertEquals(LocalDate.of(2022, 3, 29), resultSet.getObject(++index, LocalDate.class));
             assertEquals("test", resultSet.getString(++index));
             assertEquals("{\"key\": \"value\"}", resultSet.getString(++index));
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/ArraysMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/ArraysMockServerTest.java
index fa0b36f7..1f7a8120 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/ArraysMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/ArraysMockServerTest.java
@@ -291,10 +291,10 @@ public class ArraysMockServerTest extends AbstractMockServerTest {
         Array array = resultSet.getArray("TIMESTAMP_ARRAY");
         Timestamp[] expected =
             new Timestamp[] {
-              com.google.cloud.Timestamp.parseTimestamp("2022-02-14T11:47:10.123456700Z")
+              com.google.cloud.Timestamp.parseTimestamp("2022-02-14T11:47:10.123456Z")
                   .toSqlTimestamp(),
               null,
-              com.google.cloud.Timestamp.parseTimestamp("2000-02-29T00:00:01.00000100Z")
+              com.google.cloud.Timestamp.parseTimestamp("2000-02-29T00:00:01.000001Z")
                   .toSqlTimestamp()
             };
         Timestamp[] actual = (Timestamp[]) array.getArray();
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/CopyOutMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/CopyOutMockServerTest.java
index 73954c35..15373904 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/CopyOutMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/CopyOutMockServerTest.java
@@ -214,7 +214,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
       copyManager.copyOut("COPY all_types TO STDOUT", writer);
 
       assertEquals(
-          "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456789+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
+          "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
           writer.toString());
 
       // Verify that we can use the connection for normal queries.
@@ -247,7 +247,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
       copyManager.copyOut("COPY all_types (col_bigint, col_varchar) TO STDOUT", writer);
 
       assertEquals(
-          "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456789+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
+          "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
           writer.toString());
 
       // Verify that we can use the connection for normal queries.
@@ -280,7 +280,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
           "COPY all_types TO STDOUT (format csv, escape '~', delimiter '-')", writer);
 
       assertEquals(
-          "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456789+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
+          "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
           writer.toString());
     }
   }
@@ -299,7 +299,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
 
       assertEquals(
           "col_bigint-col_bool-col_bytea-col_float8-col_int-col_numeric-col_timestamptz-col_date-col_varchar-col_jsonb\n"
-              + "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456789+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
+              + "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
           writer.toString());
     }
   }
@@ -320,7 +320,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
 
       assertEquals(
           "col_bigint\n"
-              + "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456789+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
+              + "1-t-\\x74657374-3.14-100-6.626-\"2022-02-16 13:18:02.123456+00\"-\"2022-03-29\"-test-\"{~\"key~\": ~\"value~\"}\"\n",
           writer.toString());
     }
   }
@@ -341,7 +341,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
 
       assertEquals(
           "col_bigint|col_bool|col_bytea|col_float8|col_int|col_numeric|col_timestamptz|col_date|col_varchar|col_jsonb\n"
-              + "1|t|\"\\\\x74657374\"|3.14|100|6.626|2022-02-16 13:18:02.123456789+00|2022-03-29|test|\"{\\\"key\\\": \\\"value\\\"}\"\n",
+              + "1|t|\"\\\\x74657374\"|3.14|100|6.626|2022-02-16 13:18:02.123456+00|2022-03-29|test|\"{\\\"key\\\": \\\"value\\\"}\"\n",
           writer.toString());
     }
   }
@@ -359,7 +359,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
           "COPY all_types TO STDOUT (format csv, escape '\\', delimiter '|', quote '\"')", writer);
 
       assertEquals(
-          "1|t|\"\\\\x74657374\"|3.14|100|6.626|2022-02-16 13:18:02.123456789+00|2022-03-29|test|\"{\\\"key\\\": \\\"value\\\"}\"\n",
+          "1|t|\"\\\\x74657374\"|3.14|100|6.626|2022-02-16 13:18:02.123456+00|2022-03-29|test|\"{\\\"key\\\": \\\"value\\\"}\"\n",
           writer.toString());
     }
   }
@@ -378,7 +378,7 @@ public class CopyOutMockServerTest extends AbstractMockServerTest {
           writer);
 
       assertEquals(
-          "\"1\"|\"t\"|\"\\\\x74657374\"|\"3.14\"|\"100\"|\"6.626\"|\"2022-02-16 13:18:02.123456789+00\"|\"2022-03-29\"|\"test\"|\"{\\\"key\\\": \\\"value\\\"}\"\n",
+          "\"1\"|\"t\"|\"\\\\x74657374\"|\"3.14\"|\"100\"|\"6.626\"|\"2022-02-16 13:18:02.123456+00\"|\"2022-03-29\"|\"test\"|\"{\\\"key\\\": \\\"value\\\"}\"\n",
           writer.toString());
     }
   }
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 5bb93078..821cc556 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -507,13 +507,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
             assertEquals(3.14d, resultSet.getDouble(++index), 0.0d);
             assertEquals(100, resultSet.getInt(++index));
             assertEquals(new BigDecimal("6.626"), resultSet.getBigDecimal(++index));
-            if (preparedThreshold < 0) {
-              // The binary format will truncate the timestamp value to microseconds.
-              assertEquals(
-                  truncatedOffsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
-            } else {
-              assertEquals(offsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
-            }
+            assertEquals(
+                truncatedOffsetDateTime, resultSet.getObject(++index, OffsetDateTime.class));
             assertEquals(LocalDate.of(2022, 3, 29), resultSet.getObject(++index, LocalDate.class));
             assertEquals("test", resultSet.getString(++index));
             assertEquals("{\"key\": \"value\"}", resultSet.getString(++index));
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/ITNodePostgresTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/ITNodePostgresTest.java
index e3463206..ee0bfa9b 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/ITNodePostgresTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/ITNodePostgresTest.java
@@ -389,7 +389,7 @@ public class ITNodePostgresTest implements IntegrationTest {
 
     String output = runTest("testCopyTo", getHost(), testEnv.getServer().getLocalPort());
     assertEquals(
-        "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456789+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
+        "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
         output);
   }
 
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/NodePostgresMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/NodePostgresMockServerTest.java
index 04014463..dcfafbec 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/NodePostgresMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/nodejs/NodePostgresMockServerTest.java
@@ -589,7 +589,7 @@ public class NodePostgresMockServerTest extends AbstractMockServerTest {
     String output = runTest("testCopyTo", getHost(), pgServer.getLocalPort());
 
     assertEquals(
-        "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456789+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
+        "1\tt\t\\\\x74657374\t3.14\t100\t6.626\t2022-02-16 13:18:02.123456+00\t2022-03-29\ttest\t{\"key\": \"value\"}\n",
         output);
   }
 
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 c2ae5b68..54532011 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
@@ -132,7 +132,7 @@ public class ArrayParserTest {
             sessionState);
 
     assertEquals(
-        "{\"2022-07-08 07:00:02.123456789+00\",NULL,\"2000-01-01 00:00:00+00\"}",
+        "{\"2022-07-08 07:00:02.123456+00\",NULL,\"2000-01-01 00:00:00+00\"}",
         parser.stringParse());
   }
 
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParserTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParserTest.java
index 932fb5d1..0f3492a9 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParserTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/TimestampParserTest.java
@@ -99,7 +99,7 @@ public class TimestampParserTest {
     SessionState sessionState = mock(SessionState.class);
     when(sessionState.getTimezone()).thenReturn(ZoneId.of("+00"));
     assertEquals(
-        "2022-07-08 07:22:59.123456789+00",
+        "2022-07-08 07:22:59.123456+00",
         new TimestampParser(
                 Timestamp.parseTimestamp("2022-07-08T07:22:59.123456789Z"), sessionState)
             .stringParse());
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/python/pg8000/Pg8000BasicsTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/python/pg8000/Pg8000BasicsTest.java
index 7fe46b85..a70e5dd0 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/python/pg8000/Pg8000BasicsTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/python/pg8000/Pg8000BasicsTest.java
@@ -109,8 +109,8 @@ public class Pg8000BasicsTest extends AbstractMockServerTest {
     String actualOutput = execute("select_all_types.py", host, pgServer.getLocalPort());
     String expectedOutput =
         "row: [1, True, b'test', 3.14, 100, Decimal('6.626'), "
-            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=tzutc()), datetime.date(2022, 3, 29), "
-            + "'test', {'key': 'value'}]\n";
+            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=datetime.timezone.utc), "
+            + "datetime.date(2022, 3, 29), 'test', {'key': 'value'}]\n";
     assertEquals(expectedOutput, actualOutput.replace("tzlocal()", "tzutc()"));
   }
 
@@ -122,12 +122,12 @@ public class Pg8000BasicsTest extends AbstractMockServerTest {
     String actualOutput = execute("select_parameterized.py", host, pgServer.getLocalPort());
     String expectedOutput =
         "first execution: [1, True, b'test', 3.14, 100, Decimal('6.626'), "
-            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=tzutc()), datetime.date(2022, 3, 29), "
-            + "'test', {'key': 'value'}]\n"
+            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=datetime.timezone.utc), "
+            + "datetime.date(2022, 3, 29), 'test', {'key': 'value'}]\n"
             + "second execution: [1, True, b'test', 3.14, 100, Decimal('6.626'), "
-            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=tzutc()), datetime.date(2022, 3, 29), "
-            + "'test', {'key': 'value'}]\n";
-    assertEquals(expectedOutput, actualOutput.replace("tzlocal()", "tzutc()"));
+            + "datetime.datetime(2022, 2, 16, 13, 18, 2, 123456, tzinfo=datetime.timezone.utc), "
+            + "datetime.date(2022, 3, 29), 'test', {'key': 'value'}]\n";
+    assertEquals(expectedOutput, actualOutput);
 
     List<WireMessage> messages = getWireMessages();
     assertEquals(25, messages.size());
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/CloseResponseTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/CloseResponseTest.java
new file mode 100644
index 00000000..564e9f4c
--- /dev/null
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/CloseResponseTest.java
@@ -0,0 +1,36 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.cloud.spanner.pgadapter.wireoutput;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class CloseResponseTest {
+
+  @Test
+  public void testMessage() {
+    DataOutputStream output = new DataOutputStream(new ByteArrayOutputStream());
+    CloseResponse closeResponse = new CloseResponse(output);
+    assertEquals('3', closeResponse.getIdentifier());
+    assertEquals("Close", closeResponse.getMessageName());
+    assertEquals("Length: 4", closeResponse.getPayloadString());
+  }
+}
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/StartupMessageResponseTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/StartupMessageResponseTest.java
new file mode 100644
index 00000000..82306c79
--- /dev/null
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/wireoutput/StartupMessageResponseTest.java
@@ -0,0 +1,46 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.cloud.spanner.pgadapter.wireoutput;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import java.nio.charset.StandardCharsets;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class StartupMessageResponseTest {
+
+  @Test
+  public void testMessage() throws Exception {
+    ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+    DataOutputStream output = new DataOutputStream(bytes);
+    StartUpMessageResponse response =
+        new StartUpMessageResponse(
+            output,
+            "key".getBytes(StandardCharsets.UTF_8),
+            "value".getBytes(StandardCharsets.UTF_8));
+    assertEquals('S', response.getIdentifier());
+    assertEquals("Start-Up", response.getMessageName());
+
+    response.sendPayload();
+
+    assertArrayEquals("key\0value\0".getBytes(StandardCharsets.UTF_8), bytes.toByteArray());
+  }
+}