advancedxy commented on code in PR #9192:
URL: https://github.com/apache/iceberg/pull/9192#discussion_r1419869752


##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/optimizer/ReplaceStaticInvoke.scala:
##########
@@ -40,14 +37,20 @@ import org.apache.spark.sql.types.StructType
 object ReplaceStaticInvoke extends Rule[LogicalPlan] {
 
   override def apply(plan: LogicalPlan): LogicalPlan =
-    plan.transformWithPruning (_.containsAllPatterns(BINARY_COMPARISON, 
FILTER)) {
+    plan.transformWithPruning (_.containsPattern(FILTER)) {
       case filter @ Filter(condition, _) =>
-        val newCondition = 
condition.transformWithPruning(_.containsPattern(BINARY_COMPARISON)) {
+        val newCondition = 
condition.transformWithPruning(_.containsAnyPattern(BINARY_COMPARISON, IN, 
INSET)) {
           case c @ BinaryComparison(left: StaticInvoke, right) if 
canReplace(left) && right.foldable =>
             c.withNewChildren(Seq(replaceStaticInvoke(left), right))
 
           case c @ BinaryComparison(left, right: StaticInvoke) if 
canReplace(right) && left.foldable =>
             c.withNewChildren(Seq(left, replaceStaticInvoke(right)))
+
+          case in @ In(value: StaticInvoke, _) if canReplace(value) =>

Review Comment:
   I believe you need to check `in.list.forall(_.foldable)` since pushable 
predicate requires all the operands are literals.
   
   The corresponding test case should also be added, something like 
`system.bucket(4, id) in (bucket_id_1, bucket_id_2)`?



##########
spark/v3.4/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSystemFunctionPushDownDQL.java:
##########
@@ -224,6 +226,35 @@ private void testBucketLongFunction(boolean partitioned) {
     Assertions.assertThat(actual.size()).isEqualTo(5);
   }
 
+  @Test
+  public void testBucketLongFunctionInClauseOnUnpartitionedTable() {

Review Comment:
   No ReplaceStaticInvoke changes for spark 3.4?
   
   Maybe you can just focus on spark v3.5, and back port it in another PR or 
change both 3.4 and 3.5 in this PR. Both are fine.



-- 
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-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to