huaxingao commented on code in PR #17956:
URL: https://github.com/apache/iceberg/pull/17956#discussion_r3998424018


##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/source/BaseSparkScanBuilder.java:
##########
@@ -62,19 +63,18 @@
 abstract class BaseSparkScanBuilder implements ScanBuilder {
 
   private static final Logger LOG = 
LoggerFactory.getLogger(BaseSparkScanBuilder.class);
-  private static final Predicate[] NO_PREDICATES = new Predicate[0];

Review Comment:
   nit: Removing `NO_PREDICATES` also took the blank line that separated the 
static constant from the instance fields. Could you add back the blank line?



##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/source/BaseSparkScanBuilder.java:
##########
@@ -62,19 +63,18 @@
 abstract class BaseSparkScanBuilder implements ScanBuilder {
 
   private static final Logger LOG = 
LoggerFactory.getLogger(BaseSparkScanBuilder.class);
-  private static final Predicate[] NO_PREDICATES = new Predicate[0];
-
   private final SparkSession spark;
   private final Table table;
   private final Schema schema;
   private final SparkReadConf readConf;
   private final boolean caseSensitive;
   private final Set<String> metaFieldNames = Sets.newLinkedHashSet();
   private final InMemoryMetricsReporter metricsReporter = new 
InMemoryMetricsReporter();
+  private final List<PartitionPredicate> partitionPredicates = 
Lists.newArrayList();
+  private final List<Predicate> pushedPredicates = Lists.newArrayList();
 
   private Schema projection;
   private List<Expression> filters = Lists.newArrayList();
-  private Predicate[] pushedPredicates = NO_PREDICATES;

Review Comment:
   nit:  `filters` (line 76) is no longer reassigned now that it's only 
appended to, so it could be `final` and move up to the `final` section..



##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkPartitioningAwareScan.java:
##########
@@ -264,4 +293,98 @@ protected String groupingKeyDesc() {
         .map(NestedField::name)
         .collect(Collectors.joining(", "));
   }
+
+  protected List<PartitionPredicate> partitionPredicates() {
+    return partitionPredicates;
+  }
+
+  private boolean matchesPartitionPredicates(
+      T task, Map<Integer, PartitionPredicateEvaluator> evaluatorsBySpecId) {
+    PartitionPredicateEvaluator evaluator =
+        evaluatorsBySpecId.computeIfAbsent(
+            task.spec().specId(), ignored -> new 
PartitionPredicateEvaluator(task.spec()));
+    return evaluator.eval(task.partition());
+  }
+
+  private List<PartitionField> activePartitionFields(PartitionSpec spec) {

Review Comment:
   nit: `activePartitionFields` and `activePartitionType` both take the spec as 
a parameter, touch no instance state, and are only called from the constructor 
— both could be `private static`.



-- 
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]

Reply via email to