stevenzwu commented on code in PR #10207:
URL: https://github.com/apache/iceberg/pull/10207#discussion_r1580202486
##########
flink/v1.19/flink/src/main/java/org/apache/iceberg/flink/source/ScanContext.java:
##########
@@ -130,7 +131,9 @@ private ScanContext(
this.watermarkColumn = watermarkColumn;
this.watermarkColumnTimeUnit = watermarkColumnTimeUnit;
- validate();
+ if (!skipValidate) {
Review Comment:
> If yes, then we change the copyWithAppendsBetween and the
copyWithSnapshotId to remove the streaming flag, as those are not a streaming
scans anymore.
that is also not correct, because `copy` should meant `copy`. The main
problem is that `ScanContext` is used for both user intention (via source
builder) and internal incremental scan. I agree that internal incremental scan
shouldn't have the streaming setting.
note that `ScanContext` is not a public class. Users can't construct this
object directly. Maybe the `validate()` method shouldn't be called by the
constructor and only be called by the `ScanContext#Builder#build()` method? or
move some more intrinsic validation to `IcebergSource` builder?
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
