Airblader commented on a change in pull request #17332:
URL: https://github.com/apache/flink/pull/17332#discussion_r713600160
##########
File path:
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/JdbcDialect.java
##########
@@ -142,4 +143,16 @@
*/
String getSelectFromStatement(
String tableName, String[] selectFields, String[] conditionFields);
+
+ /** Create catalog instance. */
+ default AbstractJdbcCatalog createCatalog(
Review comment:
Logically this seems misplaced to me, because creating a catalog doesn't
relate to the definition of a dialect. However, I don't have a better
suggestion either, we shouldn't introduce even more custom infrastructure.
##########
File path:
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/JdbcDialect.java
##########
@@ -142,4 +143,16 @@
*/
String getSelectFromStatement(
String tableName, String[] selectFields, String[] conditionFields);
+
+ /** Create catalog instance. */
+ default AbstractJdbcCatalog createCatalog(
+ String catalogName,
+ String defaultDatabase,
+ String username,
+ String pwd,
+ String baseUrl) {
+ JdbcDialect dialect = JdbcDialectLoader.load(baseUrl);
+ throw new UnsupportedOperationException(
+ String.format("Catalog for '%s' is not supported yet.",
dialect));
Review comment:
This load call is unnecessary, it'll just find itself (`this`) anyway.
We're already in the loaded dialect here. We can also just directly use
`dialectName`.
```suggestion
String.format("Catalog for JDBC dialect '%s' is not
supported yet.", dialectName()));
```
##########
File path:
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/JdbcDialect.java
##########
@@ -142,4 +143,16 @@
*/
String getSelectFromStatement(
String tableName, String[] selectFields, String[] conditionFields);
+
+ /** Create catalog instance. */
+ default AbstractJdbcCatalog createCatalog(
+ String catalogName,
Review comment:
We should move all of these parameters into a `Context` interface and
pass that so we can easily add more information if we need to without breaking
the API. This is similar to how table factories work, and basically the JDBC
dialect infrastructure is just a clone of table factories anyway.
--
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]
