Skip to content

Commit

Permalink
[#6630] fix(jdbc-catalog): jdbc.pool.test-on-borrow does not work whe…
Browse files Browse the repository at this point in the history
…n connecting to JDBC catalog (#6639)

<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
   Examples:
     - "[#123] feat(operator): support xxx"
     - "[#233] fix: check null before access result in xxx"
     - "[MINOR] refactor: fix typo in variable name"
     - "[MINOR] docs: fix typo in README"
     - "[#255] test: fix flaky test NameOfTheTest"
   Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->

### What changes were proposed in this pull request?

jdbc.pool.test-on-borrow does not work when connecting to JDBC catalog

### Why are the changes needed?

Fix: #6630

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

N/A
  • Loading branch information
sunxiaojian authored Mar 9, 2025
1 parent 0616fbc commit 0c9f581
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.jdbc;

import static org.apache.gravitino.connector.PropertyEntry.booleanPropertyEntry;
import static org.apache.gravitino.connector.PropertyEntry.integerPropertyEntry;
import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry;
import static org.apache.gravitino.connector.PropertyEntry.stringPropertyEntry;
Expand All @@ -42,7 +43,8 @@ public class JdbcCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata
JdbcConfig.USERNAME.getKey(),
JdbcConfig.PASSWORD.getKey(),
JdbcConfig.POOL_MIN_SIZE.getKey(),
JdbcConfig.POOL_MAX_SIZE.getKey());
JdbcConfig.POOL_MAX_SIZE.getKey(),
JdbcConfig.TEST_ON_BORROW.getKey());

static {
List<PropertyEntry<?>> propertyEntries =
Expand Down Expand Up @@ -100,6 +102,14 @@ public class JdbcCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata
false /* immutable */,
JdbcConfig.POOL_MAX_SIZE.getDefaultValue(),
true /* hidden */,
false /* reserved */),
booleanPropertyEntry(
JdbcConfig.TEST_ON_BORROW.getKey(),
JdbcConfig.TEST_ON_BORROW.getDoc(),
false /* required */,
false /* immutable */,
JdbcConfig.TEST_ON_BORROW.getDefaultValue(),
true /* hidden */,
false /* reserved */));
PROPERTIES_METADATA =
ImmutableMap.<String, PropertyEntry<?>>builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,20 @@
package org.apache.gravitino.catalog.jdbc;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.sql.SQLException;
import java.util.HashMap;
import javax.sql.DataSource;
import org.apache.commons.dbcp2.BasicDataSource;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.catalog.jdbc.converter.SqliteColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteExceptionConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteTypeConverter;
import org.apache.gravitino.catalog.jdbc.operation.SqliteDatabaseOperations;
import org.apache.gravitino.catalog.jdbc.operation.SqliteTableOperations;
import org.apache.gravitino.catalog.jdbc.utils.DataSourceUtils;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
Expand All @@ -51,4 +58,20 @@ public void testTestConnection() {
"comment",
ImmutableMap.of()));
}

@Test
public void testConfigTestOnBorrow() throws SQLException {
HashMap<String, String> properties = Maps.newHashMap();
properties.put(JdbcConfig.JDBC_DRIVER.getKey(), "org.sqlite.JDBC");
properties.put(JdbcConfig.JDBC_URL.getKey(), "jdbc:sqlite::memory:");
properties.put(JdbcConfig.USERNAME.getKey(), "test");
properties.put(JdbcConfig.PASSWORD.getKey(), "test");
properties.put(JdbcConfig.TEST_ON_BORROW.getKey(), "false");

DataSource dataSource =
Assertions.assertDoesNotThrow(() -> DataSourceUtils.createDataSource(properties));
Assertions.assertTrue(dataSource instanceof org.apache.commons.dbcp2.BasicDataSource);
Assertions.assertTrue(((BasicDataSource) dataSource).getTestOnBorrow() == false);
((BasicDataSource) dataSource).close();
}
}

0 comments on commit 0c9f581

Please sign in to comment.