You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by zoudan <zo...@163.com> on 2020/05/06 01:43:56 UTC

Re: [DISCUSS]Refactor flink-jdbc connector structure

+1 from my side, it will make jdbc connector unified with other ones.

Best,
Dan Zou


> 在 2020年4月30日,23:16,Jark Wu <im...@gmail.com> 写道:
> 
> Big +1 from my side.
> The new structure and class names look nicer now.
> 
> Regarding to the compability problem, I have looked into the public APIs in
> flink-jdbc, there are 3 kinds of APIs now:
> 1) new introduced JdbcSink for DataStream users in 1.11
> 2) JDBCAppendTableSink, JDBCUpsertTableSink, JDBCTableSource are introduced
> since 1.9
> 3) very ancient JDBCOutputFormat and JDBCInputFormat
> 
> For (1), as it's an un-released API, so I think it's safe to move to new
> package. cc @Khachatryan Roman <kh...@gmail.com> who
> contributed this.
> For (2), because TableSource and TableSink are not designed to be accessed
> by users since 1.11, so I think it's fine to move them.
> For (3), I'm not sure how many users are still using these out-of-date
> classes.
> But I think it's fine to keep them for one more version, and drop them in
> the next version.
> 
> 
> Best,
> Jark
> 
> On Thu, 30 Apr 2020 at 22:57, Flavio Pompermaier <po...@okkam.it>
> wrote:
> 
>> Very big +1 from me
>> 
>> Best,
>> Flavio
>> 
>> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <da...@alpinegizmo.com>
>> wrote:
>> 
>>> I'm very happy to see the jdbc connector being normalized in this way. +1
>>> from me.
>>> 
>>> David
>>> 
>>> On Thu, Apr 30, 2020 at 2:14 PM Timo Walther <tw...@apache.org> wrote:
>>> 
>>>> Hi Leonard,
>>>> 
>>>> this sounds like a nice refactoring for consistency. +1 from my side.
>>>> 
>>>> However, I'm not sure how much backwards compatibility is required.
>>>> Maybe others can comment on this.
>>>> 
>>>> Thanks,
>>>> Timo
>>>> 
>>>> On 30.04.20 14:09, Leonard Xu wrote:
>>>>> Hi, dear community
>>>>> 
>>>>> Recently, I’m thinking to refactor the flink-jdbc connector structure
>>>> before release 1.11.
>>>>> After the refactor, in the future,  we can easily introduce unified
>>>> pluggable JDBC dialect for Table and DataStream, and we can have a
>> better
>>>> module organization and implementations.
>>>>> 
>>>>> So, I propose following changes:
>>>>> 1) Use `Jdbc` instead of `JDBC` in the new public API and interface
>>>> name. The Datastream API `JdbcSink` which imported in this version has
>>>> followed this standard.
>>>>> 
>>>>> 2) Move all interface and classes from `org.apache.flink.java.io
>>> .jdbc`(old
>>>> package) to `org.apache.flink.connector.jdbc`(new package) to follow
>> the
>>>> base connector path in FLIP-27.
>>>>> I think we can move JDBC TableSource, TableSink and factory from old
>>>> package to new package because TableEnvironment#registerTableSource、
>>>> TableEnvironment#registerTableSink  will be removed in 1.11 ans these
>>>> classes are not exposed to users[1].
>>>>> We can move Datastream API JdbcSink from old package to new package
>>>> because it’s  introduced in this version.
>>>>> We will still keep `JDBCInputFormat` and `JDBCOoutoutFormat` in old
>>>> package and deprecate them.
>>>>> Other classes/interfaces are internal used and we can move to new
>>>> package without breaking compatibility.
>>>>> 3) Rename `flink-jdbc` to `flink-connector-jdbc`. well, this is a
>>>> compatibility broken change but in order to comply with other
>> connectors
>>>> and it’s real a connector rather than a flink-jdc-driver[2] we’d better
>>>> decide do it ASAP.
>>>>> 
>>>>> 
>>>>> What do you think? Any feedback is appreciate.
>>>>> 
>>>>> 
>>>>> Best,
>>>>> Leonard Xu
>>>>> 
>>>>> [1]
>>>> 
>>> 
>> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
>>>> <
>>>> 
>>> 
>> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
>>>>> 
>>>>> [2]https://github.com/ververica/flink-jdbc-driver <
>>>> https://github.com/ververica/flink-jdbc-driver>
>>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>> 


Re: [DISCUSS]Refactor flink-jdbc connector structure

Posted by Canbin Zheng <fe...@gmail.com>.
That sounds like a good refactoring for consistency, +1 from my side

Regards,
Canbin Zheng

zoudan <zo...@163.com> 于2020年5月6日周三 上午9:44写道:

> +1 from my side, it will make jdbc connector unified with other ones.
>
> Best,
> Dan Zou
>
>
> > 在 2020年4月30日,23:16,Jark Wu <im...@gmail.com> 写道:
> >
> > Big +1 from my side.
> > The new structure and class names look nicer now.
> >
> > Regarding to the compability problem, I have looked into the public APIs
> in
> > flink-jdbc, there are 3 kinds of APIs now:
> > 1) new introduced JdbcSink for DataStream users in 1.11
> > 2) JDBCAppendTableSink, JDBCUpsertTableSink, JDBCTableSource are
> introduced
> > since 1.9
> > 3) very ancient JDBCOutputFormat and JDBCInputFormat
> >
> > For (1), as it's an un-released API, so I think it's safe to move to new
> > package. cc @Khachatryan Roman <kh...@gmail.com> who
> > contributed this.
> > For (2), because TableSource and TableSink are not designed to be
> accessed
> > by users since 1.11, so I think it's fine to move them.
> > For (3), I'm not sure how many users are still using these out-of-date
> > classes.
> > But I think it's fine to keep them for one more version, and drop them in
> > the next version.
> >
> >
> > Best,
> > Jark
> >
> > On Thu, 30 Apr 2020 at 22:57, Flavio Pompermaier <po...@okkam.it>
> > wrote:
> >
> >> Very big +1 from me
> >>
> >> Best,
> >> Flavio
> >>
> >> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <da...@alpinegizmo.com>
> >> wrote:
> >>
> >>> I'm very happy to see the jdbc connector being normalized in this way.
> +1
> >>> from me.
> >>>
> >>> David
> >>>
> >>> On Thu, Apr 30, 2020 at 2:14 PM Timo Walther <tw...@apache.org>
> wrote:
> >>>
> >>>> Hi Leonard,
> >>>>
> >>>> this sounds like a nice refactoring for consistency. +1 from my side.
> >>>>
> >>>> However, I'm not sure how much backwards compatibility is required.
> >>>> Maybe others can comment on this.
> >>>>
> >>>> Thanks,
> >>>> Timo
> >>>>
> >>>> On 30.04.20 14:09, Leonard Xu wrote:
> >>>>> Hi, dear community
> >>>>>
> >>>>> Recently, I’m thinking to refactor the flink-jdbc connector structure
> >>>> before release 1.11.
> >>>>> After the refactor, in the future,  we can easily introduce unified
> >>>> pluggable JDBC dialect for Table and DataStream, and we can have a
> >> better
> >>>> module organization and implementations.
> >>>>>
> >>>>> So, I propose following changes:
> >>>>> 1) Use `Jdbc` instead of `JDBC` in the new public API and interface
> >>>> name. The Datastream API `JdbcSink` which imported in this version has
> >>>> followed this standard.
> >>>>>
> >>>>> 2) Move all interface and classes from `org.apache.flink.java.io
> >>> .jdbc`(old
> >>>> package) to `org.apache.flink.connector.jdbc`(new package) to follow
> >> the
> >>>> base connector path in FLIP-27.
> >>>>> I think we can move JDBC TableSource, TableSink and factory from old
> >>>> package to new package because TableEnvironment#registerTableSource、
> >>>> TableEnvironment#registerTableSink  will be removed in 1.11 ans these
> >>>> classes are not exposed to users[1].
> >>>>> We can move Datastream API JdbcSink from old package to new package
> >>>> because it’s  introduced in this version.
> >>>>> We will still keep `JDBCInputFormat` and `JDBCOoutoutFormat` in old
> >>>> package and deprecate them.
> >>>>> Other classes/interfaces are internal used and we can move to new
> >>>> package without breaking compatibility.
> >>>>> 3) Rename `flink-jdbc` to `flink-connector-jdbc`. well, this is a
> >>>> compatibility broken change but in order to comply with other
> >> connectors
> >>>> and it’s real a connector rather than a flink-jdc-driver[2] we’d
> better
> >>>> decide do it ASAP.
> >>>>>
> >>>>>
> >>>>> What do you think? Any feedback is appreciate.
> >>>>>
> >>>>>
> >>>>> Best,
> >>>>> Leonard Xu
> >>>>>
> >>>>> [1]
> >>>>
> >>>
> >>
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
> >>>> <
> >>>>
> >>>
> >>
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
> >>>>>
> >>>>> [2]https://github.com/ververica/flink-jdbc-driver <
> >>>> https://github.com/ververica/flink-jdbc-driver>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
>
>

Re: [DISCUSS]Refactor flink-jdbc connector structure

Posted by Leonard Xu <xb...@gmail.com>.
Thanks all for involved the discussion. 

All opinions are +1 and no -1 until now, so I create a jira ticket to track[1].

@Jark Wu @Jinsong lee 
Thanks for your suggestions on compatibility,  ancient JDBCOutputFormat, JDBCInputFormat and ParameterValuesProvider
will keep in old package, and I’ll cc you when the PR is ready, hope you can take a little time to help review.

@Jingsong lee
As for flink-hbase connector, it encounters same issue with flink-jdbc. They’re the only two out-of-style cases in connector family,
 I’m +1 to refactor flink-hbase module and the refactor will be similar.

 I create a jira ticket[2] for flink-hbase, but I think we may need some discussion about the detail of refactor,
 we could do this in the jira or open another mail thread if necessary.  



Best,
Leonard Xu
[1] https://issues.apache.org/jira/browse/FLINK-17537 <https://issues.apache.org/jira/browse/FLINK-17537>
[2] https://issues.apache.org/jira/browse/FLINK-17538 <https://issues.apache.org/jira/browse/FLINK-17538>




> 在 2020年5月6日,10:05,Jingsong Li <ji...@gmail.com> 写道:
> 
> Thanks Leonard for starting this discussion.
> 
> +1 from my side.
> 
>> JDBCOutputFormat
> Already deprecated, so let it in old package.
> 
>> JDBCInputFormat
> Not only input format, but also ParameterValuesProvider, looks good to me
> for keeping them in old package.
> You can wait until the next time you refactor it. As
> `JdbcBatchingOutputFormat` does.
> 
> `flink-hbase` need be changed to `flink-connector-hbase` too.
> 
> Best,
> Jingsong Lee
> 
> On Wed, May 6, 2020 at 9:44 AM zoudan <zo...@163.com> wrote:
> 
>> +1 from my side, it will make jdbc connector unified with other ones.
>> 
>> Best,
>> Dan Zou
>> 
>> 
>>> 在 2020年4月30日,23:16,Jark Wu <im...@gmail.com> 写道:
>>> 
>>> Big +1 from my side.
>>> The new structure and class names look nicer now.
>>> 
>>> Regarding to the compability problem, I have looked into the public APIs
>> in
>>> flink-jdbc, there are 3 kinds of APIs now:
>>> 1) new introduced JdbcSink for DataStream users in 1.11
>>> 2) JDBCAppendTableSink, JDBCUpsertTableSink, JDBCTableSource are
>> introduced
>>> since 1.9
>>> 3) very ancient JDBCOutputFormat and JDBCInputFormat
>>> 
>>> For (1), as it's an un-released API, so I think it's safe to move to new
>>> package. cc @Khachatryan Roman <kh...@gmail.com> who
>>> contributed this.
>>> For (2), because TableSource and TableSink are not designed to be
>> accessed
>>> by users since 1.11, so I think it's fine to move them.
>>> For (3), I'm not sure how many users are still using these out-of-date
>>> classes.
>>> But I think it's fine to keep them for one more version, and drop them in
>>> the next version.
>>> 
>>> 
>>> Best,
>>> Jark
>>> 
>>> On Thu, 30 Apr 2020 at 22:57, Flavio Pompermaier <po...@okkam.it>
>>> wrote:
>>> 
>>>> Very big +1 from me
>>>> 
>>>> Best,
>>>> Flavio
>>>> 
>>>> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <da...@alpinegizmo.com>
>>>> wrote:
>>>> 
>>>>> I'm very happy to see the jdbc connector being normalized in this way.
>> +1
>>>>> from me.
>>>>> 
>>>>> David
>>>>> 
>>>>> On Thu, Apr 30, 2020 at 2:14 PM Timo Walther <tw...@apache.org>
>> wrote:
>>>>> 
>>>>>> Hi Leonard,
>>>>>> 
>>>>>> this sounds like a nice refactoring for consistency. +1 from my side.
>>>>>> 
>>>>>> However, I'm not sure how much backwards compatibility is required.
>>>>>> Maybe others can comment on this.
>>>>>> 
>>>>>> Thanks,
>>>>>> Timo
>>>>>> 
>>>>>> On 30.04.20 14:09, Leonard Xu wrote:
>>>>>>> Hi, dear community
>>>>>>> 
>>>>>>> Recently, I’m thinking to refactor the flink-jdbc connector structure
>>>>>> before release 1.11.
>>>>>>> After the refactor, in the future,  we can easily introduce unified
>>>>>> pluggable JDBC dialect for Table and DataStream, and we can have a
>>>> better
>>>>>> module organization and implementations.
>>>>>>> 
>>>>>>> So, I propose following changes:
>>>>>>> 1) Use `Jdbc` instead of `JDBC` in the new public API and interface
>>>>>> name. The Datastream API `JdbcSink` which imported in this version has
>>>>>> followed this standard.
>>>>>>> 
>>>>>>> 2) Move all interface and classes from `org.apache.flink.java.io
>>>>> .jdbc`(old
>>>>>> package) to `org.apache.flink.connector.jdbc`(new package) to follow
>>>> the
>>>>>> base connector path in FLIP-27.
>>>>>>> I think we can move JDBC TableSource, TableSink and factory from old
>>>>>> package to new package because TableEnvironment#registerTableSource、
>>>>>> TableEnvironment#registerTableSink  will be removed in 1.11 ans these
>>>>>> classes are not exposed to users[1].
>>>>>>> We can move Datastream API JdbcSink from old package to new package
>>>>>> because it’s  introduced in this version.
>>>>>>> We will still keep `JDBCInputFormat` and `JDBCOoutoutFormat` in old
>>>>>> package and deprecate them.
>>>>>>> Other classes/interfaces are internal used and we can move to new
>>>>>> package without breaking compatibility.
>>>>>>> 3) Rename `flink-jdbc` to `flink-connector-jdbc`. well, this is a
>>>>>> compatibility broken change but in order to comply with other
>>>> connectors
>>>>>> and it’s real a connector rather than a flink-jdc-driver[2] we’d
>> better
>>>>>> decide do it ASAP.
>>>>>>> 
>>>>>>> 
>>>>>>> What do you think? Any feedback is appreciate.
>>>>>>> 
>>>>>>> 
>>>>>>> Best,
>>>>>>> Leonard Xu
>>>>>>> 
>>>>>>> [1]
>>>>>> 
>>>>> 
>>>> 
>> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
>>>>>> <
>>>>>> 
>>>>> 
>>>> 
>> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
>>>>>>> 
>>>>>>> [2]https://github.com/ververica/flink-jdbc-driver <
>>>>>> https://github.com/ververica/flink-jdbc-driver>
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>> 
>> 
> 
> -- 
> Best, Jingsong Lee


Re: [DISCUSS]Refactor flink-jdbc connector structure

Posted by Jingsong Li <ji...@gmail.com>.
Thanks Leonard for starting this discussion.

+1 from my side.

> JDBCOutputFormat
Already deprecated, so let it in old package.

> JDBCInputFormat
Not only input format, but also ParameterValuesProvider, looks good to me
for keeping them in old package.
You can wait until the next time you refactor it. As
`JdbcBatchingOutputFormat` does.

`flink-hbase` need be changed to `flink-connector-hbase` too.

Best,
Jingsong Lee

On Wed, May 6, 2020 at 9:44 AM zoudan <zo...@163.com> wrote:

> +1 from my side, it will make jdbc connector unified with other ones.
>
> Best,
> Dan Zou
>
>
> > 在 2020年4月30日,23:16,Jark Wu <im...@gmail.com> 写道:
> >
> > Big +1 from my side.
> > The new structure and class names look nicer now.
> >
> > Regarding to the compability problem, I have looked into the public APIs
> in
> > flink-jdbc, there are 3 kinds of APIs now:
> > 1) new introduced JdbcSink for DataStream users in 1.11
> > 2) JDBCAppendTableSink, JDBCUpsertTableSink, JDBCTableSource are
> introduced
> > since 1.9
> > 3) very ancient JDBCOutputFormat and JDBCInputFormat
> >
> > For (1), as it's an un-released API, so I think it's safe to move to new
> > package. cc @Khachatryan Roman <kh...@gmail.com> who
> > contributed this.
> > For (2), because TableSource and TableSink are not designed to be
> accessed
> > by users since 1.11, so I think it's fine to move them.
> > For (3), I'm not sure how many users are still using these out-of-date
> > classes.
> > But I think it's fine to keep them for one more version, and drop them in
> > the next version.
> >
> >
> > Best,
> > Jark
> >
> > On Thu, 30 Apr 2020 at 22:57, Flavio Pompermaier <po...@okkam.it>
> > wrote:
> >
> >> Very big +1 from me
> >>
> >> Best,
> >> Flavio
> >>
> >> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <da...@alpinegizmo.com>
> >> wrote:
> >>
> >>> I'm very happy to see the jdbc connector being normalized in this way.
> +1
> >>> from me.
> >>>
> >>> David
> >>>
> >>> On Thu, Apr 30, 2020 at 2:14 PM Timo Walther <tw...@apache.org>
> wrote:
> >>>
> >>>> Hi Leonard,
> >>>>
> >>>> this sounds like a nice refactoring for consistency. +1 from my side.
> >>>>
> >>>> However, I'm not sure how much backwards compatibility is required.
> >>>> Maybe others can comment on this.
> >>>>
> >>>> Thanks,
> >>>> Timo
> >>>>
> >>>> On 30.04.20 14:09, Leonard Xu wrote:
> >>>>> Hi, dear community
> >>>>>
> >>>>> Recently, I’m thinking to refactor the flink-jdbc connector structure
> >>>> before release 1.11.
> >>>>> After the refactor, in the future,  we can easily introduce unified
> >>>> pluggable JDBC dialect for Table and DataStream, and we can have a
> >> better
> >>>> module organization and implementations.
> >>>>>
> >>>>> So, I propose following changes:
> >>>>> 1) Use `Jdbc` instead of `JDBC` in the new public API and interface
> >>>> name. The Datastream API `JdbcSink` which imported in this version has
> >>>> followed this standard.
> >>>>>
> >>>>> 2) Move all interface and classes from `org.apache.flink.java.io
> >>> .jdbc`(old
> >>>> package) to `org.apache.flink.connector.jdbc`(new package) to follow
> >> the
> >>>> base connector path in FLIP-27.
> >>>>> I think we can move JDBC TableSource, TableSink and factory from old
> >>>> package to new package because TableEnvironment#registerTableSource、
> >>>> TableEnvironment#registerTableSink  will be removed in 1.11 ans these
> >>>> classes are not exposed to users[1].
> >>>>> We can move Datastream API JdbcSink from old package to new package
> >>>> because it’s  introduced in this version.
> >>>>> We will still keep `JDBCInputFormat` and `JDBCOoutoutFormat` in old
> >>>> package and deprecate them.
> >>>>> Other classes/interfaces are internal used and we can move to new
> >>>> package without breaking compatibility.
> >>>>> 3) Rename `flink-jdbc` to `flink-connector-jdbc`. well, this is a
> >>>> compatibility broken change but in order to comply with other
> >> connectors
> >>>> and it’s real a connector rather than a flink-jdc-driver[2] we’d
> better
> >>>> decide do it ASAP.
> >>>>>
> >>>>>
> >>>>> What do you think? Any feedback is appreciate.
> >>>>>
> >>>>>
> >>>>> Best,
> >>>>> Leonard Xu
> >>>>>
> >>>>> [1]
> >>>>
> >>>
> >>
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
> >>>> <
> >>>>
> >>>
> >>
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-Remove-registration-of-TableSource-TableSink-in-Table-Env-and-ConnectTableDescriptor-td37270.html
> >>>>>
> >>>>> [2]https://github.com/ververica/flink-jdbc-driver <
> >>>> https://github.com/ververica/flink-jdbc-driver>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
>
>

-- 
Best, Jingsong Lee