sjwiesman commented on a change in pull request #17264:
URL: https://github.com/apache/flink/pull/17264#discussion_r710258559



##########
File path: 
flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/dialect/JdbcDialects.java
##########
@@ -18,23 +18,25 @@
 
 package org.apache.flink.connector.jdbc.dialect;
 
-import java.util.Arrays;
-import java.util.List;
+import org.apache.flink.annotation.Internal;
+
 import java.util.Optional;
+import java.util.ServiceLoader;
 
 /** Default JDBC dialects. */
+@Internal
 public final class JdbcDialects {
 
-    private static final List<JdbcDialect> DIALECTS =
-            Arrays.asList(new DerbyDialect(), new MySQLDialect(), new 
PostgresDialect());
-
     /** Fetch the JdbcDialect class corresponding to a given database url. */
     public static Optional<JdbcDialect> get(String url) {
-        for (JdbcDialect dialect : DIALECTS) {
-            if (dialect.canHandle(url)) {
-                return Optional.of(dialect);
+        ClassLoader cl = Thread.currentThread().getContextClassLoader();
+
+        for (JdbcDialectFactory factory : 
ServiceLoader.load(JdbcDialectFactory.class, cl)) {
+            if (factory.canHandle(url)) {

Review comment:
       yep, I'm just going to copy the logic of FactoryUtil. I also did a pass 
and realized nothing that uses this class actually handles `Optional.empty` 
gracefully, they all just throw. That makes it simpler to just throw if we 
don't find exactly one dialect on the classpath. 




-- 
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]


Reply via email to