You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2019/08/15 13:21:43 UTC
[cassandra] branch trunk updated: Avoid result truncate in decimal
operations
This is an automated email from the ASF dual-hosted git repository.
benedict pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new d60e798 Avoid result truncate in decimal operations
d60e798 is described below
commit d60e7988736ed4358595e9c781b110a5bbb5f812
Author: Liudmila Kornilova <ko...@gmail.com>
AuthorDate: Thu Aug 8 17:36:19 2019 +0800
Avoid result truncate in decimal operations
add (+), subtract (-) and multiply (*) operations:
* before:
* precision of result used to be always 34 (see MathContext.DECIMAL128)
* after:
* precision (number of significant digits) of result is at most 10000.
If result exceeds given precision it will be rounded using HALF_UP mode
division (/) operation:
* before:
* precision used to be always 34 (see MathContext.DECIMAL128)
* after:
* expected scale is set to minimum precision (32) minus estimated position of first digit in quotient
* scale should be at least as big as maximum scale of operands
* scale should not be less than 32
* scale should not be bigger than 1000
* if actual quotient scale is bigger than calculated scale then result is rounded using HALF_UP mode
* trailing zeros are stripped
modulo (%) operation:
* before:
* ArithmeticException used to occur when implicit division produces number with precision bigger
than 34 (see MathContext.DECIMAL128)
* after:
* No exception
patch by Liudmila Kornilova; reviewed by Benedict Elliott Smith for CASSANDRA-15232
---
CHANGES.txt | 1 +
.../apache/cassandra/db/marshal/DecimalType.java | 29 +++++++++--
.../cql3/functions/OperationFctsTest.java | 56 +++++++++++++++-------
3 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 99c021e..a5d4378 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,7 @@
4.0
* Improve readability of Table metrics Virtual tables units (CASSANDRA-15194)
* Fix error with non-existent table for nodetool tablehistograms (CASSANDRA-14410)
+ * Avoid result truncation in decimal operations (CASSANDRA-15232)
* Catch non-IOException in FileUtils.close to make sure that all resources are closed (CASSANDRA-15225)
* Align load column in nodetool status output (CASSANDRA-14787)
* CassandraNetworkAuthorizer uses cached roles info (CASSANDRA-15089)
diff --git a/src/java/org/apache/cassandra/db/marshal/DecimalType.java b/src/java/org/apache/cassandra/db/marshal/DecimalType.java
index b98bf00..110dc0e 100644
--- a/src/java/org/apache/cassandra/db/marshal/DecimalType.java
+++ b/src/java/org/apache/cassandra/db/marshal/DecimalType.java
@@ -20,6 +20,7 @@ package org.apache.cassandra.db.marshal;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.math.MathContext;
+import java.math.RoundingMode;
import java.nio.ByteBuffer;
import org.apache.cassandra.cql3.CQL3Type;
@@ -34,6 +35,10 @@ import org.apache.cassandra.utils.ByteBufferUtil;
public class DecimalType extends NumberType<BigDecimal>
{
public static final DecimalType instance = new DecimalType();
+ private static final int MIN_SCALE = 32;
+ private static final int MIN_SIGNIFICANT_DIGITS = MIN_SCALE;
+ private static final int MAX_SCALE = 1000;
+ private static final MathContext MAX_PRECISION = new MathContext(10000);
DecimalType() {super(ComparisonType.CUSTOM);} // singleton
@@ -142,27 +147,41 @@ public class DecimalType extends NumberType<BigDecimal>
public ByteBuffer add(NumberType<?> leftType, ByteBuffer left, NumberType<?> rightType, ByteBuffer right)
{
- return decompose(leftType.toBigDecimal(left).add(rightType.toBigDecimal(right), MathContext.DECIMAL128));
+ return decompose(leftType.toBigDecimal(left).add(rightType.toBigDecimal(right), MAX_PRECISION));
}
public ByteBuffer substract(NumberType<?> leftType, ByteBuffer left, NumberType<?> rightType, ByteBuffer right)
{
- return decompose(leftType.toBigDecimal(left).subtract(rightType.toBigDecimal(right), MathContext.DECIMAL128));
+ return decompose(leftType.toBigDecimal(left).subtract(rightType.toBigDecimal(right), MAX_PRECISION));
}
public ByteBuffer multiply(NumberType<?> leftType, ByteBuffer left, NumberType<?> rightType, ByteBuffer right)
{
- return decompose(leftType.toBigDecimal(left).multiply(rightType.toBigDecimal(right), MathContext.DECIMAL128));
+ return decompose(leftType.toBigDecimal(left).multiply(rightType.toBigDecimal(right), MAX_PRECISION));
}
public ByteBuffer divide(NumberType<?> leftType, ByteBuffer left, NumberType<?> rightType, ByteBuffer right)
{
- return decompose(leftType.toBigDecimal(left).divide(rightType.toBigDecimal(right), MathContext.DECIMAL128));
+ BigDecimal leftOperand = leftType.toBigDecimal(left);
+ BigDecimal rightOperand = rightType.toBigDecimal(right);
+
+ // Predict position of first significant digit in the quotient.
+ // Note: it is possible to improve prediction accuracy by comparing first significant digits in operands
+ // but it requires additional computations so this step is omitted
+ int quotientFirstDigitPos = (leftOperand.precision() - leftOperand.scale()) - (rightOperand.precision() - rightOperand.scale());
+
+ int scale = MIN_SIGNIFICANT_DIGITS - quotientFirstDigitPos;
+ scale = Math.max(scale, leftOperand.scale());
+ scale = Math.max(scale, rightOperand.scale());
+ scale = Math.max(scale, MIN_SCALE);
+ scale = Math.min(scale, MAX_SCALE);
+
+ return decompose(leftOperand.divide(rightOperand, scale, RoundingMode.HALF_UP).stripTrailingZeros());
}
public ByteBuffer mod(NumberType<?> leftType, ByteBuffer left, NumberType<?> rightType, ByteBuffer right)
{
- return decompose(leftType.toBigDecimal(left).remainder(rightType.toBigDecimal(right), MathContext.DECIMAL128));
+ return decompose(leftType.toBigDecimal(left).remainder(rightType.toBigDecimal(right)));
}
public ByteBuffer negate(ByteBuffer input)
diff --git a/test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java b/test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
index d27b746..c8ee935 100644
--- a/test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
+++ b/test/unit/org/apache/cassandra/cql3/functions/OperationFctsTest.java
@@ -177,35 +177,35 @@ public class OperationFctsTest extends CQLTester
row((short) 0, (short) 1, 1, 2L, 2.75F, 3.25, BigInteger.valueOf(3), new BigDecimal("4.25")));
assertRows(execute("SELECT a / c, b / c, c / c, d / c, e / c, f / c, g / c, h / c FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
- row(0, 0, 1, 1L, 1.8333334F, 2.1666666666666665, BigInteger.valueOf(2), new BigDecimal("2.833333333333333333333333333333333")));
+ row(0, 0, 1, 1L, 1.8333334F, 2.1666666666666665, BigInteger.valueOf(2), new BigDecimal("2.83333333333333333333333333333333")));
assertRows(execute("SELECT a / d, b / d, c / d, d / d, e / d, f / d, g / d, h / d FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
row(0L, 0L, 0L, 1L, 1.375, 1.625, BigInteger.valueOf(1), new BigDecimal("2.125")));
assertRows(execute("SELECT a / e, b / e, c / e, d / e, e / e, f / e, g / e, h / e FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
- row(0.18181819F, 0.36363637F, 0.54545456F, 0.7272727272727273, 1.0F, 1.1818181818181819, new BigDecimal("1.272727272727272727272727272727273"), new BigDecimal("1.545454545454545454545454545454545")));
+ row(0.18181819F, 0.36363637F, 0.54545456F, 0.7272727272727273, 1.0F, 1.1818181818181819, new BigDecimal("1.27272727272727272727272727272727"), new BigDecimal("1.54545454545454545454545454545455")));
assertRows(execute("SELECT a / f, b / f, c / f, d / f, e / f, f / f, g / f, h / f FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
- row(0.15384615384615385, 0.3076923076923077, 0.46153846153846156, 0.6153846153846154, 0.8461538461538461, 1.0, new BigDecimal("1.076923076923076923076923076923077"), new BigDecimal("1.307692307692307692307692307692308")));
+ row(0.15384615384615385, 0.3076923076923077, 0.46153846153846156, 0.6153846153846154, 0.8461538461538461, 1.0, new BigDecimal("1.07692307692307692307692307692308"), new BigDecimal("1.30769230769230769230769230769231")));
assertRows(execute("SELECT a / g, b / g, c / g, d / g, e / g, f / g, g / g, h / g FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
row(BigInteger.valueOf(0),
BigInteger.valueOf(0),
BigInteger.valueOf(0),
BigInteger.valueOf(0),
- new BigDecimal("0.7857142857142857142857142857142857"),
- new BigDecimal("0.9285714285714285714285714285714286"),
+ new BigDecimal("0.78571428571428571428571428571429"),
+ new BigDecimal("0.92857142857142857142857142857143"),
BigInteger.valueOf(1),
- new BigDecimal("1.214285714285714285714285714285714")));
+ new BigDecimal("1.21428571428571428571428571428571")));
assertRows(execute("SELECT a / h, b / h, c / h, d / h, e / h, f / h, g / h, h / h FROM %s WHERE a = 1 AND b = 2 AND c = 3 / 1"),
- row(new BigDecimal("0.1176470588235294117647058823529412"),
- new BigDecimal("0.2352941176470588235294117647058824"),
- new BigDecimal("0.3529411764705882352941176470588235"),
- new BigDecimal("0.4705882352941176470588235294117647"),
- new BigDecimal("0.6470588235294117647058823529411765"),
- new BigDecimal("0.7647058823529411764705882352941176"),
- new BigDecimal("0.8235294117647058823529411764705882"),
+ row(new BigDecimal("0.11764705882352941176470588235294"),
+ new BigDecimal("0.23529411764705882352941176470588"),
+ new BigDecimal("0.35294117647058823529411764705882"),
+ new BigDecimal("0.47058823529411764705882352941176"),
+ new BigDecimal("0.64705882352941176470588235294118"),
+ new BigDecimal("0.76470588235294117647058823529412"),
+ new BigDecimal("0.82352941176470588235294117647059"),
new BigDecimal("1")));
// Test modulo operations
@@ -266,6 +266,16 @@ public class OperationFctsTest extends CQLTester
}
@Test
+ public void testModuloWithDecimals() throws Throwable
+ {
+ createTable("CREATE TABLE %s (numerator decimal, dec_mod decimal, int_mod int, bigint_mod bigint, PRIMARY KEY((numerator, dec_mod)))");
+ execute("INSERT INTO %s (numerator, dec_mod, int_mod, bigint_mod) VALUES (123456789112345678921234567893123456, 2, 2, 2)");
+
+ assertRows(execute("SELECT numerator %% dec_mod, numerator %% int_mod, numerator %% bigint_mod from %s"),
+ row(new BigDecimal("0"), new BigDecimal("0.0"), new BigDecimal("0.0")));
+ }
+
+ @Test
public void testSingleOperationsWithLiterals() throws Throwable
{
createTable("CREATE TABLE %s (pk int, c1 tinyint, c2 smallint, v text, PRIMARY KEY(pk, c1, c2))");
@@ -438,7 +448,7 @@ public class OperationFctsTest extends CQLTester
row(0, 1, 1, 2L, 2.75F, 3.25, BigInteger.valueOf(3), new BigDecimal("4.25")));
assertRows(execute("SELECT a / 3, b / 3, c / 3, d / 3, e / 3, f / 3, g / 3, h / 3 FROM %s WHERE a = 1 AND b = 2"),
- row(0, 0, 1, 1L, 1.8333334F, 2.1666666666666665, BigInteger.valueOf(2), new BigDecimal("2.833333333333333333333333333333333")));
+ row(0, 0, 1, 1L, 1.8333334F, 2.1666666666666665, BigInteger.valueOf(2), new BigDecimal("2.83333333333333333333333333333333")));
assertRows(execute("SELECT a / " + bigInt + ","
+ " b / " + bigInt + ","
@@ -456,10 +466,10 @@ public class OperationFctsTest extends CQLTester
BigInteger.valueOf(7).divide(BigInteger.valueOf(bigInt))));
assertRows(execute("SELECT a / 5.5, b / 5.5, c / 5.5, d / 5.5, e / 5.5, f / 5.5, g / 5.5, h / 5.5 FROM %s WHERE a = 1 AND b = 2"),
- row(0.18181818181818182, 0.36363636363636365, 0.5454545454545454, 0.7272727272727273, 1.0, 1.1818181818181819, new BigDecimal("1.272727272727272727272727272727273"), new BigDecimal("1.545454545454545454545454545454545")));
+ row(0.18181818181818182, 0.36363636363636365, 0.5454545454545454, 0.7272727272727273, 1.0, 1.1818181818181819, new BigDecimal("1.27272727272727272727272727272727"), new BigDecimal("1.54545454545454545454545454545455")));
assertRows(execute("SELECT a / 6.5, b / 6.5, c / 6.5, d / 6.5, e / 6.5, f / 6.5, g / 6.5, h / 6.5 FROM %s WHERE a = 1 AND b = 2"),
- row(0.15384615384615385, 0.3076923076923077, 0.46153846153846156, 0.6153846153846154, 0.8461538461538461, 1.0, new BigDecimal("1.076923076923076923076923076923077"), new BigDecimal("1.307692307692307692307692307692308")));
+ row(0.15384615384615385, 0.3076923076923077, 0.46153846153846156, 0.6153846153846154, 0.8461538461538461, 1.0, new BigDecimal("1.07692307692307692307692307692308"), new BigDecimal("1.30769230769230769230769230769231")));
// Test modulo operations
@@ -503,6 +513,18 @@ public class OperationFctsTest extends CQLTester
}
@Test
+ public void testDivisionWithDecimals() throws Throwable
+ {
+ createTable("CREATE TABLE %s (numerator decimal, denominator decimal, PRIMARY KEY((numerator, denominator)))");
+ execute("INSERT INTO %s (numerator, denominator) VALUES (8.5, 200000000000000000000000000000000000)");
+ execute("INSERT INTO %s (numerator, denominator) VALUES (10000, 3)");
+
+ assertRows(execute("SELECT numerator / denominator from %s"),
+ row(new BigDecimal("0.0000000000000000000000000000000000425")),
+ row(new BigDecimal("3333.33333333333333333333333333333333")));
+ }
+
+ @Test
public void testWithCounters() throws Throwable
{
createTable("CREATE TABLE %s (a int PRIMARY KEY, b counter)");
@@ -663,7 +685,7 @@ public class OperationFctsTest extends CQLTester
OperationExecutionException.class,
"SELECT g / a FROM %s WHERE a = 0 AND b = 2");
- assertInvalidThrowMessage("the operation 'decimal / tinyint' failed: Division by zero",
+ assertInvalidThrowMessage("the operation 'decimal / tinyint' failed: BigInteger divide by zero",
OperationExecutionException.class,
"SELECT h / a FROM %s WHERE a = 0 AND b = 2");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org