[GitHub] [flink] hequn8128 commented on issue #8050: [FLINK-11067][table] Convert TableEnvironments to interfaces

[GitBox <gi...@apache.org>]

hequn8128 commented on issue #8050: [FLINK-11067][table] Convert TableEnvironments to interfaces
URL: https://github.com/apache/flink/pull/8050#issuecomment-479572670
 
 
   @dawidwys  Thanks a lot for your review. I think you have given a lot of good suggestions!
   
   1. As for the useless APIs in `TableEnvironment` interface, maybe we can deprecate first and remove them later. This can make our interface compitable. 
   
   2. To uncouple the `Descriptor` with `TableEnvironment`,  we can change registerTableSource to getTableSource in `RegistrableDescriptor`(also should rename `RegistrableDescriptor`). Once we get the table source or sink, then we can register the table source or sink with TableEnvironment. Althougn this makes users impossible to register table source(or sink) directly, I am ok with it.
   
   3. For the `TableFactoryUtil`, I am not sure if we can port it into api or common module(I will make sure later). However, I do find we can also uncouple TableEnvironment here. In the current implementation, it seems table env is only used to decide whether it is a batch or stream source(or sink). We can also use descriptor to decide, i.e., a StreamTableDescriptor means a stream source(or sink). 
   
   For 2 and 3, I think the biggest obstacle is we may need to fix the problem in a backwards compatible way. @twalthr Would be great to also have your opinions here.
   
   Best, Hequn

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services