Stephen0421 commented on PR #7643: URL: https://github.com/apache/paimon/pull/7643#issuecomment-4533771017
> Thanks for the detailed implementation and tests. I found one correctness issue in the explicit Flink action path: > > `ExpirePartitionsAction` still builds a `PartitionExpireStrategy` from the user-provided `expireStrategy` argument and calls `FileStore.newPartitionExpire(..., expireStrategy)`. However, the chain-table branch in `AbstractFileStore.newPartitionExpire(String, FileStoreTable, Duration, Duration, PartitionExpireStrategy)` ignores that argument and always returns `newChainTablePartitionExpire(...)`. > > This means an explicit action call with `expireStrategy = update-time` (or a custom strategy) against a chain table will silently run the chain table's values-time expiration instead of rejecting the unsupported strategy. The schema validation added in this PR catches persisted/dynamic table options, but it does not cover this action path because the action passes the strategy only as an argument to the overload. > > Could we add the same validation in this overload before creating `ChainTablePartitionExpire` (for example, require the provided strategy to be `PartitionValuesTimeExpireStrategy`), or route the action through dynamic options so `SchemaValidation` rejects non-`values-time` consistently? Already add validation in AbstractFileStore.newPartitionExpire. -- 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]
