Skip to content

Commit

Permalink
More helpful optimizations found by clang-tidy
Browse files Browse the repository at this point in the history
  • Loading branch information
Algunenano committed May 15, 2024
1 parent 61fb090 commit 2165cc3
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 18 deletions.
2 changes: 2 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ Checks: [

# This is a good check, but clang-tidy crashes, see https://github.com/llvm/llvm-project/issues/91872
'-modernize-use-constraints',
# https://github.com/abseil/abseil-cpp/issues/1667
'-clang-analyzer-optin.core.EnumCastOutOfRange'
]

WarningsAsErrors: '*'
Expand Down
4 changes: 2 additions & 2 deletions src/Processors/QueryPlan/SourceStepWithFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Block SourceStepWithFilter::applyPrewhereActions(Block block, const PrewhereInfo
{
if (prewhere_info->row_level_filter)
{
block = prewhere_info->row_level_filter->updateHeader(std::move(block));
block = prewhere_info->row_level_filter->updateHeader(block);
auto & row_level_column = block.getByName(prewhere_info->row_level_column_name);
if (!row_level_column.type->canBeUsedInBooleanContext())
{
Expand All @@ -36,7 +36,7 @@ Block SourceStepWithFilter::applyPrewhereActions(Block block, const PrewhereInfo

if (prewhere_info->prewhere_actions)
{
block = prewhere_info->prewhere_actions->updateHeader(std::move(block));
block = prewhere_info->prewhere_actions->updateHeader(block);

auto & prewhere_column = block.getByName(prewhere_info->prewhere_column_name);
if (!prewhere_column.type->canBeUsedInBooleanContext())
Expand Down
16 changes: 6 additions & 10 deletions src/Processors/Transforms/FilterTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,26 +174,22 @@ static std::unique_ptr<IFilterDescription> combineFilterAndIndices(
}

Block FilterTransform::transformHeader(
Block header,
const ActionsDAG * expression,
const String & filter_column_name,
bool remove_filter_column)
const Block & header, const ActionsDAG * expression, const String & filter_column_name, bool remove_filter_column)
{
if (expression)
header = expression->updateHeader(std::move(header));
Block result = expression ? expression->updateHeader(header) : header;

auto filter_type = header.getByName(filter_column_name).type;
auto filter_type = result.getByName(filter_column_name).type;
if (!filter_type->onlyNull() && !isUInt8(removeNullable(removeLowCardinality(filter_type))))
throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_COLUMN_FOR_FILTER,
"Illegal type {} of column {} for filter. Must be UInt8 or Nullable(UInt8).",
filter_type->getName(), filter_column_name);

if (remove_filter_column)
header.erase(filter_column_name);
result.erase(filter_column_name);
else
replaceFilterToConstant(header, filter_column_name);
replaceFilterToConstant(result, filter_column_name);

return header;
return result;
}

FilterTransform::FilterTransform(
Expand Down
7 changes: 2 additions & 5 deletions src/Processors/Transforms/FilterTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ class FilterTransform : public ISimpleTransform
const Block & header_, ExpressionActionsPtr expression_, String filter_column_name_,
bool remove_filter_column_, bool on_totals_ = false, std::shared_ptr<std::atomic<size_t>> rows_filtered_ = nullptr);

static Block transformHeader(
Block header,
const ActionsDAG * expression,
const String & filter_column_name,
bool remove_filter_column);
static Block
transformHeader(const Block & header, const ActionsDAG * expression, const String & filter_column_name, bool remove_filter_column);

String getName() const override { return "FilterTransform"; }

Expand Down
2 changes: 1 addition & 1 deletion src/Processors/Transforms/TotalsHavingTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Block TotalsHavingTransform::transformHeader(

if (expression)
{
block = expression->updateHeader(std::move(block));
block = expression->updateHeader(block);
if (remove_filter)
block.erase(filter_column_name);
}
Expand Down

0 comments on commit 2165cc3

Please sign in to comment.