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?
--
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]
