[DISCUSS]Refactor flink-jdbc connector structure

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS]Refactor flink-jdbc connector structure

Leonard Xu
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>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

Timo Walther-2
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>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

David Anderson-3
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 <[hidden email]> 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>
> >
> >
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

Flavio Pompermaier
Very big +1 from me

Best,
Flavio

On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
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 <[hidden email]> 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>
> > >
> > >
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

Jark Wu-2
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 <[hidden email]> 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 <[hidden email]>
wrote:

> Very big +1 from me
>
> Best,
> Flavio
>
> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
> 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 <[hidden email]> 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>
> > > >
> > > >
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

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

Best,
Dan Zou


> 在 2020年4月30日,23:16,Jark Wu <[hidden email]> 写道:
>
> 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 <[hidden email]> 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 <[hidden email]>
> wrote:
>
>> Very big +1 from me
>>
>> Best,
>> Flavio
>>
>> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
>> 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 <[hidden email]> 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>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

felixzheng
That sounds like a good refactoring for consistency, +1 from my side

Regards,
Canbin Zheng

zoudan <[hidden email]> 于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 <[hidden email]> 写道:
> >
> > 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 <[hidden email]> 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 <[hidden email]>
> > wrote:
> >
> >> Very big +1 from me
> >>
> >> Best,
> >> Flavio
> >>
> >> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
> >> 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 <[hidden email]>
> 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>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

Jingsong Li
In reply to this post by zoudan
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 <[hidden email]> wrote:

> +1 from my side, it will make jdbc connector unified with other ones.
>
> Best,
> Dan Zou
>
>
> > 在 2020年4月30日,23:16,Jark Wu <[hidden email]> 写道:
> >
> > 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 <[hidden email]> 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 <[hidden email]>
> > wrote:
> >
> >> Very big +1 from me
> >>
> >> Best,
> >> Flavio
> >>
> >> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
> >> 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 <[hidden email]>
> 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
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS]Refactor flink-jdbc connector structure

Leonard Xu
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 <[hidden email]> 写道:
>
> 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 <[hidden email]> wrote:
>
>> +1 from my side, it will make jdbc connector unified with other ones.
>>
>> Best,
>> Dan Zou
>>
>>
>>> 在 2020年4月30日,23:16,Jark Wu <[hidden email]> 写道:
>>>
>>> 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 <[hidden email]> 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 <[hidden email]>
>>> wrote:
>>>
>>>> Very big +1 from me
>>>>
>>>> Best,
>>>> Flavio
>>>>
>>>> On Thu, Apr 30, 2020 at 4:47 PM David Anderson <[hidden email]>
>>>> 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 <[hidden email]>
>> 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