JingsongLi commented on code in PR #8623:
URL: https://github.com/apache/paimon/pull/8623#discussion_r3584594877
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -288,13 +289,13 @@ private static Comparable<?> toParquetObject(
return Binary.fromReusedByteArray((byte[]) value);
} else if (value instanceof Decimal) {
Decimal decimal = (Decimal) value;
- int precision = decimal.precision();
+ int precision = ((DecimalType) type).getPrecision();
if (ParquetSchemaConverter.is32BitDecimal(precision)) {
return (int) decimal.toUnscaledLong();
} else if (ParquetSchemaConverter.is64BitDecimal(precision)) {
Review Comment:
**[major] Avoid narrowing an out-of-range decimal literal.**
With the field precision now selecting the INT32 branch, a DECIMAL(9,0)
field combined with the literal 4294967297 is narrowed to 1. For example,
lt(column, 4294967297) becomes lt(column, 1), so a stored value of 1 can be
incorrectly filtered even though it satisfies the original predicate. Please
normalize and validate the literal against the field precision and scale, and
decline pushdown (or simplify the predicate safely) when it is out of domain.
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -313,6 +314,24 @@ private static Comparable<?> toParquetObject(
throw new UnsupportedOperationException();
}
+ private static Binary decimalToBinary(Decimal decimal, int precision) {
+ int numBytes =
ParquetSchemaConverter.computeMinBytesForDecimalPrecision(precision);
+ byte[] unscaledBytes = decimal.toUnscaledBytes();
+ if (unscaledBytes.length == numBytes) {
+ return Binary.fromConstantByteArray(unscaledBytes);
+ }
+
+ byte[] paddedBytes = new byte[numBytes];
+ Arrays.fill(paddedBytes, unscaledBytes[0] < 0 ? (byte) -1 : (byte) 0);
+ System.arraycopy(
+ unscaledBytes,
Review Comment:
**[major] Guard literals that are wider than the target physical width.**
For a DECIMAL(20,0) field, numBytes is 9, while a 38-digit literal can have
a 16-byte unscaled representation. The destination index here becomes -7, and
ParquetFilters.convert throws ArrayIndexOutOfBoundsException before reading
starts. Please detect unscaledBytes.length > numBytes and safely skip pushdown
instead of attempting to pad it.
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -288,13 +289,13 @@ private static Comparable<?> toParquetObject(
return Binary.fromReusedByteArray((byte[]) value);
} else if (value instanceof Decimal) {
Decimal decimal = (Decimal) value;
- int precision = decimal.precision();
+ int precision = ((DecimalType) type).getPrecision();
if (ParquetSchemaConverter.is32BitDecimal(precision)) {
Review Comment:
**[major] Normalize the literal to the field scale before encoding it.**
This uses the field precision but never DecimalType.getScale(). A
DECIMAL(20,2) value 100.00 is stored as unscaled 10000, while an equivalent
scale-0 literal 100 is encoded as 100; dictionary filtering can then report
canDrop=true and silently remove a matching row group. Please rescale exactly
to the field scale and skip pushdown when exact normalization is not possible.
##########
paimon-format/src/main/java/org/apache/parquet/filter2/predicate/ParquetFilters.java:
##########
@@ -288,13 +289,13 @@ private static Comparable<?> toParquetObject(
return Binary.fromReusedByteArray((byte[]) value);
} else if (value instanceof Decimal) {
Decimal decimal = (Decimal) value;
- int precision = decimal.precision();
+ int precision = ((DecimalType) type).getPrecision();
if (ParquetSchemaConverter.is32BitDecimal(precision)) {
return (int) decimal.toUnscaledLong();
Review Comment:
**[major] The predicate physical type cannot be inferred from logical
precision alone.**
Paimon can read valid external or migrated decimal files backed by INT32,
INT64, BINARY, or FIXED_LEN_BYTE_ARRAY, but the filter column and literal type
are selected only from logical precision. For example, a valid
FIXED_LEN_BYTE_ARRAY(4) DECIMAL(9,2) file gets an IntColumn predicate and fails
SchemaCompatibilityValidator instead of falling back to an unfiltered read.
Please bind decimal predicates to the actual per-file PrimitiveType, or disable
decimal pushdown when that physical type is unknown.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
