Repository: GoogleCloudPlatform/pgadapter
Pull request #88 (2022-03-29 18:50:11)

feat: add support for DATE data type
Adds tests for the `DATE` data type, and fixes some parsing issues with dates in text format.


Patch:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/DateParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/DateParser.java
index bd3433e4..61e979ba 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/DateParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/DateParser.java
@@ -26,6 +26,17 @@ import org.postgresql.util.ByteConverter;
 
 /** Translate wire protocol dates to desired formats. */
 public class DateParser extends Parser<Date> {
+  // Valid format for date: 'yyyy-MM-dd [+-]HH[:mi]'.
+  // Timezone information is optional. Timezone may also be specified using only hour value.
+  // NOTE: Following algorithm might perform slowly due to exception handling; sadly, this seems
+  //       to be the accepted default method for date validation.
+  private static final SimpleDateFormat[] VALID_DATE_FORMATS = {
+    new SimpleDateFormat("yyyy-MM-dd HH:mm"),
+    new SimpleDateFormat("yyyy-MM-dd +HH:mm"),
+    new SimpleDateFormat("yyyy-MM-dd -HH:mm"),
+    new SimpleDateFormat("yyyy-MM-dd +HH"),
+    new SimpleDateFormat("yyyy-MM-dd -HH")
+  };
 
   public DateParser(ResultSet item, int position) {
     this.item = item.getDate(position);
@@ -40,12 +51,13 @@ public class DateParser extends Parser<Date> {
       switch (formatCode) {
         case TEXT:
           String stringValue = new String(item, UTF8);
-          this.item = Date.parseDate(stringValue);
+          // Use the first 10 characters of the date string, as the string might contain a timezone
+          // identifier, which is not supported by parseDate(String).
+          this.item = Date.parseDate(stringValue.substring(0, 10));
           break;
         case BINARY:
           long days = ByteConverter.int4(item, 0) + PG_EPOCH_DAYS;
-          this.validateRange(days);
-          LocalDate localDate = LocalDate.ofEpochDay(days);
+          LocalDate localDate = LocalDate.ofEpochDay(validateRange(days));
           this.item =
               Date.fromYearMonthDay(
                   localDate.getYear(), localDate.getMonthValue(), localDate.getDayOfMonth());
@@ -64,20 +76,12 @@ public class DateParser extends Parser<Date> {
    */
   public static boolean isDate(String value) {
     Preconditions.checkNotNull(value);
-    // Valid format for date: 'yyyy-mm-dd [+-]HH:mi'.
-    // Timezone information is optional. Timezone may also be specified using only hour value.
-    // NOTE: Following algorithm might perform slowly due to exception handling; sadly, this seems
-    //       to be the accepted default method for date validation.
-    SimpleDateFormat[] dateFormats = {
-      new SimpleDateFormat("yyyy-mm-dd HH:mm"),
-      new SimpleDateFormat("yyyy-mm-dd +HH:mm"),
-      new SimpleDateFormat("yyyy-mm-dd -HH:mm")
-    };
-    for (SimpleDateFormat dateFormat : dateFormats) {
+    for (SimpleDateFormat dateFormat : VALID_DATE_FORMATS) {
       try {
         dateFormat.parse(value);
         return true;
       } catch (ParseException e) {
+        // ignore and try the next
       }
     }
     return false;
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 ac63e293..be70082a 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
@@ -64,11 +64,15 @@ public abstract class Parser<T> {
    *     no type could be guessed.
    */
   private static int guessType(byte[] item, FormatCode formatCode) {
+    // TODO: Put 'guessType' behind a command line flag so it is only enabled when wanted.
     if (formatCode == FormatCode.TEXT && item != null) {
       String value = new String(item, StandardCharsets.UTF_8);
       if (TimestampParser.isTimestamp(value)) {
         return Oid.TIMESTAMPTZ;
       }
+      if (DateParser.isDate(value)) {
+        return Oid.DATE;
+      }
     }
     return Oid.UNSPECIFIED;
   }


Test patch:
diff --git a/src/test/golang/pgadapter_pgx_tests/pgx.go b/src/test/golang/pgadapter_pgx_tests/pgx.go
index 8090ad44..c0250982 100644
--- a/src/test/golang/pgadapter_pgx_tests/pgx.go
+++ b/src/test/golang/pgadapter_pgx_tests/pgx.go
@@ -113,7 +113,7 @@ func TestQueryAllDataTypes(connString string) *C.char {
 	var float8Value float64
 	var numericValue pgtype.Numeric // pgx default maps numeric to string
 	var timestamptzValue time.Time
-	//var dateValue time.Time
+	var dateValue time.Time
 	var varcharValue string
 
 	err = conn.QueryRow(ctx, "SELECT * FROM AllTypes").Scan(
@@ -123,7 +123,7 @@ func TestQueryAllDataTypes(connString string) *C.char {
 		&float8Value,
 		&numericValue,
 		&timestamptzValue,
-		//&dateValue,
+		&dateValue,
 		&varcharValue,
 	)
 	if err != nil {
@@ -146,6 +146,10 @@ func TestQueryAllDataTypes(connString string) *C.char {
 	if g, w := numericValue, wantNumericValue; !reflect.DeepEqual(g, w) {
 		return C.CString(fmt.Sprintf("value mismatch\n Got: %v\nWant: %v", g, w))
 	}
+	wantDateValue, _ := time.Parse("2006-01-02", "2022-03-29")
+	if g, w := dateValue, wantDateValue; !reflect.DeepEqual(g, w) {
+		return C.CString(fmt.Sprintf("value mismatch\n Got: %v\nWant: %v", g, w))
+	}
 	wantTimestamptzValue, _ := time.Parse(time.RFC3339Nano, "2022-02-16T13:18:02.123456000Z")
 	if g, w := timestamptzValue, wantTimestamptzValue; !reflect.DeepEqual(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/AbstractMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/AbstractMockServerTest.java
index 3250167e..bec5393f 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/AbstractMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/AbstractMockServerTest.java
@@ -158,6 +158,10 @@ public abstract class AbstractMockServerTest {
                       Field.newBuilder()
                           .setName("col_timestamptz")
                           .setType(Type.newBuilder().setCode(TypeCode.TIMESTAMP).build()))
+                  .addFields(
+                      Field.newBuilder()
+                          .setName("col_date")
+                          .setType(Type.newBuilder().setCode(TypeCode.DATE).build()))
                   .addFields(
                       Field.newBuilder()
                           .setName("col_varchar")
@@ -181,6 +185,7 @@ public abstract class AbstractMockServerTest {
                   .addValues(Value.newBuilder().setStringValue("6.626").build())
                   .addValues(
                       Value.newBuilder().setStringValue("2022-02-16T13:18:02.123456789Z").build())
+                  .addValues(Value.newBuilder().setStringValue("2022-03-29").build())
                   .addValues(Value.newBuilder().setStringValue("test").build())
                   .build())
           .build();
@@ -196,6 +201,7 @@ public abstract class AbstractMockServerTest {
                   .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
                   .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
                   .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
+                  .addValues(Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build())
                   .build())
           .build();
 
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 f26b9178..2c76044a 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import com.google.cloud.ByteArray;
+import com.google.cloud.Date;
 import com.google.cloud.Timestamp;
 import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
 import com.google.cloud.spanner.Statement;
@@ -40,6 +41,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
 import java.time.Instant;
+import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
@@ -51,6 +53,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
+import org.postgresql.core.Oid;
 
 @RunWith(JUnit4.class)
 public class JdbcMockServerTest extends AbstractMockServerTest {
@@ -66,7 +69,12 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
    * mode for queries and DML statements.
    */
   private String createUrl() {
-    return String.format("jdbc:postgresql://localhost:%d/", pgServer.getLocalPort());
+    // Force binary transfer of date parameters to prevent PostgreSQL from sending them with type
+    // Oid.UNSPECIFIED. This setting is however at this moment ignored and depends on this PR:
+    // https://github.com/pgjdbc/pgjdbc/pull/2476
+    return String.format(
+        "jdbc:postgresql://localhost:%d/?binaryTransferEnable=%d",
+        pgServer.getLocalPort(), Oid.DATE);
   }
 
   @Test
@@ -99,7 +107,7 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
   @Test
   public void testQueryWithParameters() throws SQLException {
     String jdbcSql =
-        "select col_bigint, col_bool, col_bytea, col_float8, col_numeric, col_timestamptz, col_varchar "
+        "select col_bigint, col_bool, col_bytea, col_float8, col_numeric, col_timestamptz, col_date, col_varchar "
             + "from all_types "
             + "where col_bigint=? "
             + "and col_bool=? "
@@ -107,9 +115,10 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
             + "and col_float8=? "
             + "and col_numeric=? "
             + "and col_timestamptz=? "
+            + "and col_date=? "
             + "and col_varchar=?";
     String pgSql =
-        "select col_bigint, col_bool, col_bytea, col_float8, col_numeric, col_timestamptz, col_varchar "
+        "select col_bigint, col_bool, col_bytea, col_float8, col_numeric, col_timestamptz, col_date, col_varchar "
             + "from all_types "
             + "where col_bigint=$1 "
             + "and col_bool=$2 "
@@ -117,7 +126,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
             + "and col_float8=$4 "
             + "and col_numeric=$5 "
             + "and col_timestamptz=$6 "
-            + "and col_varchar=$7";
+            + "and col_date=$7 "
+            + "and col_varchar=$8";
     mockSpanner.putStatementResult(StatementResult.query(Statement.of(pgSql), ALL_TYPES_RESULTSET));
     mockSpanner.putStatementResult(
         StatementResult.query(
@@ -135,6 +145,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
                 .bind("p6")
                 .to(Timestamp.parseTimestamp("2022-02-16T13:18:02.123457000Z"))
                 .bind("p7")
+                .to(Date.parseDate("2022-03-29"))
+                .bind("p8")
                 .to("test")
                 .build(),
             ALL_TYPES_RESULTSET));
@@ -143,13 +155,16 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
         LocalDateTime.of(2022, 2, 16, 13, 18, 2, 123456789).atOffset(ZoneOffset.UTC);
     try (Connection connection = DriverManager.getConnection(createUrl())) {
       try (PreparedStatement preparedStatement = connection.prepareStatement(jdbcSql)) {
-        preparedStatement.setLong(1, 1L);
-        preparedStatement.setBoolean(2, true);
-        preparedStatement.setBytes(3, "test".getBytes(StandardCharsets.UTF_8));
-        preparedStatement.setDouble(4, 3.14d);
-        preparedStatement.setBigDecimal(5, new BigDecimal("6.626"));
-        preparedStatement.setTimestamp(6, java.sql.Timestamp.from(Instant.from(zonedDateTime)));
-        preparedStatement.setString(7, "test");
+        int index = 0;
+        preparedStatement.setLong(++index, 1L);
+        preparedStatement.setBoolean(++index, true);
+        preparedStatement.setBytes(++index, "test".getBytes(StandardCharsets.UTF_8));
+        preparedStatement.setDouble(++index, 3.14d);
+        preparedStatement.setBigDecimal(++index, new BigDecimal("6.626"));
+        preparedStatement.setTimestamp(
+            ++index, java.sql.Timestamp.from(Instant.from(zonedDateTime)));
+        preparedStatement.setDate(++index, java.sql.Date.valueOf(LocalDate.of(2022, 3, 29)));
+        preparedStatement.setString(++index, "test");
         try (ResultSet resultSet = preparedStatement.executeQuery()) {
           assertTrue(resultSet.next());
           assertEquals(1L, resultSet.getLong(1));
@@ -183,8 +198,10 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
     assertEquals("6.626", params.get("p5").getStringValue());
     assertEquals(TypeCode.TIMESTAMP, types.get("p6").getCode());
     assertEquals("2022-02-16T13:18:02.123457000Z", params.get("p6").getStringValue());
-    assertEquals(TypeCode.STRING, types.get("p7").getCode());
-    assertEquals("test", params.get("p7").getStringValue());
+    assertEquals(TypeCode.DATE, types.get("p7").getCode());
+    assertEquals("2022-03-29", params.get("p7").getStringValue());
+    assertEquals(TypeCode.STRING, types.get("p8").getCode());
+    assertEquals("test", params.get("p8").getStringValue());
   }
 
   @Test
@@ -193,8 +210,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
         StatementResult.update(
             Statement.newBuilder(
                     "insert into all_types "
-                        + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_varchar) "
-                        + "values ($1, $2, $3, $4, $5, $6, $7, $8)")
+                        + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) "
+                        + "values ($1, $2, $3, $4, $5, $6, $7, $8, $9)")
                 .bind("p1")
                 .to(2L)
                 .bind("p2")
@@ -210,6 +227,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
                 .bind("p7")
                 .to((com.google.cloud.spanner.Value) null)
                 .bind("p8")
+                .to((Date) null)
+                .bind("p9")
                 .to((String) null)
                 .build(),
             1L));
@@ -219,28 +238,20 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
             ALL_TYPES_NULLS_RESULTSET));
 
     try (Connection connection = DriverManager.getConnection(createUrl())) {
-      // TODO: Add col_timestamptz to the statement when PgAdapter allows untyped null values.
-      // The PG JDBC driver sends date and timestamp parameters with type code Oid.UNSPECIFIED.
-      // TODO: Add col_numeric to the statement when the mock serverr supports numeric parameters.
       try (PreparedStatement statement =
           connection.prepareStatement(
               "insert into all_types "
-                  + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_varchar) "
-                  + "values (?, ?, ?, ?, ?, ?, ?, ?)")) {
+                  + "(col_bigint, col_bool, col_bytea, col_float8, col_int, col_numeric, col_timestamptz, col_date, col_varchar) "
+                  + "values (?, ?, ?, ?, ?, ?, ?, ?, ?)")) {
         int index = 0;
         statement.setLong(++index, 2);
         statement.setNull(++index, Types.BOOLEAN);
         statement.setNull(++index, Types.BINARY);
         statement.setNull(++index, Types.DOUBLE);
         statement.setNull(++index, Types.INTEGER);
-        // TODO: Enable the next line when the mock server supports numeric parameter values.
         statement.setNull(++index, Types.NUMERIC);
-        // TODO: Enable the next line when PgAdapter allows untyped null values.
-        // This is currently blocked on both the Spangres backend allowing untyped null values in
-        // SQL statements, as well as the Java client library and/or JDBC driver allowing untyped
-        // null values.
-        // See also https://github.com/googleapis/java-spanner/pull/1680
         statement.setNull(++index, Types.TIMESTAMP_WITH_TIMEZONE);
+        statement.setNull(++index, Types.DATE);
         statement.setNull(++index, Types.VARCHAR);
 
         assertEquals(1, statement.executeUpdate());
@@ -267,6 +278,8 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
         assertTrue(resultSet.wasNull());
         assertNull(resultSet.getTimestamp(++index));
         assertTrue(resultSet.wasNull());
+        assertNull(resultSet.getDate(++index));
+        assertTrue(resultSet.wasNull());
         assertNull(resultSet.getString(++index));
         assertTrue(resultSet.wasNull());