You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/07/01 00:55:31 UTC

[GitHub] [flink] JingsongLi opened a new pull request, #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

JingsongLi opened a new pull request, #20119:
URL: https://github.com/apache/flink/pull/20119

   ## What is the purpose of the change
   
   In [FLINK-25990](https://issues.apache.org/jira/browse/FLINK-25990) ,
   Add a method "DataStream<RowData> produceDataStream(ProviderContext providerContext, StreamExecutionEnvironment execEnv)" in DataStreamScanProvider.
   But this method has no default implementation, this is not compatible when users upgrade their DataStreamScanProvider implementation from 1.14 to 1.15.
   
   DataStreamSinkProvider is the same.
   
   ## Brief change log
   
   DataStreamScanProvider's new method should be:
   ```
       default DataStream<RowData> produceDataStream(
               ProviderContext providerContext, StreamExecutionEnvironment execEnv) {
           return produceDataStream(execEnv);
       }
   ```
   
   ## Verifying this change
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (yes / **no**)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
     - The serializers: (yes / **no** / don't know)
     - The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know)
     - The S3 file system connector: (yes / **no** / don't know)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (yes / **no**)


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] qingwei91 commented on a diff in pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
qingwei91 commented on code in PR #20119:
URL: https://github.com/apache/flink/pull/20119#discussion_r912527749


##########
flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/connector/sink/DataStreamSinkProvider.java:
##########
@@ -54,8 +54,10 @@
      *
      * @see SingleOutputStreamOperator#uid(String)
      */
-    DataStreamSink<?> consumeDataStream(
-            ProviderContext providerContext, DataStream<RowData> dataStream);
+    default DataStreamSink<?> consumeDataStream(
+            ProviderContext providerContext, DataStream<RowData> dataStream) {
+        return consumeDataStream(dataStream);

Review Comment:
   Hi, can you elaborate what is the consequence of using this default implementation? 



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] flinkbot commented on pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #20119:
URL: https://github.com/apache/flink/pull/20119#issuecomment-1171812883

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "a4a333089ada574fcc22e5a0de3d10df3292bdad",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "a4a333089ada574fcc22e5a0de3d10df3292bdad",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * a4a333089ada574fcc22e5a0de3d10df3292bdad UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] JingsongLi merged pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
JingsongLi merged PR #20119:
URL: https://github.com/apache/flink/pull/20119


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] JingsongLi commented on a diff in pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #20119:
URL: https://github.com/apache/flink/pull/20119#discussion_r912568525


##########
flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/connector/sink/DataStreamSinkProvider.java:
##########
@@ -54,8 +54,10 @@
      *
      * @see SingleOutputStreamOperator#uid(String)
      */
-    DataStreamSink<?> consumeDataStream(
-            ProviderContext providerContext, DataStream<RowData> dataStream);
+    default DataStreamSink<?> consumeDataStream(
+            ProviderContext providerContext, DataStream<RowData> dataStream) {
+        return consumeDataStream(dataStream);

Review Comment:
   The old method will not be called.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] twalthr commented on a diff in pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
twalthr commented on code in PR #20119:
URL: https://github.com/apache/flink/pull/20119#discussion_r912736423


##########
flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/connector/source/DataStreamScanProvider.java:
##########
@@ -48,8 +48,10 @@ public interface DataStreamScanProvider extends ScanTableSource.ScanRuntimeProvi
      *
      * @see SingleOutputStreamOperator#uid(String)
      */
-    DataStream<RowData> produceDataStream(
-            ProviderContext providerContext, StreamExecutionEnvironment execEnv);
+    default DataStream<RowData> produceDataStream(

Review Comment:
   Just out of curiosity: Will this prevent errors in downstream systems? Previously implemented lambas will still fail, no?



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink] JingsongLi commented on a diff in pull request #20119: [FLINK-28322][table-api] DataStreamScan(Sink)Provider's new method is not compatible

Posted by GitBox <gi...@apache.org>.
JingsongLi commented on code in PR #20119:
URL: https://github.com/apache/flink/pull/20119#discussion_r914348586


##########
flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/connector/source/DataStreamScanProvider.java:
##########
@@ -48,8 +48,10 @@ public interface DataStreamScanProvider extends ScanTableSource.ScanRuntimeProvi
      *
      * @see SingleOutputStreamOperator#uid(String)
      */
-    DataStream<RowData> produceDataStream(
-            ProviderContext providerContext, StreamExecutionEnvironment execEnv);
+    default DataStream<RowData> produceDataStream(

Review Comment:
   Yes, lambda will still fail, and downstream systems will still require code changes.
   However, downstream systems can have the possibility to be compatible with 1.14 and 1.15, without having to maintain two copies of the code.



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org