branch-4.1: [feat](paimon) support jdbc catalog type #61094#61694
branch-4.1: [feat](paimon) support jdbc catalog type #61094#61694yiguolei merged 1 commit intoapache:branch-4.1from
Conversation
## Summary - add Paimon JDBC metastore properties and driver loading support - register paimon.catalog.type=jdbc in paimon properties/catalog factory - allow paimon jdbc type in external table/system table thrift conversion - add FE unit tests for jdbc properties mapping and validation ## Test - ./run-fe-ut.sh --run org.apache.doris.datasource.property.metastore.PaimonJdbcMetaStorePropertiesTest (cherry picked from commit 79153d3)
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Pull request overview
Cherry-pick of #61094 to add support for using a JDBC-backed metastore for Paimon external catalogs/tables in FE, including driver loading and test coverage.
Changes:
- Add
paimon.catalog.type=jdbcsupport via newPaimonJdbcMetaStorePropertiesand factory registration. - Allow Paimon JDBC catalogs to pass through FE→BE thrift conversion for external/system tables.
- Add FE unit tests and an external regression test validating JDBC catalog behavior and basic read/write.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| regression-test/suites/external_table_p0/paimon/test_paimon_jdbc_catalog.groovy | New regression test that creates a Paimon JDBC catalog and validates reads after Spark writes. |
| regression-test/data/external_table_p0/paimon/test_paimon_jdbc_catalog.out | Golden output for the new regression query test. |
| fe/fe-core/src/test/java/org/apache/doris/datasource/property/metastore/PaimonJdbcMetaStorePropertiesTest.java | New FE UTs for JDBC property mapping/validation and factory creation. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonPropertiesFactory.java | Register jdbc subtype for Paimon metastore properties. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/PaimonJdbcMetaStoreProperties.java | New implementation for Paimon JDBC metastore properties, options building, and driver loading. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonSysExternalTable.java | Treat JDBC catalogs as supported for thrift conversion. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java | Treat JDBC catalogs as supported for thrift conversion. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalogFactory.java | Allow creating PAIMON_JDBC catalogs. |
| fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java | Add PAIMON_JDBC catalog type constant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String driverKey = fullDriverUrl + "#" + driverClassName; | ||
| if (!REGISTERED_DRIVER_KEYS.add(driverKey)) { | ||
| LOG.info("JDBC driver already registered for Paimon catalog: {} from {}", | ||
| driverClassName, fullDriverUrl); | ||
| return; |
There was a problem hiding this comment.
REGISTERED_DRIVER_KEYS.add(driverKey) happens before driver class loading/registration, but the key is only removed on caught Exception. If registration fails with an Error (e.g., NoClassDefFoundError / LinkageError from a bad JAR), the key can remain and future attempts will incorrectly short-circuit as “already registered”. Consider ensuring the key is removed on any failure (e.g., via try/finally that only keeps the key on success, or by handling broader failure types).
| private void registerJdbcDriver(String driverUrl, String driverClassName) { | ||
| try { | ||
| if (StringUtils.isBlank(driverClassName)) { | ||
| throw new IllegalArgumentException( | ||
| "jdbc.driver_class or paimon.jdbc.driver_class is required when jdbc.driver_url " |
There was a problem hiding this comment.
The dynamic JDBC driver registration implementation (registerJdbcDriver/DriverShim and related caches) is very similar to the existing implementation in IcebergJdbcMetaStoreProperties. Keeping two near-identical implementations tends to cause long-term drift and missed fixes in one path. Consider extracting a shared helper for driver URL normalization + classloader caching + DriverManager registration and reuse it from both classes.
| def proc = new ProcessBuilder("/bin/bash", "-c", cmd).redirectErrorStream(true).start() | ||
| int exitcode = proc.waitFor() | ||
| String output = proc.text | ||
| if (exitcode != 0) { | ||
| logger.info("exit code: ${exitcode}, output\n: ${output}") |
There was a problem hiding this comment.
executeCommand uses proc.waitFor() with no timeout, so a hung curl/docker/spark-sql command can block the entire regression run indefinitely. Also the catch (IOException) path is labeled as a timeout, but waitFor() does not throw IOException. Consider using waitFor(timeout, TimeUnit) (or Groovy’s waitForOrKill) and handling InterruptedException, reporting the real exception message when command execution fails.
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
Cherry-picked from #61094