parthchandra commented on code in PR #723: URL: https://github.com/apache/datafusion-comet/pull/723#discussion_r1697308894
########## common/src/main/java/org/apache/comet/vector/CometVector.java: ########## @@ -73,31 +86,35 @@ public boolean isFixedLength() { @Override public Decimal getDecimal(int i, int precision, int scale) { if (!useDecimal128 && precision <= Decimal.MAX_INT_DIGITS() && type instanceof IntegerType) { - return Decimal.createUnsafe(getInt(i), precision, scale); + return createDecimal(getInt(i), precision, scale); } else if (!useDecimal128 && precision <= Decimal.MAX_LONG_DIGITS()) { - return Decimal.createUnsafe(getLong(i), precision, scale); + return createDecimal(getLong(i), precision, scale); } else { byte[] bytes = getBinaryDecimal(i); BigInteger bigInteger = new BigInteger(bytes); BigDecimal javaDecimal = new BigDecimal(bigInteger, scale); - try { - return Decimal.apply(javaDecimal, precision, scale); - } catch (ArithmeticException e) { - throw new ArithmeticException( - "Cannot convert " - + javaDecimal - + " (bytes: " - + bytes - + ", integer: " - + bigInteger - + ") to decimal with precision: " - + precision - + " and scale: " - + scale); - } + return createDecimal(javaDecimal, precision, scale); } } + /** This method skips the negative scale check, otherwise the same as Decimal.createUnsafe(). */ + private Decimal createDecimal(long unscaled, int precision, int scale) { + Decimal dec = new Decimal(); Review Comment: Seems the check for negative scale is a feature for ANSI correctness? https://issues.apache.org/jira/browse/SPARK-30252 ########## common/src/main/java/org/apache/comet/vector/CometVector.java: ########## @@ -73,31 +86,35 @@ public boolean isFixedLength() { @Override public Decimal getDecimal(int i, int precision, int scale) { if (!useDecimal128 && precision <= Decimal.MAX_INT_DIGITS() && type instanceof IntegerType) { - return Decimal.createUnsafe(getInt(i), precision, scale); + return createDecimal(getInt(i), precision, scale); } else if (!useDecimal128 && precision <= Decimal.MAX_LONG_DIGITS()) { - return Decimal.createUnsafe(getLong(i), precision, scale); + return createDecimal(getLong(i), precision, scale); } else { byte[] bytes = getBinaryDecimal(i); BigInteger bigInteger = new BigInteger(bytes); BigDecimal javaDecimal = new BigDecimal(bigInteger, scale); - try { - return Decimal.apply(javaDecimal, precision, scale); - } catch (ArithmeticException e) { - throw new ArithmeticException( - "Cannot convert " - + javaDecimal - + " (bytes: " - + bytes - + ", integer: " - + bigInteger - + ") to decimal with precision: " - + precision - + " and scale: " - + scale); - } + return createDecimal(javaDecimal, precision, scale); } } + /** This method skips the negative scale check, otherwise the same as Decimal.createUnsafe(). */ + private Decimal createDecimal(long unscaled, int precision, int scale) { + Decimal dec = new Decimal(); + dec.org$apache$spark$sql$types$Decimal$$longVal_$eq(unscaled); + dec.org$apache$spark$sql$types$Decimal$$_precision_$eq(precision); + dec.org$apache$spark$sql$types$Decimal$$_scale_$eq(scale); + return dec; + } + + /** This method skips a few checks, otherwise the same as Decimal.apply(). */ + private Decimal createDecimal(BigDecimal value, int precision, int scale) { + Decimal dec = new Decimal(); + Platform.putObjectVolatile(dec, decimalValOffset, new scala.math.BigDecimal(value)); Review Comment: The spark version has some additional checks for rounding and precision. Do we not need those? ``` this.decimalVal = decimal.setScale(scale, ROUND_HALF_UP) if (decimalVal.precision > precision) { throw QueryExecutionErrors.decimalPrecisionExceedsMaxPrecisionError( decimalVal.precision, precision) } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org