[GitHub] [iceberg] singhpk234 commented on a diff in pull request #6899: Spark 3.3: Optimize metadata-only DELETEs

["singhpk234 (via GitHub)" <gi...@apache.org>]

singhpk234 commented on code in PR #6899:
URL: https://github.com/apache/iceberg/pull/6899#discussion_r1113735659


##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java:
##########
@@ -314,6 +318,10 @@ private boolean canDeleteUsingMetadata(Expression deleteExpr) {
     }
   }
 
+  private boolean isCaseSensitive() {
+    return Boolean.parseBoolean(sparkSession().conf().get("spark.sql.caseSensitive"));

Review Comment:
   [minor] should we make this instance variable ?



##########
spark/v3.3/spark/src/main/java/org/apache/iceberg/spark/source/SparkTable.java:
##########
@@ -274,18 +275,21 @@ public boolean canDeleteWhere(Filter[] filters) {
       }
     }
 
-    return deleteExpr == Expressions.alwaysTrue() || canDeleteUsingMetadata(deleteExpr);
+    return selectsPartitions(deleteExpr) || canDeleteUsingMetadata(deleteExpr);
+  }
+
+  private boolean selectsPartitions(Expression expr) {

Review Comment:
   +1, if selectsPartitions returns true and it's an disjunction, so canDeleteUsingMetadata will not be called.
   
   [question] if we can move selectsPartition into an util, for ex: SparkUtil we can make SparkScanBuilder use the same. As in one place i.e here we are checking if all are true, and other i.e SparkScanBuilder if atleast one is false.



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

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


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