XiaoJiang521 commented on code in PR #5096:
URL: https://github.com/apache/seatunnel/pull/5096#discussion_r1286624758


##########
seatunnel-connectors-v2/connector-jdbc/src/main/java/org/apache/seatunnel/connectors/seatunnel/jdbc/catalog/AbstractJdbcCatalog.java:
##########
@@ -88,58 +98,107 @@ public AbstractJdbcCatalog(
         this.defaultUrl = urlInfo.getOrigin();
         this.suffix = urlInfo.getSuffix();
         this.defaultSchema = Optional.ofNullable(defaultSchema);
+        this.connectionMap = new ConcurrentHashMap<>();
     }
 
+    protected abstract String getCatalogType();
+
     @Override
     public String getDefaultDatabase() {
         return defaultDatabase;
     }
 
-    public String getCatalogName() {
-        return catalogName;
+    protected Connection getConnection(String url) {
+        if (connectionMap.containsKey(url)) {
+            return connectionMap.get(url);
+        }
+        try {
+            Connection connection = DriverManager.getConnection(url, username, 
pwd);
+            connectionMap.put(url, connection);
+            return connection;
+        } catch (SQLException e) {
+            throw new CatalogException(String.format("Failed connecting to %s 
via JDBC.", url), e);
+        }
     }
 
-    public String getUsername() {
-        return username;
+    @Override
+    public void open() throws CatalogException {
+        getConnection(defaultUrl);
+        LOG.info("Catalog {} established connection to {}", catalogName, 
defaultUrl);
     }
 
-    public String getPassword() {
-        return pwd;
+    @Override
+    public void close() throws CatalogException {
+        for (Map.Entry<String, Connection> entry : connectionMap.entrySet()) {
+            try {
+                entry.getValue().close();
+            } catch (SQLException e) {
+                throw new CatalogException(
+                        String.format("Failed to close %s via JDBC.", 
entry.getKey()), e);
+            }
+        }
+        connectionMap.clear();
+        LOG.info("Catalog {} closing", catalogName);
     }

Review Comment:
   
   <img width="979" alt="image" 
src="https://github.com/apache/seatunnel/assets/131635688/9d0324cf-d0df-4f69-9b86-5bfe2d7d0d19";>
   Wouldn't it be problematic to abstract this part? For example, closing the 
catalog in MySQL, but the catalogs in Oracle and PostgreSQL are still in use. 
Wouldn't closing like this cause the connections in Oracle and PostgreSQL to be 
closed as well? @whhe 



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