[CONJ-923] correctly return 64 bits generated id / updated rows

mariadb-corporation · Feb 14, 2022 · c999c2e · c999c2e
1 parent fdc448d
commit c999c2e
Show file tree

Hide file tree

Showing 12 changed files with 89 additions and 36 deletions.
diff --git a/src/main/java/org/mariadb/jdbc/client/ReadableByteBuf.java b/src/main/java/org/mariadb/jdbc/client/ReadableByteBuf.java
@@ -92,7 +92,17 @@ public interface ReadableByteBuf {
    *
    * @return encoded length
    */
-  int readLengthNotNull();
+  long readLongLengthEncodedNotNull();
+
+  /**
+   * Read encoded length value that cannot be null see
+   * https://mariadb.com/kb/en/protocol-data-types/#length-encoded-integers
+   *
+   * <p>this is readLongLengthEncodedNotNull limited to 32 bits </p>
+   *
+   * @return encoded length
+   */
+  int readIntLengthEncodedNotNull();
 
   /**
    * Utility to skip length encoded string, returning initial position

diff --git a/src/main/java/org/mariadb/jdbc/client/impl/ConnectionHelper.java b/src/main/java/org/mariadb/jdbc/client/impl/ConnectionHelper.java
@@ -307,8 +307,8 @@ public static void authenticationHandler(
           // see https://mariadb.com/kb/en/library/ok_packet/
           // *************************************************************************************
           buf.skip(); // 0x00 OkPacket Header
-          buf.skip(buf.readLengthNotNull()); // affectedRows
-          buf.skip(buf.readLengthNotNull());
+          buf.readLongLengthEncodedNotNull(); // skip affectedRows
+          buf.readLongLengthEncodedNotNull(); // skip insert id
           // insertId
           context.setServerStatus(buf.readShort());
           break authentication_loop;

diff --git a/src/main/java/org/mariadb/jdbc/client/impl/StandardReadableByteBuf.java b/src/main/java/org/mariadb/jdbc/client/impl/StandardReadableByteBuf.java
@@ -74,7 +74,21 @@ public short getUnsignedByte() {
     return (short) (buf[pos] & 0xff);
   }
 
-  public int readLengthNotNull() {
+  public long readLongLengthEncodedNotNull() {
+    int type = (buf[pos++] & 0xff);
+    switch (type) {
+      case 252:
+        return readUnsignedShort();
+      case 253:
+        return readUnsignedMedium();
+      case 254:
+        return readLong();
+      default:
+        return type;
+    }
+  }
+
+  public int readIntLengthEncodedNotNull() {
     int type = (buf[pos++] & 0xff);
     switch (type) {
       case 252:
@@ -204,7 +218,7 @@ public byte[] readBytesNullEnd() {
   }
 
   public StandardReadableByteBuf readLengthBuffer() {
-    int len = readLengthNotNull();
+    int len = this.readIntLengthEncodedNotNull();
     byte[] tmp = new byte[len];
     readBytes(tmp);
     return new StandardReadableByteBuf(tmp, len);

diff --git a/src/main/java/org/mariadb/jdbc/client/result/Result.java b/src/main/java/org/mariadb/jdbc/client/result/Result.java
@@ -176,8 +176,8 @@ protected boolean readNext() throws IOException, SQLException {
             serverStatus = readBuf.readUnsignedShort();
           } else {
             // OK_Packet with a 0xFE header
-            readBuf.skip(readBuf.readLengthNotNull()); // skip update count
-            readBuf.skip(readBuf.readLengthNotNull()); // skip insert id
+            readBuf.readLongLengthEncodedNotNull(); // skip update count
+            readBuf.readLongLengthEncodedNotNull(); // skip insert id
             serverStatus = readBuf.readUnsignedShort();
             warnings = readBuf.readUnsignedShort();
           }
@@ -230,8 +230,8 @@ protected void skipRemaining() throws IOException, SQLException {
               serverStatus = buf.readUnsignedShort();
             } else {
               // OK_Packet with a 0xFE header
-              buf.skip(buf.readLengthNotNull()); // skip update count
-              buf.skip(buf.readLengthNotNull()); // skip insert id
+              buf.readLongLengthEncodedNotNull(); // skip update count
+              buf.readLongLengthEncodedNotNull(); // skip insert id
               serverStatus = buf.readUnsignedShort();
               warnings = buf.readUnsignedShort();
             }

diff --git a/src/main/java/org/mariadb/jdbc/message/ClientMessage.java b/src/main/java/org/mariadb/jdbc/message/ClientMessage.java
@@ -182,7 +182,7 @@ default Completion readPacket(
         // * ResultSet
         // *********************************************************************************************************
       default:
-        int fieldCount = buf.readLengthNotNull();
+        int fieldCount = buf.readIntLengthEncodedNotNull();
 
         Column[] ci;
         boolean canSkipMeta = context.canSkipMeta() && this.canSkipMeta();

diff --git a/src/main/java/org/mariadb/jdbc/message/server/ColumnDefinitionPacket.java b/src/main/java/org/mariadb/jdbc/message/server/ColumnDefinitionPacket.java
@@ -46,7 +46,7 @@ private ColumnDefinitionPacket(
     this.length = length;
     this.dataType = dataType;
     this.decimals = (byte) 0;
-    this.flags = ColumnFlags.PRIMARY_KEY;
+    this.flags = ColumnFlags.AUTO_INCREMENT | ColumnFlags.UNSIGNED;
     this.stringPos = stringPos;
     this.extTypeName = null;
   }
@@ -152,27 +152,27 @@ public static ColumnDefinitionPacket create(String name, DataType type) {
 
   public String getSchema() {
     buf.pos(stringPos[0]);
-    return buf.readString(buf.readLengthNotNull());
+    return buf.readString(buf.readIntLengthEncodedNotNull());
   }
 
   public String getTableAlias() {
     buf.pos(stringPos[1]);
-    return buf.readString(buf.readLengthNotNull());
+    return buf.readString(buf.readIntLengthEncodedNotNull());
   }
 
   public String getTable() {
     buf.pos(stringPos[useAliasAsName ? 1 : 2]);
-    return buf.readString(buf.readLengthNotNull());
+    return buf.readString(buf.readIntLengthEncodedNotNull());
   }
 
   public String getColumnAlias() {
     buf.pos(stringPos[3]);
-    return buf.readString(buf.readLengthNotNull());
+    return buf.readString(buf.readIntLengthEncodedNotNull());
   }
 
   public String getColumnName() {
     buf.pos(stringPos[4]);
-    return buf.readString(buf.readLengthNotNull());
+    return buf.readString(buf.readIntLengthEncodedNotNull());
   }
 
   public long getLength() {

diff --git a/src/main/java/org/mariadb/jdbc/message/server/OkPacket.java b/src/main/java/org/mariadb/jdbc/message/server/OkPacket.java
@@ -27,35 +27,35 @@ public class OkPacket implements Completion {
    */
   public OkPacket(ReadableByteBuf buf, Context context) {
     buf.skip(); // ok header
-    this.affectedRows = buf.readLengthNotNull();
-    this.lastInsertId = buf.readLengthNotNull();
+    this.affectedRows = buf.readLongLengthEncodedNotNull();
+    this.lastInsertId = buf.readLongLengthEncodedNotNull();
     context.setServerStatus(buf.readUnsignedShort());
     context.setWarning(buf.readUnsignedShort());
 
     if ((context.getServerCapabilities() & Capabilities.CLIENT_SESSION_TRACK) != 0
         && buf.readableBytes() > 0) {
-      buf.skip(buf.readLengthNotNull()); // skip info
+      buf.skip(buf.readIntLengthEncodedNotNull()); // skip info
       while (buf.readableBytes() > 0) {
-        if (buf.readLengthNotNull() > 0) {
+        if (buf.readIntLengthEncodedNotNull() > 0) {
           switch (buf.readByte()) {
             case StateChange.SESSION_TRACK_SYSTEM_VARIABLES:
-              buf.readLengthNotNull();
-              String variable = buf.readString(buf.readLengthNotNull());
+              buf.readIntLengthEncodedNotNull();
+              String variable = buf.readString(buf.readIntLengthEncodedNotNull());
               Integer len = buf.readLength();
               String value = len == null ? null : buf.readString(len);
               logger.debug("System variable change:  {} = {}", variable, value);
               break;
 
             case StateChange.SESSION_TRACK_SCHEMA:
-              buf.readLengthNotNull();
+              buf.readIntLengthEncodedNotNull();
               Integer dbLen = buf.readLength();
               String database = dbLen == null ? null : buf.readString(dbLen);
               context.setDatabase(database.isEmpty() ? null : database);
               logger.debug("Database change: is '{}'", database);
               break;
 
             default:
-              buf.skip(buf.readLengthNotNull());
+              buf.skip(buf.readIntLengthEncodedNotNull());
           }
         }
       }

diff --git a/src/main/java/org/mariadb/jdbc/plugin/authentication/standard/CachingSha2PasswordPlugin.java b/src/main/java/org/mariadb/jdbc/plugin/authentication/standard/CachingSha2PasswordPlugin.java
@@ -115,7 +115,7 @@ public ReadableByteBuf process(Writer out, Reader in, Context context)
 
       default:
         // fast authentication result
-        byte[] authResult = new byte[buf.readLengthNotNull()];
+        byte[] authResult = new byte[buf.readIntLengthEncodedNotNull()];
         buf.readBytes(authResult);
         switch (authResult[0]) {
           case 3:

diff --git a/src/main/java/org/mariadb/jdbc/plugin/codec/IntCodec.java b/src/main/java/org/mariadb/jdbc/plugin/codec/IntCodec.java
@@ -87,14 +87,8 @@ public int decodeTextInt(final ReadableByteBuf buf, final int length, final Colu
         return (int) LongCodec.parseNotEmpty(buf, length);
 
       case INTEGER:
-        result = LongCodec.parseNotEmpty(buf, length);
-        break;
-
       case BIGINT:
         result = LongCodec.parseNotEmpty(buf, length);
-        if (result < 0 & !column.isSigned()) {
-          throw new SQLDataException("int overflow");
-        }
         break;
 
       case BIT:
@@ -140,7 +134,7 @@ public int decodeTextInt(final ReadableByteBuf buf, final int length, final Colu
     }
 
     int res = (int) result;
-    if (res != result) {
+    if (res != result || (result < 0 && !column.isSigned())) {
       throw new SQLDataException("integer overflow");
     }
     return res;

diff --git a/src/main/java/org/mariadb/jdbc/plugin/codec/ShortCodec.java b/src/main/java/org/mariadb/jdbc/plugin/codec/ShortCodec.java
@@ -177,9 +177,6 @@ public short decodeBinaryShort(ReadableByteBuf buf, int length, Column column)
 
       case BIGINT:
         result = buf.readLong();
-        if (result < 0 & !column.isSigned()) {
-          throw new SQLDataException("int overflow");
-        }
         break;
 
       case BIT:

diff --git a/src/test/java/org/mariadb/jdbc/integration/StatementTest.java b/src/test/java/org/mariadb/jdbc/integration/StatementTest.java
@@ -6,12 +6,15 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 
+import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.sql.*;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import org.junit.jupiter.api.*;
 import org.mariadb.jdbc.Connection;
 import org.mariadb.jdbc.Statement;
+import org.mariadb.jdbc.client.result.CompleteResult;
 
 public class StatementTest extends Common {
 
@@ -22,6 +25,7 @@ public static void drop() throws SQLException {
     stmt.execute("DROP TABLE IF EXISTS executeGenerated");
     stmt.execute("DROP TABLE IF EXISTS executeGenerated2");
     stmt.execute("DROP TABLE IF EXISTS testAffectedRow");
+    stmt.execute("DROP TABLE IF EXISTS bigIntId");
   }
 
   @BeforeAll
@@ -34,9 +38,43 @@ public static void beforeAll2() throws SQLException {
     stmt.execute(
         "CREATE TABLE executeGenerated2 (t1 int not null primary key auto_increment, t2 int)");
     stmt.execute("CREATE TABLE testAffectedRow(id int)");
+    stmt.execute(
+        "CREATE TABLE bigIntId(`id` bigint(20) unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, val VARCHAR(256))");
     stmt.execute("FLUSH TABLES");
   }
 
+  @Test
+  public void longGeneratedId() throws SQLException {
+    longGeneratedId(BigInteger.ONE);
+    longGeneratedId(BigInteger.valueOf(Integer.MAX_VALUE));
+    longGeneratedId(BigInteger.valueOf(4294967295L));
+    longGeneratedId(BigInteger.valueOf(Long.MAX_VALUE));
+  }
+
+  public void longGeneratedId(BigInteger expected) throws SQLException {
+    Statement stmt = sharedConn.createStatement();
+    stmt.execute("ALTER TABLE bigIntId AUTO_INCREMENT=" + expected.toString());
+    stmt.execute(
+        "INSERT INTO bigIntId(val) value ('est')", java.sql.Statement.RETURN_GENERATED_KEYS);
+    ResultSet rs = stmt.getGeneratedKeys();
+    assertTrue(rs.next());
+
+    if (expected.compareTo(BigInteger.valueOf(Integer.MAX_VALUE)) >= 1) {
+      assertThrowsContains(SQLDataException.class, () -> rs.getInt(1), "integer overflow");
+    } else {
+      assertEquals(expected.intValueExact(), rs.getInt(1));
+    }
+
+    if (expected.compareTo(BigInteger.valueOf(Long.MAX_VALUE)) >= 1) {
+      assertThrowsContains(
+          SQLDataException.class, () -> rs.getLong(1), "cannot be decoded as Long");
+    } else {
+      assertEquals(expected.longValueExact(), rs.getLong(1));
+    }
+    assertTrue(expected.compareTo(((CompleteResult) rs).getBigInteger(1)) == 0);
+    assertTrue(new BigDecimal(expected).compareTo(rs.getBigDecimal(1)) == 0);
+  }
+
   @Test
   public void getConnection() throws SQLException {
     Statement stmt = sharedConn.createStatement();

diff --git a/src/test/java/org/mariadb/jdbc/integration/codec/LongCodecTest.java b/src/test/java/org/mariadb/jdbc/integration/codec/LongCodecTest.java
@@ -349,7 +349,7 @@ private void getIntUnsigned(ResultSet rs) throws SQLException {
       fail();
     } catch (SQLDataException e) {
       assertTrue(
-          e.getMessage().contains("int overflow")
+          e.getMessage().contains("integer overflow")
               || e.getMessage()
                   .contains("value '18446744073709551615' cannot be decoded as Integer"));
     }