Skip to content

Commit fbe86d0

Browse files
fix: skip negative scale checks for creating decimals (apache#723)
1 parent ef90bac commit fbe86d0

File tree

3 files changed

+35
-19
lines changed

3 files changed

+35
-19
lines changed

common/src/main/java/org/apache/comet/vector/CometDictionary.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class CometDictionary implements AutoCloseable {
3030
private final int numValues;
3131

3232
/** Decoded dictionary values. We only need to copy values for decimal type. */
33-
private ByteArrayWrapper[] binaries;
33+
private volatile ByteArrayWrapper[] binaries;
3434

3535
public CometDictionary(CometPlainVector values) {
3636
this.values = values;

common/src/main/java/org/apache/comet/vector/CometVector.java

+34-17
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@ public abstract class CometVector extends ColumnVector {
4545
private final byte[] DECIMAL_BYTES = new byte[DECIMAL_BYTE_WIDTH];
4646
protected final boolean useDecimal128;
4747

48+
private static final long decimalValOffset;
49+
50+
static {
51+
try {
52+
java.lang.reflect.Field unsafeField = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
53+
unsafeField.setAccessible(true);
54+
final sun.misc.Unsafe unsafe = (sun.misc.Unsafe) unsafeField.get(null);
55+
decimalValOffset = unsafe.objectFieldOffset(Decimal.class.getDeclaredField("decimalVal"));
56+
} catch (Throwable e) {
57+
throw new RuntimeException(e);
58+
}
59+
}
60+
4861
protected CometVector(DataType type, boolean useDecimal128) {
4962
super(type);
5063
this.useDecimal128 = useDecimal128;
@@ -73,31 +86,35 @@ public boolean isFixedLength() {
7386
@Override
7487
public Decimal getDecimal(int i, int precision, int scale) {
7588
if (!useDecimal128 && precision <= Decimal.MAX_INT_DIGITS() && type instanceof IntegerType) {
76-
return Decimal.createUnsafe(getInt(i), precision, scale);
89+
return createDecimal(getInt(i), precision, scale);
7790
} else if (!useDecimal128 && precision <= Decimal.MAX_LONG_DIGITS()) {
78-
return Decimal.createUnsafe(getLong(i), precision, scale);
91+
return createDecimal(getLong(i), precision, scale);
7992
} else {
8093
byte[] bytes = getBinaryDecimal(i);
8194
BigInteger bigInteger = new BigInteger(bytes);
8295
BigDecimal javaDecimal = new BigDecimal(bigInteger, scale);
83-
try {
84-
return Decimal.apply(javaDecimal, precision, scale);
85-
} catch (ArithmeticException e) {
86-
throw new ArithmeticException(
87-
"Cannot convert "
88-
+ javaDecimal
89-
+ " (bytes: "
90-
+ bytes
91-
+ ", integer: "
92-
+ bigInteger
93-
+ ") to decimal with precision: "
94-
+ precision
95-
+ " and scale: "
96-
+ scale);
97-
}
96+
return createDecimal(javaDecimal, precision, scale);
9897
}
9998
}
10099

100+
/** This method skips the negative scale check, otherwise the same as Decimal.createUnsafe(). */
101+
private Decimal createDecimal(long unscaled, int precision, int scale) {
102+
Decimal dec = new Decimal();
103+
dec.org$apache$spark$sql$types$Decimal$$longVal_$eq(unscaled);
104+
dec.org$apache$spark$sql$types$Decimal$$_precision_$eq(precision);
105+
dec.org$apache$spark$sql$types$Decimal$$_scale_$eq(scale);
106+
return dec;
107+
}
108+
109+
/** This method skips a few checks, otherwise the same as Decimal.apply(). */
110+
private Decimal createDecimal(BigDecimal value, int precision, int scale) {
111+
Decimal dec = new Decimal();
112+
Platform.putObjectVolatile(dec, decimalValOffset, new scala.math.BigDecimal(value));
113+
dec.org$apache$spark$sql$types$Decimal$$_precision_$eq(precision);
114+
dec.org$apache$spark$sql$types$Decimal$$_scale_$eq(scale);
115+
return dec;
116+
}
117+
101118
/**
102119
* Reads a 16-byte byte array which are encoded big-endian for decimal128 into internal byte
103120
* array.

pom.xml

-1
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,6 @@ under the License.
711711
</execution>
712712
</executions>
713713
<configuration>
714-
-->
715714
<scalaVersion>${scala.version}</scalaVersion>
716715
<checkMultipleScalaVersions>true</checkMultipleScalaVersions>
717716
<failOnMultipleScalaVersions>true</failOnMultipleScalaVersions>

0 commit comments

Comments
 (0)