Fix LOGICAL_ERROR "Table has no columns." when Merge matches a dangling Alias#104790
Conversation
…ngling `Alias`
When the regexp of a `Merge` table matches an `Alias` storage whose target
table has been dropped, the query aborted with the internal
`LOGICAL_ERROR` "Table has no columns." raised in
`ReadFromMerge::createChildrenPlans`.
Root cause:
* `StorageAlias::getInMemoryMetadataPtr` (`src/Storages/StorageAlias.h`)
returns the alias's own empty metadata when its target is missing
(`tryGetTargetTable` is null). This is necessary for metadata-only
operations such as `system.tables`/`system.columns` queries on a
dangling alias, exercised by the existing
`03636_storage_alias_basic` test.
* `ReadFromMerge::createChildrenPlans` (`src/Storages/StorageMerge.cpp`)
iterates the selected children, calls `getInMemoryMetadataPtr` on
each, and bails out with `LOGICAL_ERROR` "Table has no columns." if
the snapshot has no columns — assuming the only legitimate empty
case is a parameterized view.
In stress tests the path is reached via `ignore_drop_queries_probability`
leaking `alias_with_missing_target` from `03636_storage_alias_basic` into
later tests, where a `Merge(currentDatabase(), 'a')` regexp matches the
dangling alias. In regular workloads the same path is reachable via a
TOCTOU race: the target table is dropped between `getSelectedTables`
(which locks the alias storage, not its target) and the metadata
snapshot read in `createChildrenPlans`.
Fix: detect the `Alias` case in the empty-metadata branch and throw a
user-facing `UNKNOWN_TABLE` exception that names both the dangling alias
and the matching `Merge` table. This mirrors the analyzer-resolution
path, where `StorageMerge::isRemote` already surfaces `UNKNOWN_TABLE`
via `StorageAlias::isRemote` → `getTargetTable`.
A new regression test `04236_merge_with_alias_missing_target_104740`
locks down the user-visible contract: a `SELECT` through a `Merge`
whose regexp matches a dangling alias must throw `UNKNOWN_TABLE`, never
`LOGICAL_ERROR`.
Closes ClickHouse#104740 (STID 3593-560a / 3593-5990).
|
cc @alexey-milovidov @azat — could you review this? Fixes the chronic STID 3593-560a / 3593-5990 |
Algunenano
added
the
can be tested
|
Workflow [PR], commit [5b5c895] Summary: ✅ AI ReviewSummaryThis PR changes Missing context / blind spots
Final Verdict
|
|
Standing down — @azat's PR #104919 supersedes this one. Quick summary for future readers:
@azat's approach fixes the contract at the source (alias storage refuses to hand out empty metadata) and isolates the "tolerate dangling target" behavior to the four system-table call sites that actually need it. That's cleaner than guarding at one consumer (our approach). Closing this PR in favor of #104919. Thanks for the alternative @azat! Session: cron:clickhouse-ci-task-worker:20260514-091500 |
|
Superseded by #104919 (see above). |
LLVM Coverage Report
Changed lines: 33.33% (3/9) · Uncovered code |
|
Thanks for the review and approval, @azat — and thanks for applying the comment-wording suggestion directly (commit 5b5c895). The new comment is much cleaner. For context to anyone following this thread: I had earlier stood down in favor of #104919, but it looks like this PR's approach was kept after all. CI is currently rebuilding on the updated commit; will monitor for green and ping again if anything else surfaces. |
robot-ch-test-poll1
added
the
pr-synced-to-cloud
6 participants
A
Mergetable whose regexp matches anAliasstorage whose target tablehas been dropped aborts the query with an internal
LOGICAL_ERROR"Table has no columns.". This shows up in CI as
STID 3593-560a / 3593-5990
and was flagged on #100752
("Enable snappy compression in HTTP interface") and
#102690
("Fix inserting into Time data type during JSON parsing") — both
unrelated to
StorageMerge/StorageAlias.Root cause
StorageAlias::getInMemoryMetadataPtrinsrc/Storages/StorageAlias.hreturns the alias's own (empty) metadata when its target is missing.
This is intentional so that metadata-only operations such as
system.tables/system.columnsqueries on a dangling alias work(exercised by the existing
03636_storage_alias_basictest).ReadFromMerge::createChildrenPlansinsrc/Storages/StorageMerge.cppiterates the selected children, calls
getInMemoryMetadataPtron each,and throws
LOGICAL_ERROR"Table has no columns." when the snapshothas no columns — assuming the only legitimate empty case is a
parameterized view.
In CI the path is reached two ways:
ignore_drop_queries_probabilityleaks thealias_with_missing_targettable from03636_storage_alias_basicinto a later test whose
Merge(currentDatabase(), 'a')regexpmatches it.
StorageMerge::getSelectedTables(which only locks the alias storage,not its target) and the metadata snapshot read in
createChildrenPlans.Fix
Detect the
Aliascase in the empty-metadata branch and throwUNKNOWN_TABLEwith an exception that names both the dangling aliasand the matching
Mergetable. This mirrors whatStorageMerge::isRemotealready surfaces during analyzer resolutionvia
StorageAlias::isRemote→getTargetTable.A new regression test
04236_merge_with_alias_missing_target_104740locks down the user-visible contract: a
SELECTthrough aMergewhose regexp matches a dangling alias must throw
UNKNOWN_TABLE,never
LOGICAL_ERROR.Closes #104740.
CI evidence: 30-day CIDB shows 6 hits across
arm_msan/arm_release/amd_tsanstress runs on master and 4 distinct unrelated PRs.
Changelog category (leave one):
Documentation entry for user-facing changes
Version info
26.5.1.789