You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by da...@apache.org on 2012/01/09 11:18:06 UTC

svn commit: r1229082 - in /db/derby/code/trunk/java: client/org/apache/derby/client/am/Cursor.java client/org/apache/derby/client/am/Decimal.java testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java

Author: dag
Date: Mon Jan  9 10:18:05 2012
New Revision: 1229082

URL: http://svn.apache.org/viewvc?rev=1229082&view=rev
Log:
DERBY-5536 Client's ResultSet#getLong does not range check when converting from a DECIMAL column

Patch derby-5536-3, which fixes this issue and adds new tests.  It
changes the implementation of am.Decimal#getLong to allow it to detect
overflow. If the number of digits indicate it can't happen, we use an
optimized path.


Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Cursor.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Decimal.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Cursor.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Cursor.java?rev=1229082&r1=1229081&r2=1229082&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Cursor.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Cursor.java Mon Jan  9 10:18:05 2012
@@ -392,20 +392,25 @@ public abstract class Cursor {
     }
 
     // Build a Java long from a fixed point decimal byte representation.
-    private final long getLongFromDECIMAL(int column) throws SqlException {
+    private final long getLongFromDECIMAL(int column, String targetType) 
+            throws SqlException {
         try {
             return org.apache.derby.client.am.Decimal.getLong(dataBuffer_,
                     columnDataPosition_[column - 1],
                     getColumnPrecision(column - 1),
                     getColumnScale(column - 1));
+        } catch (ArithmeticException e) {
+            throw new SqlException(agent_.logWriter_,
+                new ClientMessageId (SQLState.LANG_OUTSIDE_RANGE_FOR_DATATYPE),
+                targetType, e);
         } catch (java.lang.IllegalArgumentException e) {
             throw new SqlException(agent_.logWriter_,
                 new ClientMessageId (SQLState.LANG_OUTSIDE_RANGE_FOR_DATATYPE),
-                "long", e);
+                targetType, e);
         } catch (java.io.UnsupportedEncodingException e) {
             throw new SqlException(agent_.logWriter_,
                 new ClientMessageId (SQLState.UNSUPPORTED_ENCODING), 
-                "DECIMAL", "long", e);
+                "DECIMAL", targetType, e);
         }
     }
 
@@ -739,7 +744,8 @@ public abstract class Cursor {
             return agent_.crossConverters_.getBooleanFromDouble(get_DOUBLE(column));
         case java.sql.Types.DECIMAL:
             // For performance we don't materialize the BigDecimal, but convert directly from decimal bytes to a long.
-            return agent_.crossConverters_.getBooleanFromLong(getLongFromDECIMAL(column));
+            return agent_.crossConverters_.getBooleanFromLong(
+                getLongFromDECIMAL(column, "boolean"));
         case java.sql.Types.CHAR:
             return agent_.crossConverters_.getBooleanFromString(getCHAR(column));
         case java.sql.Types.VARCHAR:
@@ -767,7 +773,8 @@ public abstract class Cursor {
             return agent_.crossConverters_.getByteFromDouble(get_DOUBLE(column));
         case java.sql.Types.DECIMAL:
             // For performance we don't materialize the BigDecimal, but convert directly from decimal bytes to a long.
-            return agent_.crossConverters_.getByteFromLong(getLongFromDECIMAL(column));
+            return agent_.crossConverters_.getByteFromLong(
+                getLongFromDECIMAL(column, "byte"));
         case java.sql.Types.CHAR:
             return agent_.crossConverters_.getByteFromString(getCHAR(column));
         case java.sql.Types.VARCHAR:
@@ -794,7 +801,8 @@ public abstract class Cursor {
             return agent_.crossConverters_.getShortFromDouble(get_DOUBLE(column));
         case java.sql.Types.DECIMAL:
             // For performance we don't materialize the BigDecimal, but convert directly from decimal bytes to a long.
-            return agent_.crossConverters_.getShortFromLong(getLongFromDECIMAL(column));
+            return agent_.crossConverters_.getShortFromLong(
+                getLongFromDECIMAL(column, "short"));
         case java.sql.Types.CHAR:
             return agent_.crossConverters_.getShortFromString(getCHAR(column));
         case java.sql.Types.VARCHAR:
@@ -821,7 +829,8 @@ public abstract class Cursor {
             return agent_.crossConverters_.getIntFromDouble(get_DOUBLE(column));
         case java.sql.Types.DECIMAL:
             // For performance we don't materialize the BigDecimal, but convert directly from decimal bytes to a long.
-            return agent_.crossConverters_.getIntFromLong(getLongFromDECIMAL(column));
+            return agent_.crossConverters_.getIntFromLong(
+                getLongFromDECIMAL(column, "int"));
         case java.sql.Types.CHAR:
             return agent_.crossConverters_.getIntFromString(getCHAR(column));
         case java.sql.Types.VARCHAR:
@@ -848,7 +857,7 @@ public abstract class Cursor {
             return agent_.crossConverters_.getLongFromDouble(get_DOUBLE(column));
         case java.sql.Types.DECIMAL:
             // For performance we don't materialize the BigDecimal, but convert directly from decimal bytes to a long.
-            return getLongFromDECIMAL(column);
+            return getLongFromDECIMAL(column, "long");
         case java.sql.Types.CHAR:
             return agent_.crossConverters_.getLongFromString(getCHAR(column));
         case java.sql.Types.VARCHAR:

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Decimal.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Decimal.java?rev=1229082&r1=1229081&r2=1229082&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Decimal.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Decimal.java Mon Jan  9 10:18:05 2012
@@ -20,6 +20,7 @@
 */
 package org.apache.derby.client.am;
 
+import java.math.BigDecimal;
 import org.apache.derby.shared.common.reference.SQLState;
 import org.apache.derby.shared.common.i18n.MessageUtil;
 
@@ -320,6 +321,7 @@ public class Decimal {
      * Build a Java <code>long</code> from a fixed point decimal byte representation.
      *
      * @throws IllegalArgumentException if the specified representation is not recognized.
+     * @throws ArithmeticException if value is too large for a long
      */
     public static final long getLong(byte[] buffer,
                                      int offset,
@@ -342,22 +344,20 @@ public class Decimal {
             signum = 1;
         }
 
-        // compute the integer part only.
-        int leftOfDecimalPoint = length * 2 - 1 - scale;
-        long integer = 0;
-        if (leftOfDecimalPoint > 0) {
-            int i = 0;
-            for (; i < leftOfDecimalPoint / 2; i++) {
-                integer = integer * 10 + signum * ((buffer[offset + i] & 0xF0) >>> 4); // high nybble.
-                integer = integer * 10 + signum * (buffer[offset + i] & 0x0F);        // low nybble.
-            }
-            if ((leftOfDecimalPoint % 2) == 1) {
-                // process high nybble of the last byte if necessary.
-                integer = integer * 10 + signum * ((buffer[offset + i] & 0xF0) >>> 4);
-            }
+        if (precision - scale <= 18) {
+            // Can be handled by long without overflow.
+            // Compute the integer part only.
+            int leftOfDecimalPoint = length * 2 - 1 - scale;
+            return signum * packedNybblesToLong(buffer, offset, 0,
+                                                leftOfDecimalPoint);
+        } else {
+            // Strip off fraction part by converting via BigInteger
+            // lest longValueExact will throw ArithmeticException
+            BigDecimal tmp = new BigDecimal(
+                getBigDecimal(buffer, offset, precision, scale).toBigInteger());
+            // throws ArithmeticException if overflow:
+            return tmp.longValueExact();
         }
-
-        return integer;
     }
 
     //--------------entry points for runtime representation-----------------------

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java?rev=1229082&r1=1229081&r2=1229082&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/ParameterMappingTest.java Mon Jan  9 10:18:05 2012
@@ -42,6 +42,7 @@ import java.sql.Time;
 import java.sql.Timestamp;
 import java.sql.Types;
 import java.util.HashSet;
+import java.math.RoundingMode;
 
 import junit.framework.Test;
 import junit.framework.TestSuite;
@@ -4529,8 +4530,9 @@ public class ParameterMappingTest extend
         ps.setBoolean(11, true);
         ps.executeUpdate();
 
-        ResultSet rs = createStatement().
-                executeQuery("select * from MultiTypeTable");
+        PreparedStatement plainSelect = 
+                prepareStatement("select * from MultiTypeTable");
+        ResultSet rs = plainSelect.executeQuery();
         rs.next();
 
         // JDBC type -> byte
@@ -4561,22 +4563,84 @@ public class ParameterMappingTest extend
         assertGetState(rs, "F04", XXX_LONG, "22003");
         assertGetState(rs, "F05", XXX_LONG, "22003");
         assertGetState(rs, "F06", XXX_LONG, "22003");
+        assertGetState(rs, "F07", XXX_LONG, "22003");
+        rs.close();
 
-        // Uncomment when DERBY-5536 is fixed
-        // assertGetState(rs, "F07", XXX_LONG, "22003");
+        // DERBY-5536: client driver change of implementation for getting long
+        // from DECIMAL, so check correctness for two cases: 1) value with 18
+        // decimal digits or less, and 2) value with more than 18 decimal
+        // digits. Reason: cross-over point in implementation; the smaller
+        // numbers use an optimized code path.  Also try with and without
+        // non-zero fraction to see what happens to the discarded fractional
+        // part (scale == 1): Conversions to long should round off in the
+        // direction of zero for both positive and negative numbers with a
+        // fractional part >< 0, cf. RoundingMode.DOWN used in the asserts
+        // below.
 
+        BigDecimal vBelow[] =
+            new BigDecimal[]{new BigDecimal(123456789012345678L),  // 18 digits
+                             new BigDecimal(-12345678901234567L)};
+
+        BigDecimal vAbove[] =
+            new BigDecimal[]{new BigDecimal(1234567890123456789L), // 19 digits
+                             new BigDecimal(-123456789012345678L)};
+
+        createStatement().executeUpdate(
+            "create table t5536(d1 decimal(19,1)," +
+            "                   d2 decimal(20,1))");
+        PreparedStatement ps5536 = prepareStatement(
+            "insert into t5536 values (?,?)");
+
+        for (int scale=0; scale < 2; scale++) {
+            for (int i=0; i < vBelow.length; i++) {
+                ps5536.setBigDecimal(
+                    1,
+                    new BigDecimal(vBelow[i].toBigInteger(), scale));
+                ps5536.setBigDecimal(
+                    2,
+                    new BigDecimal(vAbove[i].toBigInteger(), scale));
+
+                ps5536.execute();
+            }
+        }
+
+
+
+        rs = createStatement().executeQuery("select * from t5536");
+
+        BigDecimal divisor[] = {BigDecimal.ONE, BigDecimal.TEN};
+
+        for (int scale=0; scale < 2; scale++) {
+            for (int i=0; i < vBelow.length; i++) {
+                rs.next();
+
+                assertEquals(
+                    "round-trip conversion error",
+                    vBelow[i].divide(divisor[scale], RoundingMode.DOWN).
+                        longValue(),
+                    rs.getLong(1));
+                assertEquals(
+                    "round-trip conversion error",
+                    vAbove[i].divide(divisor[scale], RoundingMode.DOWN).
+                        longValue(),
+                    rs.getLong(2));
+            }
+        }
 
-        // JDBC type -> float
         rs.close();
-        Statement s = createStatement(ResultSet.TYPE_FORWARD_ONLY,
-                ResultSet.CONCUR_UPDATABLE);
-        rs = s.executeQuery("SELECT * FROM MultiTypeTable");
+
+
+        // JDBC type -> float
+        PreparedStatement uSelect = prepareStatement(
+            "SELECT * FROM MultiTypeTable",
+            ResultSet.TYPE_FORWARD_ONLY,
+            ResultSet.CONCUR_UPDATABLE);
+        rs = uSelect.executeQuery();
         rs.next();
         rs.updateDouble("F06", Float.MAX_VALUE * 10.0);
         rs.updateRow();
 
-        rs = createStatement().
-                executeQuery("select * from MultiTypeTable");
+        rs = plainSelect.executeQuery();
         rs.next();
 
         assertGetState(rs, "F06", XXX_FLOAT, "22003");
@@ -4601,8 +4665,8 @@ public class ParameterMappingTest extend
         ps.setBoolean(11, false);
         ps.executeUpdate();
 
-        rs = createStatement().
-                executeQuery("select * from MultiTypeTable");
+        rs = plainSelect.executeQuery();
+
         rs.next();
         // JDBC type -> byte
         assertGetState(rs, "F01", XXX_BYTE, "22003");
@@ -4635,15 +4699,13 @@ public class ParameterMappingTest extend
 
         // JDBC type -> float
         rs.close();
-        s = createStatement(ResultSet.TYPE_FORWARD_ONLY,
-                ResultSet.CONCUR_UPDATABLE);
-        rs = s.executeQuery("SELECT * FROM MultiTypeTable");
+
+        rs = uSelect.executeQuery();
         rs.next();
         rs.updateDouble("F06", -Float.MAX_VALUE * 10.0);
         rs.updateRow();
 
-        rs = createStatement().
-                executeQuery("select * from MultiTypeTable");
+        rs = plainSelect.executeQuery();
         rs.next();
 
         assertGetState(rs, "F06", XXX_FLOAT, "22003");