hequn8128 edited a comment 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 compatible. 
   
   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. Although 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 keep backward 
compatible while converting TableEnvironment into an interface. @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:
[email protected]


With regards,
Apache Git Services

Reply via email to