Code changed to fix the Connection Pool 'close' implementation

Summary:
Code changed to throw an exception in case of decimal overflow for long/short/int

Code changed to change the Codec Instance Type from CLOB to String for non-binary BLOB Column types.

Code changed to ensure retry is limited by retriesAllDown.

Code changed to fix the issue with negatvie time value which was returning erronous LocalDateTime values.

Code changed to return no rows in the getGeneratedKeys method when no keys generated while running insert query.

Code changed to add microsecond precision in the TimeStamp encoding.

Test Plan: Test cases were updated which got impacted due to the changes in the above mentioned code

Reviewers: hniemchenko-ua

Reviewed By: hniemchenko-ua

Subscribers: engineering-list

JIRA Issues: PLAT-6313

Differential Revision: https://grizzly.internal.memcompute.com/D58319

src/main/java/com/singlestore/jdbc/Connection.java

@@ -190,8 +190,7 @@ public void rollback() throws SQLException {
   @Override
   public void close() throws SQLException {
     if (poolConnection != null) {
-      SingleStorePoolConnection poolConnection = this.poolConnection;
-      poolConnection.close();
+      poolConnection.fireConnectionClosed(new ConnectionEvent(poolConnection));
       return;
     }
     client.close();

src/main/java/com/singlestore/jdbc/Statement.java

@@ -826,16 +826,17 @@ public ResultSet getGeneratedKeys() throws SQLException {
 
     if (currResult instanceof OkPacket) {
       OkPacket ok = ((OkPacket) currResult);
-      List<String[]> insertIds = new ArrayList<>();
-      insertIds.add(new String[] {String.valueOf(ok.getLastInsertId())});
-      for (int i = 0; i < results.size(); i++) {
-        if (results.get(i) instanceof OkPacket) {
-          insertIds.add(
-              new String[] {String.valueOf(((OkPacket) results.get(i)).getLastInsertId())});
+      if (ok.getLastInsertId() != 0) {
+        List<String[]> insertIds = new ArrayList<>();
+        insertIds.add(new String[] {String.valueOf(ok.getLastInsertId())});
+        for (Completion result : results) {
+          if (result instanceof OkPacket) {
+            insertIds.add(new String[] {String.valueOf(((OkPacket) result).getLastInsertId())});
+          }
         }
+        String[][] ids = insertIds.toArray(new String[0][]);
+        return CompleteResult.createResultSet("insert_id", DataType.BIGINT, ids, con.getContext());
       }
-      String[][] ids = insertIds.toArray(new String[0][]);
-      return CompleteResult.createResultSet("insert_id", DataType.BIGINT, ids, con.getContext());
     }
 
     return new CompleteResult(new ColumnDefinitionPacket[0], new byte[0][], con.getContext());

src/main/java/com/singlestore/jdbc/client/MultiPrimaryClient.java

@@ -71,7 +71,8 @@ protected Client connectHost(boolean readOnly, boolean failFast) throws SQLExcep
     int maxRetries = conf.retriesAllDown();
 
     while ((host = conf.haMode().getAvailableHost(conf.addresses(), denyList, !readOnly))
-        .isPresent()) {
+            .isPresent()
+        && maxRetries > 0) {
       try {
         return conf.transactionReplay()
             ? new ClientReplayImpl(conf, host.get(), lock, false)

src/main/java/com/singlestore/jdbc/codec/list/IntCodec.java

@@ -119,9 +119,9 @@ public int decodeTextInt(
       case CHAR:
         String str = buf.readString(length);
         try {
-          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValue();
+          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValueExact();
           break;
-        } catch (NumberFormatException nfe) {
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Integer", str));
         }
 
@@ -226,9 +226,9 @@ public int decodeBinaryInt(ReadableByteBuf buf, int length, ColumnDefinitionPack
       case CHAR:
         String str = buf.readString(length);
         try {
-          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValue();
+          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValueExact();
           break;
-        } catch (NumberFormatException nfe) {
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Integer", str));
         }
 

src/main/java/com/singlestore/jdbc/codec/list/LocalDateTimeCodec.java

@@ -152,6 +152,13 @@ public LocalDateTime decodeText(
 
       case TIME:
         parts = LocalTimeCodec.parseTime(buf, length, column);
+        if (parts[0] == -1) {
+          return LocalDateTime.of(1970, 1, 1, 0, 0)
+              .minusHours(parts[1] % 24)
+              .minusMinutes(parts[2])
+              .minusSeconds(parts[3])
+              .minusNanos(parts[4]);
+        }
         return LocalDateTime.of(1970, 1, 1, parts[1] % 24, parts[2], parts[3]).plusNanos(parts[4]);
 
       case YEAR:
@@ -182,13 +189,23 @@ public LocalDateTime decodeBinary(
     switch (column.getType()) {
       case TIME:
         // specific case for TIME, to handle value not in 00:00:00-23:59:59
-        buf.skip(5); // skip negative and days
+        boolean negate = buf.readByte() == 1;
+        int day = buf.readInt();
         hour = buf.readByte();
         minutes = buf.readByte();
         seconds = buf.readByte();
         if (length > 8) {
           microseconds = buf.readUnsignedInt();
         }
+
+        if (negate) {
+          return LocalDateTime.of(1970, 1, 1, 0, 0)
+              .minusDays(day)
+              .minusHours(hour)
+              .minusMinutes(minutes)
+              .minusSeconds(seconds)
+              .minusNanos(microseconds * 1000);
+        }
         break;
 
       case BLOB:

src/main/java/com/singlestore/jdbc/codec/list/LongCodec.java

@@ -124,8 +124,8 @@ public long decodeTextLong(ReadableByteBuf buf, int length, ColumnDefinitionPack
       case DECIMAL:
         String str2 = buf.readAscii(length);
         try {
-          return new BigDecimal(str2).setScale(0, RoundingMode.DOWN).longValue();
-        } catch (NumberFormatException nfe) {
+          return new BigDecimal(str2).setScale(0, RoundingMode.DOWN).longValueExact();
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Long", str2));
         }
 
@@ -246,7 +246,7 @@ public long decodeBinaryLong(ReadableByteBuf buf, int length, ColumnDefinitionPa
         String str = buf.readString(length);
         try {
           return new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValueExact();
-        } catch (NumberFormatException nfe) {
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Long", str));
         }
 

src/main/java/com/singlestore/jdbc/codec/list/ShortCodec.java

@@ -109,9 +109,9 @@ public short decodeTextShort(ReadableByteBuf buf, int length, ColumnDefinitionPa
       case CHAR:
         String str = buf.readString(length);
         try {
-          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValue();
+          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValueExact();
           break;
-        } catch (NumberFormatException nfe) {
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Short", str));
         }
 
@@ -203,9 +203,9 @@ public short decodeBinaryShort(ReadableByteBuf buf, int length, ColumnDefinition
       case CHAR:
         String str = buf.readString(length);
         try {
-          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValue();
+          result = new BigDecimal(str).setScale(0, RoundingMode.DOWN).longValueExact();
           break;
-        } catch (NumberFormatException nfe) {
+        } catch (NumberFormatException | ArithmeticException nfe) {
           throw new SQLDataException(String.format("value '%s' cannot be decoded as Short", str));
         }
 

src/main/java/com/singlestore/jdbc/codec/list/TimestampCodec.java

@@ -319,22 +319,33 @@ public Timestamp decodeBinary(
   public void encodeText(
       PacketWriter encoder, Context context, Object val, Calendar providedCal, Long maxLen)
       throws IOException {
+    Timestamp ts = (Timestamp) val;
     Calendar cal = providedCal == null ? Calendar.getInstance() : providedCal;
-    SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
+    SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
     sdf.setTimeZone(cal.getTimeZone());
-    String dateString = sdf.format(val);
+    String dateString = sdf.format(ts);
 
     encoder.writeByte('\'');
     encoder.writeAscii(dateString);
+    int microseconds = ts.getNanos() / 1000;
+    if (microseconds > 0) {
+      if (microseconds % 1000 == 0) {
+        encoder.writeAscii("." + Integer.toString(microseconds / 1000 + 1000).substring(1));
+      } else {
+        encoder.writeAscii("." + Integer.toString(microseconds + 1000000).substring(1));
+      }
+    }
     encoder.writeByte('\'');
   }
 
   @Override
   public void encodeBinary(PacketWriter encoder, Object value, Calendar providedCal, Long maxLength)
       throws IOException {
+    Timestamp ts = (Timestamp) value;
     Calendar cal = providedCal == null ? Calendar.getInstance() : providedCal;
-    cal.setTimeInMillis(((Timestamp) value).getTime());
-    if (cal.get(Calendar.MILLISECOND) == 0) {
+    cal.setTimeInMillis(ts.getTime());
+
+    if (ts.getNanos() == 0) {
       encoder.writeByte(7); // length
       encoder.writeShort((short) cal.get(Calendar.YEAR));
       encoder.writeByte((cal.get(Calendar.MONTH) + 1));
@@ -350,7 +361,7 @@ public void encodeBinary(PacketWriter encoder, Object value, Calendar providedCa
       encoder.writeByte(cal.get(Calendar.HOUR_OF_DAY));
       encoder.writeByte(cal.get(Calendar.MINUTE));
       encoder.writeByte(cal.get(Calendar.SECOND));
-      encoder.writeInt(cal.get(Calendar.MILLISECOND) * 1000);
+      encoder.writeInt(ts.getNanos() / 1000);
     }
   }
 

src/main/java/com/singlestore/jdbc/message/server/ColumnDefinitionPacket.java

@@ -433,7 +433,7 @@ public Codec<?> getDefaultCodec(Configuration conf) {
       case MEDIUMBLOB:
       case LONGBLOB:
       case BLOB:
-        return isBinary() ? BlobCodec.INSTANCE : ClobCodec.INSTANCE;
+        return isBinary() ? BlobCodec.INSTANCE : StringCodec.INSTANCE;
       case TIME:
         return TimeCodec.INSTANCE;
       case YEAR:

src/main/java/com/singlestore/jdbc/pool/InternalPoolConnection.java

@@ -8,7 +8,6 @@
 import com.singlestore.jdbc.Connection;
 import com.singlestore.jdbc.SingleStorePoolConnection;
 import java.util.concurrent.atomic.AtomicLong;
-import javax.sql.*;
 
 public class InternalPoolConnection extends SingleStorePoolConnection {
   private final AtomicLong lastUsed;
@@ -23,10 +22,6 @@ public InternalPoolConnection(Connection connection) {
     lastUsed = new AtomicLong(System.nanoTime());
   }
 
-  public void close() {
-    fireConnectionClosed(new ConnectionEvent(this));
-  }
-
   /**
    * Indicate last time this pool connection has been used.
    *

src/test/java/com/singlestore/jdbc/integration/PooledConnectionTest.java

@@ -39,7 +39,7 @@ public void testPooledConnectionClosed() throws Exception {
     MyEventListener listener = new MyEventListener();
     pc.addConnectionEventListener(listener);
     pc.addStatementEventListener(listener);
-    connection.close();
+    pc.close();
     assertTrue(listener.closed);
     assertThrows(SQLException.class, () -> connection.createStatement().execute("select 1"));
     pc.removeConnectionEventListener(listener);

src/test/java/com/singlestore/jdbc/integration/StatementTest.java

@@ -67,6 +67,28 @@ public void getConnection() throws SQLException {
     assertEquals(ResultSet.HOLD_CURSORS_OVER_COMMIT, stmt.getResultSetHoldability());
   }
 
+  @Test
+  public void ensureGetGeneratedKeysReturnsEmptyResult() throws SQLException {
+    Statement stmt = sharedConn.createStatement();
+    stmt.execute("CREATE TABLE IF NOT EXISTS key_test (id INT(11) NOT NULL)");
+    try (PreparedStatement ps =
+        sharedConn.prepareStatement(
+            "INSERT INTO key_test(id) VALUES(5)", Statement.RETURN_GENERATED_KEYS)) {
+      ps.execute();
+      ResultSet rs = ps.getGeneratedKeys();
+      assertFalse(rs.next());
+    }
+    try (PreparedStatement ps =
+        sharedConn.prepareStatement(
+            "UPDATE key_test set id=7 WHERE id=5", Statement.RETURN_GENERATED_KEYS)) {
+      ps.execute();
+      ResultSet rs = ps.getGeneratedKeys();
+      assertFalse(rs.next());
+    }
+
+    stmt.execute("DROP TABLE key_test");
+  }
+
   @Test
   public void execute() throws SQLException {
     Statement stmt = sharedConn.createStatement();

src/test/java/com/singlestore/jdbc/integration/codec/ClobCodecTest.java

@@ -87,14 +87,12 @@ public void getObjectPrepare() throws SQLException {
   }
 
   public void getObject(ResultSet rs) throws SQLException {
-    assertEquals(new SingleStoreClob("0".getBytes()), rs.getObject(1));
+    assertEquals("0", rs.getObject(1));
     assertFalse(rs.wasNull());
-    assertEquals(new SingleStoreClob("1".getBytes()), rs.getObject(2));
-    assertEquals(new SingleStoreClob("1".getBytes()), rs.getObject("t2alias"));
+    assertEquals("1", rs.getObject(2));
+    assertEquals("1", rs.getObject("t2alias"));
     assertFalse(rs.wasNull());
-    assertEquals(
-        new SingleStoreClob(("some" + fourByteUnicode).getBytes(StandardCharsets.UTF_8)),
-        rs.getObject(3));
+    assertEquals(("some" + fourByteUnicode), rs.getObject(3));
     assertFalse(rs.wasNull());
     assertNull(rs.getObject(4));
     assertTrue(rs.wasNull());
@@ -724,7 +722,7 @@ public void getMetaData() throws SQLException {
     ResultSetMetaData meta = rs.getMetaData();
     assertEquals("TINYTEXT", meta.getColumnTypeName(1));
     assertEquals(sharedConn.getCatalog(), meta.getCatalogName(1));
-    assertEquals("java.sql.Clob", meta.getColumnClassName(1));
+    assertEquals("java.lang.String", meta.getColumnClassName(1));
     assertEquals("t1alias", meta.getColumnLabel(1));
     assertEquals("t1", meta.getColumnName(1));
     assertEquals(Types.VARCHAR, meta.getColumnType(1));

src/test/java/com/singlestore/jdbc/integration/codec/DateTimeCodecTest.java

@@ -56,7 +56,7 @@ public static void beforeAll2() throws SQLException {
         "CREATE TABLE DateTimeCodec (t1 DATETIME , t2 DATETIME(6), t3 DATETIME(6), t4 DATETIME(6), id INT)");
     stmt.execute(
         "INSERT INTO DateTimeCodec VALUES "
-            + "('2010-01-12 01:55:12', '1000-01-01 01:55:13.2', '9999-12-31 18:30:12.55', null, 1)");
+            + "('2010-01-12 01:55:12', '1000-01-01 01:55:13.212345', '9999-12-31 18:30:12.55', null, 1)");
     stmt.execute(
         createRowstore()
             + " TABLE DateTimeCodec2 (id int not null primary key auto_increment, t1 DATETIME(6))");
@@ -102,7 +102,7 @@ public void getObject(ResultSet rs) throws SQLException {
         ((Timestamp) rs.getObject(1)).getTime());
     assertFalse(rs.wasNull());
     assertEquals(
-        Timestamp.valueOf("1000-01-01 01:55:13.2").getTime(),
+        Timestamp.valueOf("1000-01-01 01:55:13.212345").getTime(),
         ((Timestamp) rs.getObject(2)).getTime());
     assertFalse(rs.wasNull());
     assertEquals(
@@ -166,8 +166,8 @@ public void getStringPrepare() throws SQLException {
   public void getString(ResultSet rs) throws SQLException {
     assertEquals("2010-01-12 01:55:12", rs.getString(1));
     assertFalse(rs.wasNull());
-    assertEquals("1000-01-01 01:55:13.200000", rs.getString(2));
-    assertEquals("1000-01-01 01:55:13.200000", rs.getString("t2alias"));
+    assertEquals("1000-01-01 01:55:13.212345", rs.getString(2));
+    assertEquals("1000-01-01 01:55:13.212345", rs.getString("t2alias"));
     assertFalse(rs.wasNull());
     assertEquals("9999-12-31 18:30:12.550000", rs.getString(3));
     assertFalse(rs.wasNull());
@@ -190,12 +190,12 @@ public void getNString(ResultSet rs) throws SQLException {
     assertEquals("2010-01-12 01:55:12", rs.getNString(1));
     assertFalse(rs.wasNull());
     String s = rs.getNString(2);
-    assertTrue(s.equals("1000-01-01 01:55:13.200000") || s.equals("1000-01-01 01:55:13.200"));
+    assertTrue(s.equals("1000-01-01 01:55:13.212345"));
     s = rs.getNString("t2alias");
-    assertTrue(s.equals("1000-01-01 01:55:13.200000") || s.equals("1000-01-01 01:55:13.200"));
+    assertTrue(s.equals("1000-01-01 01:55:13.212345"));
     assertFalse(rs.wasNull());
     s = rs.getNString(3);
-    assertTrue(s.equals("9999-12-31 18:30:12.550000") || s.equals("9999-12-31 18:30:12.550"));
+    assertTrue(s.equals("9999-12-31 18:30:12.550000"));
     assertFalse(rs.wasNull());
     assertNull(rs.getNString(4));
     assertTrue(rs.wasNull());
@@ -448,9 +448,9 @@ public void getTime(ResultSet rs) throws SQLException {
     assertFalse(rs.wasNull());
 
     assertEquals(
-        6913200, rs.getTime(2, Calendar.getInstance(TimeZone.getTimeZone("UTC"))).getTime());
+        6913212, rs.getTime(2, Calendar.getInstance(TimeZone.getTimeZone("UTC"))).getTime());
     assertFalse(rs.wasNull());
-    assertEquals(Time.valueOf("01:55:13").getTime() + 200, rs.getTime(2).getTime());
+    assertEquals(Time.valueOf("01:55:13").getTime() + 212, rs.getTime(2).getTime());
     assertFalse(rs.wasNull());
     assertEquals(Time.valueOf("18:30:12").getTime() + 550, rs.getTime(3).getTime());
     assertFalse(rs.wasNull());
@@ -471,7 +471,7 @@ public void getDurationPrepare() throws SQLException {
 
   public void getDuration(ResultSet rs) throws SQLException {
     assertEquals(Duration.parse("PT265H55M12S"), rs.getObject(1, Duration.class));
-    assertEquals(Duration.parse("PT1H55M13.2S"), rs.getObject(2, Duration.class));
+    assertEquals(Duration.parse("PT1H55M13.212345S"), rs.getObject(2, Duration.class));
     assertNull(rs.getObject(4, Duration.class));
   }
 
@@ -489,7 +489,7 @@ public void getLocalTimePrepare() throws SQLException {
   public void getLocalTime(ResultSet rs) throws SQLException {
     assertEquals(LocalTime.parse("01:55:12"), rs.getObject(1, LocalTime.class));
     assertFalse(rs.wasNull());
-    assertEquals(LocalTime.parse("01:55:13.2"), rs.getObject(2, LocalTime.class));
+    assertEquals(LocalTime.parse("01:55:13.212345"), rs.getObject(2, LocalTime.class));
     assertFalse(rs.wasNull());
     assertEquals(LocalTime.parse("18:30:12.55"), rs.getObject(3, LocalTime.class));
     assertFalse(rs.wasNull());
@@ -535,7 +535,7 @@ public void getTimestamp(ResultSet rs) throws SQLException {
     assertEquals(Timestamp.valueOf("2010-01-12 01:55:12").getTime(), rs.getTimestamp(1).getTime());
     assertFalse(rs.wasNull());
     assertEquals(
-        Timestamp.valueOf("1000-01-01 01:55:13.2").getTime(), rs.getTimestamp(2).getTime());
+        Timestamp.valueOf("1000-01-01 01:55:13.212345").getTime(), rs.getTimestamp(2).getTime());
     assertFalse(rs.wasNull());
     assertEquals(
         Timestamp.valueOf("9999-12-31 18:30:12.55").getTime(), rs.getTimestamp(3).getTime());