rdblue commented on code in PR #6524:
URL: https://github.com/apache/iceberg/pull/6524#discussion_r1064181041
##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkScanBuilder.java:
##########
@@ -105,42 +105,52 @@ public SparkScanBuilder caseSensitive(boolean
isCaseSensitive) {
@Override
public Filter[] pushFilters(Filter[] filters) {
+ // there are 3 kinds of filters:
+ // (1) filters that can be pushed down completely and don't have to
evaluated by Spark
+ // (e.g. filters that select entire partitions)
+ // (2) filters that can be pushed down partially and require record-level
filtering in Spark
+ // (e.g. filters that may select some but not necessarily all rows in
a file)
+ // (3) filters that can't be pushed down at all and have to be evaluated
by Spark
+ // (e.g. unsupported filters)
+ // filters (1) and (2) are used prune files during job planning in Iceberg
+ // filters (2) and (3) form a set of post scan filters and must be
evaluated by Spark
+
List<Expression> expressions =
Lists.newArrayListWithExpectedSize(filters.length);
- List<Filter> pushed = Lists.newArrayListWithExpectedSize(filters.length);
+ List<Filter> pushableFilters =
Lists.newArrayListWithExpectedSize(filters.length);
+ List<Filter> postScanFilters =
Lists.newArrayListWithExpectedSize(filters.length);
for (Filter filter : filters) {
- Expression expr = null;
try {
- expr = SparkFilters.convert(filter);
- } catch (IllegalArgumentException e) {
- // converting to Iceberg Expression failed, so this expression cannot
be pushed down
- LOG.info(
- "Failed to convert filter to Iceberg expression, skipping push
down for this expression: {}. {}",
- filter,
- e.getMessage());
- }
+ Expression expr = SparkFilters.convert(filter);
- if (expr != null) {
- try {
+ if (expr != null) {
+ // try binding the expression to ensure it can be pushed down
Binder.bind(schema.asStruct(), expr, caseSensitive);
expressions.add(expr);
- pushed.add(filter);
- } catch (ValidationException e) {
- // binding to the table schema failed, so this expression cannot be
pushed down
- LOG.info(
- "Failed to bind expression to table schema, skipping push down
for this expression: {}. {}",
- filter,
- e.getMessage());
+ pushableFilters.add(filter);
+ }
+
+ if (expr == null || requiresSparkFiltering(expr)) {
+ postScanFilters.add(filter);
+ } else {
+ LOG.info("Evaluating completely on Iceberg side: {}", filter);
Review Comment:
Is this worth an INFO message? That seems verbose to me. How about debug?
--
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]
