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