rdblue commented on code in PR #18546:
URL: https://github.com/apache/iceberg/pull/18546#discussion_r4331481452
##########
core/src/main/java/org/apache/iceberg/BaseOverwriteFiles.java:
##########
@@ -108,6 +119,8 @@ public OverwriteFiles validateFromSnapshot(long snapshotId)
{
public OverwriteFiles conflictDetectionFilter(Expression
newConflictDetectionFilter) {
Preconditions.checkArgument(
newConflictDetectionFilter != null, "Conflict detection filter cannot
be null");
+ Preconditions.checkState(
+ !rowFilterSet, "Cannot set a conflict detection filter when
overwriting by row filter");
Review Comment:
I would not make `rowFilter` mutually exclusive with
`conflictDetectionFilter`. I see why you might want it to be, but the behavior
when `conflictDetectionFilter` is set is to use it in all cases. That means it
is compatible with `rowFilter` and we don't need a behavior change for this.
I think the motivation was to allow either `overwriteByRowFilter` or
`conflictDetectionFilter` to configure the new validator API directly, but we
can just wait until `commit` and run final configuration at that time.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
