Conversation
|
This is an automated comment for commit 5668bce with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
|
| /// (Assuming that view has empty list of columns iff it's parametrized.) | ||
| if (storage->isView()) | ||
| throw Exception(ErrorCodes::STORAGE_REQUIRES_PARAMETER, "Parametrized view can't be queried through a Merge table."); | ||
| else | ||
| throw Exception(ErrorCodes::LOGICAL_ERROR, "Table has no columns."); |
There was a problem hiding this comment.
(The real changes are: this, getLogger call above, and try-catch outside this. Everything else is indentation change from wrapping code in try-catch. Idk why git/github's diff algorithm failed so badly on this.)
src/Storages/StorageMerge.cpp
Outdated
| ASTPtr required_columns_expr_list = std::make_shared<ASTExpressionList>(); | ||
| ASTPtr column_expr; | ||
| /// (Assuming that view has empty list of columns iff it's parametrized.) | ||
| if (storage->isView()) |
There was a problem hiding this comment.
may be better like this?
| if (storage->isView()) | |
| if (storage->isView() && storage->as<StorageView>() && storage->as<StorageView>()->isParameterizedView()) |
|
We need this to make CI greener :) |
| query_info_.input_order_info = input_sorting_info; | ||
| } | ||
|
|
||
| auto logger = getLogger("StorageMerge"); |
There was a problem hiding this comment.
BTW maybe it make sense to make it a member already (we can include a Merge table name as well there, as for MergeTree)?
There was a problem hiding this comment.
Let's do this in follow-up PR, this PR is too green, it would be a shame not to merge :)
Changelog category (leave one):
Stress test failed like this: #74703 . There was a Merge table that merges lots of tables (everything named
test.*in the whole stress test). The error implies that one of these tables hadStorageInMemoryMetadatawith no physical columns.I spent a while skimming the
create table test.*queries in server log looking for weird engines or column attributes or data types and trying to reproduce the error using thosecreate tablestatements, but failed.Also skimmed call sites of
setInMemoryMetadatalooking for one that may pass empty list of columns, and found one: patametrized view (when no parameters were passed). And it reproduces the crash!:But the stress test failure was probably caused by something else as there's no
create view (default\.)?test[^\. ]* .*\{in server log.So this PR makes StorageMerge:
Fixes: #74703