eskabetxe commented on code in PR #161: URL: https://github.com/apache/flink-connector-jdbc/pull/161#discussion_r2550446708
########## flink-connector-jdbc-core/src/main/java/org/apache/flink/connector/jdbc/core/database/extractor/RowDataResultExtractor.java: ########## Review Comment: org.apache.flink.connector.jdbc.core.database is basically the API for database connectors.. so I think we should avoid that package ########## flink-connector-jdbc-core/src/main/java/org/apache/flink/connector/jdbc/core/datastream/source/reader/extractor/ResultExtractor.java: ########## @@ -16,7 +16,7 @@ * limitations under the License. */ -package org.apache.flink.connector.jdbc.core.datastream.source.reader.extractor; Review Comment: This is a public interface we should not move it so easy. If we are moving it we need to keep an class (deprecated) in origin that extends the moved one to keep retro compatibility, but I would avoid database as is the API for connectors (the things that connectors need to implement) I understand that this refactor is because of the new implementation, but being table using the datastream why don't we put the new clase on the current package? that is a valid implementation also for datastream and useful in some cases (reading from jdbc and sending to paimon/iceberg as example) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
