This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-2b41cab6-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 1810fca3b17c515f2ac27a6cb4ee71c09339e991 Author: roryqi <[email protected]> AuthorDate: Tue Oct 20 09:48:39 2026 +0800 [MINOR] fix(server): Default JDBC drivers in catalog REST APIs (#13942) ### What changes were proposed in this pull request? Add JDBC driver defaults to catalog creation and connection tests after merging provider configuration. Derive the driver from the provider, from the URI for an Iceberg JDBC backend, or from the JDBC URL for OceanBase. Preserve a driver supplied in the request or provider configuration. ### Why are the changes needed? REST callers currently need to know and supply `jdbc-driver` even when the server can determine it. This causes otherwise valid requests to fail. ### Does this PR introduce _any_ user-facing change? The two public REST APIs can now accept supported JDBC catalog requests without `jdbc-driver`. No property keys are added. ### How was this patch tested? Added tests for driver selection and configuration precedence. Ran `./gradlew :core:spotlessApply :server:spotlessApply :core:test --tests org.apache.gravitino.catalog.TestJdbcDriverProperties --tests org.apache.gravitino.catalog.TestCatalogManager :server:test --tests org.apache.gravitino.server.web.rest.TestCatalogOperations -PskipITs`. --- .../apache/gravitino/catalog/CatalogManager.java | 5 +- .../gravitino/catalog/JdbcDriverProperties.java | 94 ++++++++++++++++++++ .../gravitino/catalog/TestCatalogManager.java | 39 +++++++++ .../catalog/TestJdbcDriverProperties.java | 99 ++++++++++++++++++++++ 4 files changed, 235 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java index 599d9fc735..153e8b7002 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -1604,11 +1604,12 @@ public class CatalogManager implements CatalogDispatcher, Closeable { return builder.withAuditInfo(newInfo); } - private Map<String, String> buildCatalogConf(String provider, Map<String, String> properties) { + Map<String, String> buildCatalogConf(String provider, Map<String, String> properties) { Map<String, String> newProperties = Optional.ofNullable(properties).orElse(Maps.newHashMap()); // load catalog-related configuration from catalog-specific configuration file Map<String, String> catalogSpecificConfig = loadCatalogSpecificConfig(newProperties, provider); - return mergeConf(newProperties, catalogSpecificConfig); + return JdbcDriverProperties.withDefaultDriver( + provider, mergeConf(newProperties, catalogSpecificConfig)); } private Pair<Map<String, String>, Map<String, String>> getCatalogAlterProperty( diff --git a/core/src/main/java/org/apache/gravitino/catalog/JdbcDriverProperties.java b/core/src/main/java/org/apache/gravitino/catalog/JdbcDriverProperties.java new file mode 100644 index 0000000000..1dfa360135 --- /dev/null +++ b/core/src/main/java/org/apache/gravitino/catalog/JdbcDriverProperties.java @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog; + +import com.google.common.collect.ImmutableMap; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import javax.annotation.Nullable; + +/** Adds the known JDBC driver class to catalog properties when the caller omits it. */ +public final class JdbcDriverProperties { + + private static final String JDBC_DRIVER = "jdbc-driver"; + private static final Map<String, String> PROVIDER_DRIVERS = + ImmutableMap.<String, String>builder() + .put("jdbc-clickhouse", "com.clickhouse.jdbc.ClickHouseDriver") + .put("jdbc-doris", "com.mysql.cj.jdbc.Driver") + .put("jdbc-hologres", "org.postgresql.Driver") + .put("jdbc-mysql", "com.mysql.cj.jdbc.Driver") + .put("jdbc-postgresql", "org.postgresql.Driver") + .put("jdbc-starrocks", "com.mysql.cj.jdbc.Driver") + .build(); + + private JdbcDriverProperties() {} + + /** + * Returns properties with a derived driver, leaving the input map unchanged. + * + * @param provider catalog provider + * @param properties requested catalog properties + * @return properties with a driver when one can be inferred + */ + @Nullable + public static Map<String, String> withDefaultDriver( + @Nullable String provider, @Nullable Map<String, String> properties) { + if (provider == null || (properties != null && properties.get(JDBC_DRIVER) != null)) { + return properties; + } + + String driver = PROVIDER_DRIVERS.get(provider.toLowerCase(Locale.ROOT)); + if ("jdbc-oceanbase".equalsIgnoreCase(provider)) { + driver = null; + String url = properties == null ? null : properties.get("jdbc-url"); + if (url != null) { + String normalizedUrl = url.trim().toLowerCase(Locale.ROOT); + if (normalizedUrl.startsWith("jdbc:oceanbase:")) { + driver = "com.oceanbase.jdbc.Driver"; + } else if (normalizedUrl.startsWith("jdbc:mysql:")) { + driver = "com.mysql.cj.jdbc.Driver"; + } + } + } + if (driver == null && "lakehouse-iceberg".equalsIgnoreCase(provider) && properties != null) { + if ("jdbc".equalsIgnoreCase(properties.get("catalog-backend"))) { + String uri = properties.get("uri"); + if (uri != null) { + String normalizedUri = uri.trim().toLowerCase(Locale.ROOT); + if (normalizedUri.startsWith("jdbc:mysql:")) { + driver = "com.mysql.cj.jdbc.Driver"; + } else if (normalizedUri.startsWith("jdbc:postgresql:")) { + driver = "org.postgresql.Driver"; + } + } + } + } + + if (driver == null) { + return properties; + } + Map<String, String> result = new HashMap<>(); + if (properties != null) { + result.putAll(properties); + } + result.put(JDBC_DRIVER, driver); + return result; + } +} diff --git a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java index c3f9aa4a67..d64aff46de 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java @@ -34,6 +34,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -95,6 +98,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; @@ -159,6 +163,41 @@ public class TestCatalogManager { } } + @Test + void testJdbcDriverDefaultAfterProviderConfigMerge(@TempDir Path catalogPackage) + throws IOException { + Path conf = Files.createDirectories(catalogPackage.resolve("conf")); + Path configFile = conf.resolve("jdbc-oceanbase.conf"); + Files.write(configFile, "jdbc-driver=configured.Driver\n".getBytes(StandardCharsets.UTF_8)); + Map<String, String> request = + ImmutableMap.of( + Catalog.PROPERTY_PACKAGE, + catalogPackage.toString(), + "jdbc-url", + "jdbc:oceanbase://localhost:2881/test"); + + Assertions.assertEquals( + "configured.Driver", + catalogManager.buildCatalogConf("jdbc-oceanbase", request).get("jdbc-driver")); + + Map<String, String> explicit = Maps.newHashMap(request); + explicit.put("jdbc-driver", "request.Driver"); + Assertions.assertEquals( + "request.Driver", + catalogManager.buildCatalogConf("jdbc-oceanbase", explicit).get("jdbc-driver")); + + Files.write( + configFile, + "jdbc-url=jdbc:oceanbase://localhost:2881/test\n".getBytes(StandardCharsets.UTF_8)); + Assertions.assertEquals( + "com.oceanbase.jdbc.Driver", + catalogManager + .buildCatalogConf( + "jdbc-oceanbase", + ImmutableMap.of(Catalog.PROPERTY_PACKAGE, catalogPackage.toString())) + .get("jdbc-driver")); + } + @Test void testPropertyValidationInAlter() throws IOException { // key1 is required and immutable and do not have default value, is not hidden and not reserved diff --git a/core/src/test/java/org/apache/gravitino/catalog/TestJdbcDriverProperties.java b/core/src/test/java/org/apache/gravitino/catalog/TestJdbcDriverProperties.java new file mode 100644 index 0000000000..5006af718c --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/catalog/TestJdbcDriverProperties.java @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog; + +import com.google.common.collect.ImmutableMap; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** Tests catalog JDBC driver defaults. */ +public class TestJdbcDriverProperties { + + @Test + void testProviderDefaults() { + Map<String, String> drivers = + ImmutableMap.<String, String>builder() + .put("jdbc-clickhouse", "com.clickhouse.jdbc.ClickHouseDriver") + .put("jdbc-doris", "com.mysql.cj.jdbc.Driver") + .put("jdbc-hologres", "org.postgresql.Driver") + .put("jdbc-mysql", "com.mysql.cj.jdbc.Driver") + .put("jdbc-postgresql", "org.postgresql.Driver") + .put("jdbc-starrocks", "com.mysql.cj.jdbc.Driver") + .build(); + drivers.forEach( + (provider, driver) -> { + Map<String, String> input = ImmutableMap.of("key", "value"); + Map<String, String> result = JdbcDriverProperties.withDefaultDriver(provider, input); + Assertions.assertEquals(driver, result.get("jdbc-driver")); + Assertions.assertEquals("value", result.get("key")); + Assertions.assertFalse(input.containsKey("jdbc-driver")); + }); + } + + @Test + void testIcebergJdbcBackendDefaults() { + Map<String, String> postgres = + ImmutableMap.of("catalog-backend", "jdbc", "uri", "jdbc:postgresql://host/db"); + Assertions.assertEquals( + "org.postgresql.Driver", + JdbcDriverProperties.withDefaultDriver("lakehouse-iceberg", postgres).get("jdbc-driver")); + + Map<String, String> mysql = + ImmutableMap.of("catalog-backend", "jdbc", "uri", "jdbc:mysql://host/db"); + Assertions.assertEquals( + "com.mysql.cj.jdbc.Driver", + JdbcDriverProperties.withDefaultDriver("lakehouse-iceberg", mysql).get("jdbc-driver")); + } + + @Test + void testOceanBaseDriverFollowsJdbcUrl() { + Map<String, String> nativeUrl = + ImmutableMap.of("jdbc-url", "jdbc:oceanbase://localhost:2881/test"); + Assertions.assertEquals( + "com.oceanbase.jdbc.Driver", + JdbcDriverProperties.withDefaultDriver("jdbc-oceanbase", nativeUrl).get("jdbc-driver")); + + Map<String, String> mysqlUrl = ImmutableMap.of("jdbc-url", "jdbc:mysql://localhost:2881/test"); + Assertions.assertEquals( + "com.mysql.cj.jdbc.Driver", + JdbcDriverProperties.withDefaultDriver("jdbc-oceanbase", mysqlUrl).get("jdbc-driver")); + + Map<String, String> unknownUrl = ImmutableMap.of("jdbc-url", "jdbc:unknown://localhost/test"); + Assertions.assertSame( + unknownUrl, JdbcDriverProperties.withDefaultDriver("jdbc-oceanbase", unknownUrl)); + } + + @Test + void testExplicitAndUnknownDrivers() { + Map<String, String> explicit = ImmutableMap.of("jdbc-driver", "custom.Driver"); + Assertions.assertSame(explicit, JdbcDriverProperties.withDefaultDriver("jdbc-mysql", explicit)); + Map<String, String> unknown = + ImmutableMap.of("catalog-backend", "jdbc", "uri", "jdbc:sqlite::memory:"); + Assertions.assertSame( + unknown, JdbcDriverProperties.withDefaultDriver("lakehouse-iceberg", unknown)); + Assertions.assertSame(unknown, JdbcDriverProperties.withDefaultDriver("hive", unknown)); + Assertions.assertSame( + unknown, JdbcDriverProperties.withDefaultDriver("jdbc-sqlserver", unknown)); + Assertions.assertNull(JdbcDriverProperties.withDefaultDriver(null, null)); + Assertions.assertEquals( + "com.mysql.cj.jdbc.Driver", + JdbcDriverProperties.withDefaultDriver("JDBC-MySQL", null).get("jdbc-driver")); + } +}
