Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: skip negative scale checks for creating decimals #723

Merged
merged 5 commits into from
Aug 1, 2024
Merged

fix: skip negative scale checks for creating decimals #723

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d8f2946
fix: skip negative scale checks for creating decimals
kazuyukitanimura Jul 25, 2024
d00e05c
fix: skip negative scale checks for creating decimals
kazuyukitanimura Jul 25, 2024
d0b8ec8
fix: skip negative scale checks for creating decimals
kazuyukitanimura Jul 25, 2024
027f9b8
fix: skip negative scale checks for creating decimals
kazuyukitanimura Jul 25, 2024
2230c66
fix: skip negative scale checks for creating decimals
kazuyukitanimura Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/src/main/java/org/apache/comet/vector/CometDictionary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class CometDictionary implements AutoCloseable {
private final int numValues;

/** Decoded dictionary values. We only need to copy values for decimal type. */
private ByteArrayWrapper[] binaries;
private volatile ByteArrayWrapper[] binaries;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not directly related but forgot to add based on #705 (comment)


public CometDictionary(CometPlainVector values) {
this.values = values;
Expand Down
13 changes: 11 additions & 2 deletions common/src/main/java/org/apache/comet/vector/CometVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ 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);
Expand All @@ -98,6 +98,15 @@ public Decimal getDecimal(int i, int precision, int 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the check for negative scale is a feature for ANSI correctness? https://issues.apache.org/jira/browse/SPARK-30252

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not need to do the check every read, we are running ANSI tests in spark_sql_test_ansi.yml

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;
}

/**
* Reads a 16-byte byte array which are encoded big-endian for decimal128 into internal byte
* array.
Expand Down
Loading