Repository: GoogleCloudPlatform/pgadapter
Pull request #572 (2023-01-03 20:08:21)

perf: reduce conversion time for timestamptz, date and bytea
Reduces the time needed to convert data of type timestamptz, date and bytea from Cloud Spanner wire-format to PostgreSQL (text) wire format.

codecov[bot]: # [Codecov](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/572?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) Report
> Merging [#572](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/572?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (0f3ecf9) into [postgresql-dialect](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/commit/9b0573e64ff66346fde2a26dd989359d6d32a019?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) (9b0573e) will **decrease** coverage by `0.00%`.
> The diff coverage is `88.88%`.

```diff
@@                   Coverage Diff                    @@
##             postgresql-dialect     #572      +/-   ##
========================================================
- Coverage                 88.41%   88.40%   -0.01%     
- Complexity                 2069     2078       +9     
========================================================
  Files                       122      122              
  Lines                      6791     6822      +31     
  Branches                    956      962       +6     
========================================================
+ Hits                       6004     6031      +27     
- Misses                      558      560       +2     
- Partials                    229      231       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| all_tests | `88.40% <88.88%> (-0.01%)` | :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/572?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform) | Coverage Δ | |
|---|---|---|
| [...le/cloud/spanner/pgadapter/parsers/DateParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/572/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3BhcnNlcnMvRGF0ZVBhcnNlci5qYXZh) | `84.37% <78.94%> (-2.86%)` | :arrow_down: |
| [.../cloud/spanner/pgadapter/parsers/BinaryParser.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/572/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3BhcnNlcnMvQmluYXJ5UGFyc2VyLmphdmE=) | `83.78% <100.00%> (+5.21%)` | :arrow_up: |
| [.../cloud/spanner/pgadapter/session/SessionState.java](https://codecov.io/gh/GoogleCloudPlatform/pgadapter/pull/572/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GoogleCloudPlatform#diff-c3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3NwYW5uZXIvcGdhZGFwdGVyL3Nlc3Npb24vU2Vzc2lvblN0YXRlLmphdmE=) | `95.69% <100.00%> (+0.11%)` | :arrow_up: |

: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/BinaryParser.java b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/BinaryParser.java
index f01a60c6..1be85a8d 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/parsers/BinaryParser.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/parsers/BinaryParser.java
@@ -22,7 +22,6 @@ import com.google.cloud.spanner.pgadapter.ProxyServer.DataFormat;
 import com.google.cloud.spanner.pgadapter.error.PGExceptionFactory;
 import java.nio.charset.StandardCharsets;
 import javax.annotation.Nonnull;
-import org.postgresql.core.Utils;
 import org.postgresql.util.PGbytea;
 
 /**
@@ -67,7 +66,21 @@ public class BinaryParser extends Parser<ByteArray> {
 
   @Override
   public String stringParse() {
-    return this.item == null ? null : "\\x" + Utils.toHexString(this.item.toByteArray());
+    return this.item == null ? null : bytesToHex(this.item.toByteArray());
+  }
+
+  private static final char[] HEX_ARRAY = "0123456789abcdef".toCharArray();
+
+  static String bytesToHex(byte[] bytes) {
+    char[] hexChars = new char[bytes.length * 2 + 2];
+    hexChars[0] = '\\';
+    hexChars[1] = 'x';
+    for (int j = 0; j < bytes.length; j++) {
+      int v = bytes[j] & 0xFF;
+      hexChars[j * 2 + 2] = HEX_ARRAY[v >>> 4];
+      hexChars[j * 2 + 3] = HEX_ARRAY[v & 0x0F];
+    }
+    return new String(hexChars);
   }
 
   @Override
@@ -86,7 +99,7 @@ public class BinaryParser extends Parser<ByteArray> {
       case POSTGRESQL_BINARY:
         return resultSet.getBytes(position).toByteArray();
       case POSTGRESQL_TEXT:
-        return ("\\x" + Utils.toHexString(resultSet.getBytes(position).toByteArray()))
+        return (bytesToHex(resultSet.getBytes(position).toByteArray()))
             .getBytes(StandardCharsets.UTF_8);
       default:
         throw new IllegalArgumentException("unknown data format: " + format);
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 489a2069..010c2bde 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
@@ -108,7 +108,32 @@ public class DateParser extends Parser<Date> {
 
   @Override
   public String stringParse() {
-    return this.item == null ? null : item.toString();
+    return this.item == null ? null : toString(this.item);
+  }
+
+  static String toString(Date date) {
+    int yearValue = date.getYear();
+    int monthValue = date.getMonth();
+    int dayValue = date.getDayOfMonth();
+    StringBuilder buf = new StringBuilder(10);
+    if (yearValue < 1000) {
+      // Note that Date cannot contain negative year numbers, so this value is always guaranteed to
+      // be between 1 and 999 (inclusive).
+      buf.append(yearValue + 10000).deleteCharAt(0);
+    } else {
+      // Year values beyond 9999 are not supported by Cloud Spanner, but it is allowed by the Date
+      // class. The ISO specification requires year numbers that have more than 4 digits to have a
+      // plus or minus sign.
+      if (yearValue > 9999) {
+        buf.append('+');
+      }
+      buf.append(yearValue);
+    }
+    return buf.append(monthValue < 10 ? "-0" : "-")
+        .append(monthValue)
+        .append(dayValue < 10 ? "-0" : "-")
+        .append(dayValue)
+        .toString();
   }
 
   @Override
@@ -130,7 +155,7 @@ public class DateParser extends Parser<Date> {
     switch (format) {
       case SPANNER:
       case POSTGRESQL_TEXT:
-        return resultSet.getDate(position).toString().getBytes(StandardCharsets.UTF_8);
+        return toString(resultSet.getDate(position)).getBytes(StandardCharsets.UTF_8);
       case POSTGRESQL_BINARY:
         return convertToPG(resultSet.getDate(position));
       default:
diff --git a/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java b/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
index 74ec2701..4692e590 100644
--- a/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
+++ b/src/main/java/com/google/cloud/spanner/pgadapter/session/SessionState.java
@@ -231,6 +231,7 @@ public class SessionState {
 
   private void internalSet(
       String extension, String name, String setting, Map<String, PGSetting> currentSettings) {
+    clearCachedValues();
     String key = toKey(extension, name);
     PGSetting newSetting = currentSettings.get(key);
     if (newSetting == null) {
@@ -252,6 +253,10 @@ public class SessionState {
     currentSettings.put(key, newSetting);
   }
 
+  private void clearCachedValues() {
+    cachedZoneId = null;
+  }
+
   /** Returns the current value of the specified setting. */
   public PGSetting get(String extension, String name) {
     return internalGet(toKey(extension, name), true);
@@ -403,8 +408,13 @@ public class SessionState {
         () -> DdlTransactionMode.valueOf(setting.getBootVal()));
   }
 
+  private ZoneId cachedZoneId;
+
   /** Returns the {@link ZoneId} of the current timezone for this session. */
   public ZoneId getTimezone() {
+    if (cachedZoneId != null) {
+      return cachedZoneId;
+    }
     PGSetting setting = internalGet(toKey(null, "timezone"), false);
     if (setting == null) {
       return ZoneId.systemDefault();
@@ -416,7 +426,7 @@ public class SessionState {
             setting::getResetVal,
             setting::getBootVal);
 
-    return zoneIdFromString(id);
+    return (cachedZoneId = zoneIdFromString(id));
   }
 
   private ZoneId zoneIdFromString(String value) {


Test patch:
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/EmulatedPsqlMockServerTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/EmulatedPsqlMockServerTest.java
index 59ad5586..5171e579 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/EmulatedPsqlMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/EmulatedPsqlMockServerTest.java
@@ -26,6 +26,8 @@ import com.google.cloud.spanner.Statement;
 import com.google.cloud.spanner.pgadapter.error.PGException;
 import com.google.cloud.spanner.pgadapter.statements.IntermediateStatement;
 import com.google.common.collect.ImmutableList;
+import com.google.protobuf.ListValue;
+import com.google.protobuf.Value;
 import com.google.rpc.ResourceInfo;
 import com.google.spanner.admin.database.v1.Database;
 import com.google.spanner.admin.database.v1.DatabaseDialect;
@@ -37,6 +39,7 @@ import com.google.spanner.v1.DatabaseName;
 import com.google.spanner.v1.ExecuteBatchDmlRequest;
 import com.google.spanner.v1.ExecuteSqlRequest;
 import com.google.spanner.v1.SessionName;
+import com.google.spanner.v1.TypeCode;
 import io.grpc.Metadata;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
@@ -307,6 +310,51 @@ public class EmulatedPsqlMockServerTest extends AbstractMockServerTest {
     }
   }
 
+  @Test
+  public void testTimezone() throws SQLException {
+    String sql = "select ts from foo";
+    mockSpanner.putStatementResult(
+        StatementResult.query(
+            Statement.of(sql),
+            com.google.spanner.v1.ResultSet.newBuilder()
+                .setMetadata(createMetadata(ImmutableList.of(TypeCode.TIMESTAMP)))
+                .addRows(
+                    ListValue.newBuilder()
+                        .addValues(
+                            Value.newBuilder()
+                                .setStringValue("2023-01-06T11:49:15.123456789Z")
+                                .build())
+                        .build())
+                .build()));
+
+    try (Connection connection = DriverManager.getConnection(createUrl("my-db"))) {
+      connection.createStatement().execute("set time zone cet");
+      try (ResultSet resultSet = connection.createStatement().executeQuery(sql)) {
+        while (resultSet.next()) {
+          assertEquals("2023-01-06 12:49:15.123456+01", resultSet.getString(1));
+        }
+      }
+      connection.createStatement().execute("set time zone ist");
+      try (ResultSet resultSet = connection.createStatement().executeQuery(sql)) {
+        while (resultSet.next()) {
+          assertEquals("2023-01-06 17:19:15.123456+05:30", resultSet.getString(1));
+        }
+      }
+      connection.createStatement().execute("set time zone 'America/Los_Angeles'");
+      try (ResultSet resultSet = connection.createStatement().executeQuery(sql)) {
+        while (resultSet.next()) {
+          assertEquals("2023-01-06 03:49:15.123456-08", resultSet.getString(1));
+        }
+      }
+      connection.createStatement().execute("set time zone -12");
+      try (ResultSet resultSet = connection.createStatement().executeQuery(sql)) {
+        while (resultSet.next()) {
+          assertEquals("2023-01-05 23:49:15.123456-12", resultSet.getString(1));
+        }
+      }
+    }
+  }
+
   static StatusRuntimeException newStatusResourceNotFoundException(
       String shortName, String resourceType, String resourceName) {
     ResourceInfo resourceInfo =
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 78604fce..078843c0 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/JdbcMockServerTest.java
@@ -42,6 +42,7 @@ import com.google.cloud.spanner.pgadapter.wireprotocol.ControlMessage.PreparedTy
 import com.google.cloud.spanner.pgadapter.wireprotocol.DescribeMessage;
 import com.google.cloud.spanner.pgadapter.wireprotocol.ExecuteMessage;
 import com.google.cloud.spanner.pgadapter.wireprotocol.ParseMessage;
+import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
 import com.google.protobuf.ListValue;
 import com.google.protobuf.Value;
@@ -77,9 +78,11 @@ import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.time.temporal.ChronoUnit;
 import java.util.*;
+import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -3368,6 +3371,29 @@ public class JdbcMockServerTest extends AbstractMockServerTest {
     assertEquals(0, mockSpanner.countRequestsOfType(CommitRequest.class));
   }
 
+  @Ignore("Only used for manual performance testing")
+  @Test
+  public void testBasePerformance() throws SQLException {
+    final int numRuns = 1000;
+    String sql = "select * from random_benchmark";
+    RandomResultSetGenerator generator = new RandomResultSetGenerator(10, Dialect.POSTGRESQL);
+    for (int run = 0; run < numRuns; run++) {
+      mockSpanner.putStatementResult(
+          StatementResult.query(Statement.of(sql + run), generator.generate()));
+    }
+    try (Connection connection = DriverManager.getConnection(createUrl())) {
+      Stopwatch watch = Stopwatch.createStarted();
+      for (int run = 0; run < numRuns; run++) {
+        try (ResultSet resultSet = connection.createStatement().executeQuery(sql + run)) {
+          while (resultSet.next()) {
+            // ignore
+          }
+        }
+      }
+      System.out.printf("Elapsed: %dms\n", watch.elapsed(TimeUnit.MILLISECONDS));
+    }
+  }
+
   private void verifySettingIsNull(Connection connection, String setting) throws SQLException {
     try (ResultSet resultSet =
         connection.createStatement().executeQuery(String.format("show %s", setting))) {
diff --git a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/DateParserTest.java b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/DateParserTest.java
index e01399f4..4bed704b 100644
--- a/src/test/java/com/google/cloud/spanner/pgadapter/parsers/DateParserTest.java
+++ b/src/test/java/com/google/cloud/spanner/pgadapter/parsers/DateParserTest.java
@@ -49,4 +49,13 @@ public class DateParserTest {
         PGException.class,
         () -> new DateParser("2022".getBytes(StandardCharsets.UTF_8), FormatCode.TEXT));
   }
+
+  @Test
+  public void testToString() {
+    assertEquals("2023-01-06", DateParser.toString(Date.fromYearMonthDay(2023, 1, 6)));
+    assertEquals("0900-01-06", DateParser.toString(Date.fromYearMonthDay(900, 1, 6)));
+    assertEquals("0016-02-28", DateParser.toString(Date.fromYearMonthDay(16, 2, 28)));
+    assertEquals("0001-01-01", DateParser.toString(Date.fromYearMonthDay(1, 1, 1)));
+    assertEquals("+10000-01-01", DateParser.toString(Date.fromYearMonthDay(10000, 1, 1)));
+  }
 }