jshmchenxi commented on code in PR #14041:
URL: https://github.com/apache/iceberg/pull/14041#discussion_r2348058389
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetFilters.java:
##########
@@ -175,7 +204,7 @@ public <T> FilterPredicate predicate(UnboundPredicate<T>
pred) {
@SuppressWarnings("checkstyle:MethodTypeParameterName")
private static <C extends Comparable<C>, COL extends Operators.Column<C> &
Operators.SupportsLtGt>
- FilterPredicate pred(Operation op, COL col, C value) {
+ FilterPredicate pred(Operation op, COL col, C value, Set<C> valueSet) {
Review Comment:
[Spark](https://github.com/huaxingao/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala)
handles predicate pushdown in a more verbose way, basically:
```
predicate match {
case OP1:
parquetType match {
case TYPE1: FilterApi.op1()
case TYPE2: FilterApi.op1()
...
}
case OP2:
parquetType match {
case TYPE1: FilterApi.op2()
case TYPE2: FilterApi.op2()
...
}
...
}
```
While Iceberg handles this more compactly. It already handles unary
predicate and literal predicate in the same pred function. I wouldn't mind
adding a new pred function, but it seems we already use different args (no arg
or C) based on predicate type in pred.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
