[fix](fe) Support ClickHouse JDBC V2 metadata#65709
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Requesting changes for one JDBC V2 compatibility gap: ClickHouse 0.9.8 reports MaterializedView engines as MATERIALIZED VIEW, but both Doris ClickHouse allowlists omit that type, so V2 catalogs lose materialized views that V1 exposed as VIEW.
Critical checkpoint conclusions:
- Goal/correctness: The capability-based namespace selection is correct for the inspected 0.4.x, 0.5+/0.7.x, proxy, and direct V2 drivers, and REMOTE TABLE makes the Distributed fixture discoverable and queryable. The missing MATERIALIZED VIEW type leaves the V2 support incomplete.
- Scope/parallel paths: The production change is focused and updates both JDBC client stacks, but the accepted omission exists in both arrays and should be fixed together.
- Concurrency/lifecycle/error handling: No new threads, locks, or special lifetime are added. The shared connector's two-field lazy publication issue was present in the base revision, and no supported-driver exception path newly introduced by this PR was found. JDBC resources and SQLExceptions follow existing client patterns.
- Compatibility/configuration/persistence: No Doris dynamic config, FE-BE protocol, persisted format, transaction, or rolling-upgrade boundary changes. V1/V2 catalog-vs-schema argument propagation is consistent; the blocker is table-type compatibility when upgrading to V2.
- Tests/results: The unit tests cover version/capability decisions and the new type string; the regression uses the real direct V2 class and proves ordinary plus Distributed counts. It does not include a materialized-view fixture, which is why the compatibility loss is missed. Per the review-runner contract, no local build or test was run; static evidence was checked against the exact upstream driver sources and current CI state.
- Performance/observability/security: The capability decision is cached and not a hot-path regression; no extra metrics/logging are needed. The change is read-only metadata behavior over an admin-trusted external catalog and does not alter an in-scope trust boundary.
- Data writes/visibility/FE-BE variables: Not applicable; no transaction, storage visibility, delete bitmap, write atomicity, or new transmitted variable is involved.
- User focus: No additional user-provided focus points were supplied; the complete PR was reviewed.
TPC-H: Total hot run time: 29374 ms |
TPC-DS: Total hot run time: 177743 ms |
ClickBench: Total hot run time: 25.19 s |
44add47 to
888206f
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#65709 Problem Summary: ClickHouse JDBC V2 reports MaterializedView engines as MATERIALIZED VIEW and filters getTables results by requested vendor table types. Doris only requested VIEW, so catalogs upgraded from V1 to V2 could no longer list or query materialized views. Include the V2 table type in both JDBC client paths and cover it with a real-driver regression fixture. ### Release note Preserve ClickHouse materialized view discovery with JDBC V2. ### Check List (For Author) - Test - [x] Regression test - [x] Unit test
|
run buildall |
|
/review |
There was a problem hiding this comment.
Requesting changes for one regression-infrastructure issue. The production-side ClickHouse JDBC V2 metadata fix is otherwise coherent, and the materialized-view omission raised in the existing thread is fixed on the current head.
Critical checkpoint conclusions:
- Goal and correctness: The capability-based namespace selection matches the inspected ClickHouse driver contracts: legacy schema mode remains intact, 0.7.x follows its configured catalog/schema capability, and JDBC V2 0.9.8 is schema-only. Adding
REMOTE TABLEandMATERIALIZED VIEWmakes the new Distributed and materialized-view fixtures discoverable through both JDBC client stacks. - Scope and parallel paths: The production change is focused and updates the legacy
JdbcClickHouseClientand plugin-drivenJdbcClickHouseConnectorClientconsistently. Database listing, table listing/existence, column lookup, and primary-key argument placement were traced end to end. - Concurrency and lifecycle: No new threads, locks, or lifecycle boundary are added. The plugin client's two-field lazy-publication race is real but unchanged from the base revision, so it was not attributed to this PR. Connector creation/reset and JDBC resource closure otherwise follow existing patterns.
- Compatibility, configuration, and persistence: No Doris dynamic configuration, persisted format, edit-log, FE-BE protocol, transaction, or rolling-upgrade contract changes. The non-default ClickHouse 0.4.x catalog-mode limitation also predates this PR; the targeted 0.7.x and 0.9.8 paths are consistent.
- Tests and results: The unit tests cover version/capability decisions and both vendor table types. The ClickHouse 23.8 fixture ordering and local
defaultcluster make all three expected counts of 2 correct, and the suite routes through the SPI JDBC client. However, its only V2 driver is fetched directly from Maven Central before those assertions, bypassing the configured regression artifact store and making the end-to-end proof fail on bucket-only or slow public egress. Per the review-runner contract, no local build or test was run;thirdparty/installed/bin/protocis unavailable. Current compile and style checks were inspected separately. - Performance and observability: The metadata capability decision is cached and not a hot-path regression; no new metric or log is needed.
- Data writes, visibility, and transmitted variables: Not applicable. This changes external metadata discovery and test fixtures, not Doris transaction visibility, storage writes, delete bitmaps, or FE-BE variables.
- User focus: No additional user-provided focus points were supplied; the complete PR was reviewed.
| if (enabled != null && enabled.equalsIgnoreCase("true")) { | ||
| String clickhousePort = context.config.otherConfigs.get("clickhouse_22_port") | ||
| String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") | ||
| String driverUrl = "https://repo.maven.apache.org/maven2/com/clickhouse/clickhouse-jdbc/0.9.8/clickhouse-jdbc-0.9.8-all.jar" |
There was a problem hiding this comment.
[P1] Keep the V2 driver in the regression artifact store
Creating this catalog computes the driver checksum by streaming the complete 12.25 MB jar with JdbcResource's fixed 10-second HTTP timeouts. This is the only active JDBC regression driver URL in this suite that uses Maven Central; the existing ClickHouse and other JDBC cases all use the configured regression bucket. Workers with bucket-only egress (or a slow public route) will therefore fail before any V2 metadata behavior is exercised. Please upload the 0.9.8 all-jar under regression/jdbc_driver and build this URL with getS3BucketName() / getS3Endpoint(), matching the existing ClickHouse suite.
TPC-H: Total hot run time: 29680 ms |
TPC-DS: Total hot run time: 177512 ms |
ClickBench: Total hot run time: 25.06 s |
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: ClickHouse JDBC V2 exposes databases as schemas and reports distributed tables as REMOTE TABLE. Doris inferred the database metadata mode from a legacy URL parameter and filtered the vendor table type, so V2 catalogs could not discover databases or distributed tables. Use JDBC metadata capabilities for catalog/schema mode and include remote tables in discovery.
### Release note
Support ClickHouse JDBC V2 schema metadata and distributed table discovery.
### Check List (For Author)
- Test: Regression test and Unit Test
- JdbcClickHouseClientTest
- JdbcClickHouseConnectorClientTest
- test_clickhouse_jdbc_v2
- Behavior changed: Yes. ClickHouse JDBC V2 catalogs now use schema metadata and expose distributed tables.
- Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65709 Problem Summary: ClickHouse JDBC V2 reports MaterializedView engines as MATERIALIZED VIEW and filters getTables results by requested vendor table types. Doris only requested VIEW, so catalogs upgraded from V1 to V2 could no longer list or query materialized views. Include the V2 table type in both JDBC client paths and cover it with a real-driver regression fixture. ### Release note Preserve ClickHouse materialized view discovery with JDBC V2. ### Check List (For Author) - Test - [x] Regression test - [x] Unit test
### What problem does this PR solve? Issue Number: None Related PR: apache#65709 Problem Summary: The ClickHouse JDBC V2 regression downloaded a 12 MB driver directly from Maven Central, which can fail on workers with restricted or slow public egress. Resolve the driver from the configured regression artifact bucket, consistent with the existing ClickHouse JDBC suites. ### Release note None ### Check List (For Author) - Test - [x] Regression suite URL assertion
888206f to
56d2193
Compare
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
TPC-H: Total hot run time: 29160 ms |
TPC-DS: Total hot run time: 177290 ms |
ClickBench: Total hot run time: 25.02 s |
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Requesting changes for one JDBC connector compatibility regression: a repackaged ClickHouse 0.7.x driver without manifest version metadata worked in the base SPI connector, but the new numeric parser rejects it before namespace metadata can be initialized.
Critical checkpoint conclusions:
- Goal/correctness: Official ClickHouse 0.3/0.4 legacy routing, configurable 0.5/0.7 catalog-or-schema routing, and direct V2 0.9.8 schema routing are consistent across database, table, column, and applicable primary-key metadata.
REMOTE TABLEandMATERIALIZED VIEWcover the new fixtures. MAIN-1 leaves the custom-driver upgrade path incompatible. - Scope/parallel paths: Both the legacy FE client and plugin-driven SPI client were traced through database listing, table listing/existence, columns, and primary keys. The accepted failure is limited to the changed SPI version parser on the active JDBC catalog path; no second changed-path issue survived review.
- Concurrency/lifecycle/error handling: No new supported-driver race or resource-lifetime defect was found; the SPI client's two-field lazy-publication ordering existed in the base revision. MAIN-1 is a deterministic hard failure: existing/replayed catalogs fail discovery after upgrade, default new catalog creation fails its metadata-backed connectivity check, and
test_connection=falsedefers the failure until first metadata access. - Compatibility/configuration/persistence: Doris permits custom driver jars without requiring
Implementation-Version, so strict numeric parsing changes an existing compatibility contract. No new dynamic configuration, persisted-field format, edit-log schema, FE-BE protocol, transaction, or storage-visibility behavior is introduced. - Tests/results: Unit and external regression coverage proves the official parseable drivers and direct V2 Distributed/materialized-view path, but it does not cover an empty reported version. The configured 0.9.8 artifact, SPI route, fixture order, and expected counts were verified. Current-head External Regression, FE UT, compile, CheckStyle, and other reported checks are green; per the review contract, no local build or test was run.
- Performance/observability: Capability selection is cached at client initialization; no hot-path, metric, or logging issue was found.
- Data writes/visibility/transmitted variables: Not applicable; this is external metadata discovery and does not change Doris writes, transaction visibility, delete bitmaps, or FE-BE variables.
- Existing threads/user focus: The earlier materialized-view and artifact-host concerns are fixed on the current head and are not duplicated. No additional user focus was supplied; the complete seven-file patch was reviewed.
### What problem does this PR solve? Issue Number: None Related PR: apache#65709 Problem Summary: Custom ClickHouse JDBC jars can omit or rewrite manifest version metadata. Treat absent or unparseable versions as non-legacy so namespace selection falls back to JDBC metadata capabilities, while preserving the known legacy version override. ### Release note None ### Check List (For Author) - Test\n - [x] JdbcClickHouseConnectorClientTest
|
run buildall |
|
/review |
There was a problem hiding this comment.
Automated review completed for head d7f722241ce8e9be68737cc2edc52a248822f7d5.
No new non-duplicate inline findings remain. The current head correctly uses the driver's metadata capability for known ClickHouse V1/V2 namespace modes, preserves the 0.3/0.4 schema override, adds the exact V2 REMOTE TABLE and MATERIALIZED VIEW filters in both client stacks, and covers source, Distributed, and materialized-view discovery with coherent unit/regression fixtures.
The existing artifact-store thread remains unresolved across the configured provider-dependent URL path: https://doris-regression.storage.googleapis.com/regression/jdbc_driver/clickhouse-jdbc-0.9.8-all.jar currently returns 404, while the 0.4.2 and 0.7.1-patch1 objects at the same prefix return 200. Please replicate the 0.9.8 all-jar to every provider this dynamic URL can select, or use one canonical artifact host. I did not add a second inline comment because this is the same issue already reported in discussion r3595104335.
Critical checkpoints:
- Goal and proof: the product change fixes V2 schema metadata plus Distributed/materialized-view discovery in both parallel FE client paths; the unit tests cover routing/type helpers and the regression queries force database, table-existence, column, and scan paths.
- Scope and clarity: the product edits are focused on namespace selection and the vendor type allowlist. The version gate documents the known 0.3/0.4 exception and otherwise delegates to driver metadata.
- Concurrency and lifecycle: no locks, threads, static initialization, or resource ownership are added. The connector's lazy two-field publication pattern is pre-existing, and supported driver capability methods are local constant/config reads, so this patch adds no substantiated concurrency or lifecycle regression.
- Configuration, compatibility, and parallel paths: no Doris config or dynamic-setting contract is added. Known-version routing and table-type behavior were checked against ClickHouse JDBC 0.3/0.4, configurable 0.7.1, and schema-only 0.9.8 in both changed implementations; versionless fallback is verified on the production connector path. There is no new symbol, storage-format, rolling-upgrade, or persisted-metadata incompatibility.
- Tests and results: the exact V1/V2 table types and scalar outputs are correct, and the two source inserts occur after materialized-view creation. The remaining provider-specific jar 404 means the new regression is not portable across every URL the test can construct. Per the review-runner contract, validation was static and no build or test command was run; GitHub CheckStyle was green and FE UT/compile were pending when inspected.
- Other checkpoints: product code adds no transaction/persistence write, data mutation, FE-BE variable, lock-order, error-swallowing, observability, or hot-path performance concern; the capability is cached after initial namespace detection and is not evaluated on the steady-state hot path. There was no additional user-provided focus.
FE UT Coverage ReportIncrement line coverage |
TPC-H: Total hot run time: 29648 ms |
TPC-DS: Total hot run time: 177501 ms |
ClickBench: Total hot run time: 25.1 s |
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: ClickHouse JDBC V2 exposes databases as schemas and reports distributed tables as
REMOTE TABLE. Doris inferred the database metadata mode from the legacydatabasetermURL parameter and filtered the vendor table type, so V2 catalogs could not discover databases or distributed tables. This change uses JDBC metadata capabilities to select catalog/schema mode and includes remote tables in discovery in both JDBC client implementations.Release note
Support ClickHouse JDBC V2 schema metadata and distributed table discovery.
Check List (For Author)
JdbcClickHouseClientTestJdbcClickHouseConnectorClientTesttest_clickhouse_jdbc_v2