You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "yuzelin (via GitHub)" <gi...@apache.org> on 2026/07/14 13:14:23 UTC
[PR] [WIP][parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
yuzelin opened a new pull request, #8623:
URL: https://github.com/apache/paimon/pull/8623
### Purpose
Fix problem of #7175
### Tests
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #8623:
URL: https://github.com/apache/paimon/pull/8623#discussion_r3592907361
##########
paimon-format/src/main/java/org/apache/paimon/format/parquet/ParquetReaderFactory.java:
##########
@@ -100,11 +104,26 @@ public class ParquetReaderFactory implements FormatReaderFactory {
public ParquetReaderFactory(
Options conf, RowType readType, int batchSize, @Nullable FilterCompat.Filter filter) {
+ this(conf, readType, batchSize, filter, null);
+ }
+
+ public static ParquetReaderFactory create(
+ Options conf, RowType readType, int batchSize, @Nullable List<Predicate> predicates) {
+ return new ParquetReaderFactory(conf, readType, batchSize, null, predicates);
+ }
+
+ private ParquetReaderFactory(
Review Comment:
Why create so many constructors?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #8623:
URL: https://github.com/apache/paimon/pull/8623
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "yuzelin (via GitHub)" <gi...@apache.org>.
yuzelin commented on PR #8623:
URL: https://github.com/apache/paimon/pull/8623#issuecomment-4992188615
> > For parquet decimal type, we need the file footer to get the physical type, so the change is big. Maybe we should forbid the decimal filter pushdown？
>
> Why change big? Please take a look to code deeply.
I think we should pass parquet file footer to 'bind decimal predicates to the actual per-file PrimitiveType', that's will make a big change.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "yuzelin (via GitHub)" <gi...@apache.org>.
yuzelin commented on PR #8623:
URL: https://github.com/apache/paimon/pull/8623#issuecomment-4987551977
For parquet decimal type, we need the file footer to get the physical type, so the change is big. Maybe we should forbid the decimal filter pushdown？
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #8623:
URL: https://github.com/apache/paimon/pull/8623#issuecomment-4993694959
- After `ParquetInputStream` reads the footer, `ParquetFilters.convert(...)` and the `read-options` construction occur before `ParquetFileReader` takes over the stream.
- If these two steps throw runtime exceptions and the stream is not `finally/close`-ed, file handles will be leaked. It is recommended to use `try/catch` to close the stream before the reader is successfully constructed.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [parquet] Fix that parquet filter pushdown wrong for decimal [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #8623:
URL: https://github.com/apache/paimon/pull/8623#issuecomment-4988613685
> For parquet decimal type, we need the file footer to get the physical type, so the change is big. Maybe we should forbid the decimal filter pushdown？
Why change big?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org