Uh oh!
There was an error while loading. Please reload this page.
[fix](jdbc) Resolve missing remote table names in schema lookup - #65718
Conversation
Jira: CIR-21001 Cause: JdbcExternalTable.initSchema passed the nullable stored remote name directly to JDBC metadata. Fix: Resolve the effective remote name once and reuse it for metadata lookup, identifier mapping, and diagnostics. Verification: JdbcExternalTableTest (2 tests); full FE reactor BUILD SUCCESS.
hello-stephen
commented
Jul 16, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Gabriel39
commented
Jul 16, 2026
run buildall |
Gabriel39
commented
Jul 16, 2026
/review |
There was a problem hiding this comment.
Requesting changes for one correctness issue.
The initSchema() change itself is sound: resolving getRemoteName() once fixes null/empty JDBC table patterns, preserves explicit remote names, and keeps metadata lookup, identifier mapping, conflict diagnostics, schema-cache identity, and scan construction consistent.
One downstream path remains incomplete: fetchRowCount() still uses raw this.remoteName, so the same recovered table returns an unknown cached/manual-analysis row count. See the inline P2.
Critical checkpoints:
- Correctness and compatibility: one P2 incomplete fallback path; no other mapping or persistence regression found.
- Lifecycle, concurrency, and performance: no additional issues found; the change snapshots a trivial effective name and adds no meaningful overhead.
- Tests: the new tests cover schema lookup, identifier mapping, and conflict diagnostics for a null stored name. A row-count regression remains needed with the fix.
- Existing review context: no prior inline comments, so this finding is not a duplicate.
- User focus: no additional focus points were provided.
Validation was static only, as required by the review runner; no builds or source changes were performed. Review converged after 2 rounds with all normal and risk-focused reviewers reporting no new valuable findings against this final comment set.
Uh oh!
There was an error while loading. Please reload this page.
Review: apache#65718 (review) Cause: fetchRowCount used the nullable stored remote name after schema and scan paths recovered the effective name. Fix: use the effective remote table name for row-count query parameters and cover the path with a regression test. Verification: JdbcExternalTableTest (3 tests); full FE reactor BUILD SUCCESS.
Gabriel39
commented
Jul 16, 2026
run buildall |
Gabriel39
commented
Jul 16, 2026
/review |
There was a problem hiding this comment.
Requesting changes for one P2 correctness issue.
The effective-name fallback now consistently covers schema lookup, identifier mapping, diagnostics, scan construction, comments, table existence, and all supported row-count branches, while preserving explicit remote names. However, the repaired metadata call still passes that name as a JDBC search pattern, so identifiers containing _ or % can still collect sibling-table columns on unfiltered client paths. See the inline P2.
Critical checkpoints:
- Correctness and compatibility: one P2 incomplete exact-table lookup; no separate mapping, lower-case-mode, replay, or persistence regression was substantiated.
- Lifecycle, concurrency, and performance: no additional issue found; the effective name is resolved once and adds no meaningful overhead.
- Tests: the three unit tests cover schema/mapping arguments, diagnostics, and the repaired row-count parameter, but the mocked metadata result cannot expose wildcard sibling matches; a metadata-level target-and-sibling regression is needed.
- Existing review context: thread 3595209244's row-count concern is fixed at the current head and was not duplicated.
- User focus: no additional focus points were provided.
Validation was static only, as required by the review runner; no builds or source changes were performed. CheckStyle and the available lightweight quality gates are green, while external FE UT and compile checks were still pending at submission. The repository-local review skill was absent after exhaustive in-repo search, so the supplied review contract was applied directly. Review converged after 2 rounds with all normal and risk-focused reviewers reporting NO_NEW_VALUABLE_FINDINGS against this final comment set.
Uh oh!
There was an error while loading. Please reload this page.
hello-stephen
commented
Jul 16, 2026
FE UT Coverage ReportIncrement line coverage |
Review: apache#65718 Cause: DatabaseMetaData.getColumns interprets table names as patterns, so underscores and percent signs can return columns from sibling tables. Fix: compare each metadata row TABLE_NAME with the requested table across generic, MySQL, GBase, and Oracle clients. Verification: JdbcClientTest; full FE reactor BUILD SUCCESS.
Gabriel39
commented
Jul 16, 2026
run buildall |
Gabriel39
commented
Jul 16, 2026
/review |
There was a problem hiding this comment.
Automated review completed after three rounds.
Two correctness issues remain in the new exact metadata filtering:
- Schema-based clients still accept a same-named table from a wildcard-matched sibling schema because only
TABLE_NAMEis checked. - MySQL
lower_case_table_names=1can return a canonical lowercase metadata name for a valid mixed-case request, and the new case-sensitive Java equality then drops every column for direct callers such as the CDC TVF.
Critical checkpoints:
- Effective remote-name fallback and mapping: schema initialization, identifier mapping, diagnostics, comments/existence, scan construction, and row-count now consistently use the effective table name. Mapped/lowercased catalog construction resolves the canonical remote name through
MetaCache; no separate replay/persistence issue was found. - JDBC compatibility: generic and vendor-specific metadata paths, PostgreSQL's existing schema/table filter, MySQL/GBase catalog routing, Oracle
/and synonym handling, OceanBase delegation, and ClickHouse catalog/schema modes were reviewed. The two inline issues are the remaining compatibility gaps. - Lifecycle, configuration, and performance: changed result-set/connection cleanup remains in
finally; no additional lifecycle, configuration, or performance regression was substantiated. - Tests and validation: the new tests cover null-name fallback, row-count propagation, and sibling-table filtering, but not the sibling-schema or MySQL case-normalization failures. Review was static as required by the runner contract. At the final status check,
check_coverage_fewas failing while FE UT and compile were pending; the linked TeamCity details required authentication, so no unsupported cause is inferred here. - Existing review context: the earlier row-count and sibling-table threads are addressed by the current head and are not duplicated here.
- User focus: no additional user-provided focus was specified.
Completion status: complete. The Round 3 normal full-review and separate risk-focused passes both returned NO_NEW_VALUABLE_FINDINGS against this exact two-comment set.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Filter JDBC metadata rows by both database and table identity, while respecting MySQL lower_case_table_names behavior reported by Connector/J. Add regressions for wildcard sibling schemas and canonical lowercase MySQL metadata names.
Gabriel39
commented
Jul 16, 2026
run buildall |
Gabriel39
commented
Jul 16, 2026
/review |
There was a problem hiding this comment.
Requesting changes for one P2 correctness issue.
The effective-name fallback now consistently covers schema lookup, identifier mapping, diagnostics, scan construction, comments/existence, and supported row-count branches, and the new metadata filter rejects wildcard-matched sibling tables and schemas. However, SQL Server's inherited filter assumes Java case-sensitive identity even when the database collation resolves identifiers case-insensitively; for a lowercased legacy fallback it can discard the only valid stored-case metadata row. See the inline P2.
Critical checkpoints:
- Correctness and compatibility: one P2 SQL Server identifier-collation regression remains; no separate MySQL, GBase, Oracle, PostgreSQL, ClickHouse, DB2, Trino, SAP HANA, mapping, replay, or persistence defect was substantiated.
- Lifecycle, concurrency, security, and performance: result-set/connection cleanup remains in
finally; no additional issue or meaningful overhead was found. - Tests and validation: the added tests cover missing-name schema and row-count propagation, wildcard sibling tables/schemas, and MySQL canonical-lowercase metadata. A SQL Server mixed-case stored-name/lowercase-fallback regression is still needed. Validation was static only, as required by the review runner; no builds or source changes were performed.
- Existing review context: the four earlier inline concerns are addressed at the current head and were not duplicated. This SQL Server case is a distinct vendor path.
- User focus: no additional user-provided focus was specified.
The repository-local review skill was absent after exhaustive in-repo search, so the supplied review contract was applied directly. Review convergence completed after Round 3 with all normal and risk-focused reviewers reporting NO_NEW_VALUABLE_FINDINGS against this exact one-comment set.
Uh oh!
There was an error while loading. Please reload this page.
hello-stephen
commented
Jul 16, 2026
FE Regression Coverage ReportIncrement line coverage |
What problem does this PR solve?
Issue Number: None
On branch-4.1,
ExternalTable#getRemoteName()defines the local table name as the fallback when the stored remote name is null or empty.JdbcExternalTable#initSchema()bypassed that API and passed the nullable field directly to JDBC metadata and identifier mapping.For JDBC, a null table pattern can enumerate columns from every table in the database. This can turn one table schema lookup into a multi-table result and produce misleading case-insensitive column conflicts for a
db.nullremote table.The same code path already uses the effective name for debug logging, comments, and scan table construction; schema initialization was the inconsistent path.
How was it fixed?
Resolve the effective remote table name once at the start of
initSchema()and reuse it for:Explicit remote-name mappings are preserved because
getRemoteName()returns the stored value when present.Release note
Fix JDBC external table schema lookup when a 4.1 table object has a missing remote table name.
Check List (For Author)
JdbcExternalTableTest(2 tests)remote_db.null; after the fix both tests pass./run-fe-ut.sh --run org.apache.doris.datasource.jdbc.JdbcExternalTableTest— full 28-module FE reactorBUILD SUCCESSRelated: #65708 was closed because master has already migrated JDBC catalogs to the plugin-driven path and does not contain this branch-4.1 code path.