VendorDatabaseIdProvider should throw exception if DB error occurs

As reported in mybatis#3040, swallowing the SQLException could lead to unexpected statement resolution.

Theoretically, this change might expose a problem in an existing solution that was previously hidden.

Author: harawata

src/main/java/org/apache/ibatis/mapping/VendorDatabaseIdProvider.java

@@ -21,8 +21,7 @@
 
 import javax.sql.DataSource;
 
-import org.apache.ibatis.logging.Log;
-import org.apache.ibatis.logging.LogFactory;
+import org.apache.ibatis.builder.BuilderException;
 
 /**
  * Vendor DatabaseId provider.
@@ -44,10 +43,9 @@ public String getDatabaseId(DataSource dataSource) {
     }
     try {
       return getDatabaseName(dataSource);
-    } catch (Exception e) {
-      LogHolder.log.error("Could not get a databaseId from dataSource", e);
+    } catch (SQLException e) {
+      throw new BuilderException("Error occurred when getting DB product name.", e);
     }
-    return null;
   }
 
   @Override
@@ -70,8 +68,4 @@ private String getDatabaseProductName(DataSource dataSource) throws SQLException
     }
   }
 
-  private static class LogHolder {
-    private static final Log log = LogFactory.getLog(VendorDatabaseIdProvider.class);
-  }
-
 }

src/test/java/org/apache/ibatis/mapping/VendorDatabaseIdProviderTest.java

@@ -17,7 +17,8 @@
 package org.apache.ibatis.mapping;
 
 import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.sql.Connection;
 import java.sql.DatabaseMetaData;
@@ -26,6 +27,7 @@
 
 import javax.sql.DataSource;
 
+import org.apache.ibatis.builder.BuilderException;
 import org.junit.jupiter.api.Test;
 
 class VendorDatabaseIdProviderTest {
@@ -77,7 +79,12 @@ void shouldNullBeReturnedOnDbError() throws Exception {
     VendorDatabaseIdProvider provider = new VendorDatabaseIdProvider();
     Properties properties = new Properties();
     properties.put("Ewok DB", "ewok");
-    assertNull(provider.getDatabaseId(dataSource));
+    try {
+      provider.getDatabaseId(dataSource);
+      fail("Should BuilderException be thrown.");
+    } catch (BuilderException e) {
+      // pass
+    }
   }
 
   private DataSource mockDataSource() throws SQLException {