dawidwys commented on a change in pull request #11692:
URL: https://github.com/apache/flink/pull/11692#discussion_r411147490
##########
File path:
flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/connector/source/abilities/PeriodicWatermarkAssignerProvider.java
##########
@@ -28,19 +28,14 @@
* generating watermarks in {@link ScanTableSource}.
*/
@PublicEvolving
-public final class PeriodicWatermarkAssignerProvider extends
SupportsWatermarkPushDown.WatermarkProvider {
+public interface PeriodicWatermarkAssignerProvider extends
SupportsWatermarkPushDown.WatermarkProvider {
- private final AssignerWithPeriodicWatermarks<RowData> periodicAssigner;
-
- private
PeriodicWatermarkAssignerProvider(AssignerWithPeriodicWatermarks<RowData>
periodicAssigner) {
- this.periodicAssigner = periodicAssigner;
- }
-
- public static PeriodicWatermarkAssignerProvider
of(AssignerWithPeriodicWatermarks<RowData> periodicAssigner) {
- return new PeriodicWatermarkAssignerProvider(periodicAssigner);
+ /**
+ * Helper method for creating a static provider.
+ */
+ static PeriodicWatermarkAssignerProvider
of(AssignerWithPeriodicWatermarks<RowData> periodicAssigner) {
Review comment:
This method does not make much sense, does it? Planner will always need
to create a Provider that implements at least two of the interfaces e.g.
`PeriodicWatermarkAssignerProvider` and FLIP-27 `WatermarkProvider`? Moreover
we probably want to have some supplier interface here. We don't necessarily
need to instantiate the `AssignerWithPeriodicWatermarks` if the source works
with the interface from FLIP-27.
Can we just remove this method? I really can't see any benefit of it
whatsoever.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
