zhoujinsong commented on code in PR #3426:
URL: https://github.com/apache/amoro/pull/3426#discussion_r1981083291


##########
amoro-format-iceberg/src/main/java/org/apache/amoro/table/TableProperties.java:
##########
@@ -115,6 +115,9 @@ private TableProperties() {}
       "self-optimizing.full.rewrite-all-files";
   public static final boolean SELF_OPTIMIZING_FULL_REWRITE_ALL_FILES_DEFAULT = 
true;
 
+  public static final String SELF_OPTIMIZING_PARTITION_FILTER = 
"self-optimizing.partition-filter";

Review Comment:
   Is `self-optimizing.filter` a better name?
   
   In the long term, we may further control the merged files within partitions 
to address potential needs that non-partitioned tables might have. Of course, 
at this stage, we can focus on control at the partition level and document it 
in the documents. However, the configuration names could be considered with a 
longer-term perspective in mind.
   
   



##########
amoro-format-iceberg/pom.xml:
##########
@@ -154,6 +154,12 @@
             <artifactId>paimon-bundle</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>com.github.jsqlparser</groupId>
+            <artifactId>jsqlparser</artifactId>
+            <version>4.7</version>

Review Comment:
   We do better manage the dependency version in pom properties.
   So maybe add this dependency into the dependencyManagement section in the 
parent pom.



##########
amoro-format-iceberg/src/main/java/org/apache/amoro/optimizing/plan/AbstractOptimizingEvaluator.java:
##########
@@ -111,7 +141,164 @@ protected void initEvaluator() {
   }
 
   protected Expression getPartitionFilter() {
-    return Expressions.alwaysTrue();
+    String partitionFilter = config.getPartitionFilter();

Review Comment:
   How about adding a util method in `org.apache.amoro.utils.ExpressionUtil`?



-- 
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: commits-unsubscr...@amoro.apache.org

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

Reply via email to