From 13fc7c7cf81c806f5993ca43c4114e4073eb2821 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Wed, 15 May 2024 10:35:10 +0200 Subject: [PATCH] Don't allow 0 for max_block_size --- src/Core/SettingsQuirks.cpp | 16 ++++++++++++---- .../QueryPlan/ReadFromSystemNumbersStep.cpp | 16 ++++------------ .../03149_numbers_max_block_size_zero.reference | 3 +-- .../03149_numbers_max_block_size_zero.sh | 7 +++++++ .../03149_numbers_max_block_size_zero.sql | 2 -- 5 files changed, 24 insertions(+), 20 deletions(-) create mode 100755 tests/queries/0_stateless/03149_numbers_max_block_size_zero.sh delete mode 100644 tests/queries/0_stateless/03149_numbers_max_block_size_zero.sql diff --git a/src/Core/SettingsQuirks.cpp b/src/Core/SettingsQuirks.cpp index 5e7d02dc448..73a0e2a9a6c 100644 --- a/src/Core/SettingsQuirks.cpp +++ b/src/Core/SettingsQuirks.cpp @@ -92,7 +92,7 @@ void applySettingsQuirks(Settings & settings, LoggerPtr log) void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log) { - auto getCurrentValue = [¤t_settings](const std::string_view name) -> Field + auto get_current_value = [¤t_settings](const std::string_view name) -> Field { Field current_value; bool has_current_value = current_settings.tryGet(name, current_value); @@ -100,7 +100,7 @@ void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log) return current_value; }; - UInt64 max_threads = getCurrentValue("max_threads").get(); + UInt64 max_threads = get_current_value("max_threads").get(); UInt64 max_threads_max_value = 256 * getNumberOfPhysicalCPUCores(); if (max_threads > max_threads_max_value) { @@ -109,7 +109,7 @@ void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log) current_settings.set("max_threads", max_threads_max_value); } - constexpr UInt64 max_sane_block_rows_size = 4294967296; // 2^32 + static constexpr UInt64 max_sane_block_rows_size = 4294967296; // 2^32 std::unordered_set block_rows_settings{ "max_block_size", "max_insert_block_size", @@ -120,7 +120,7 @@ void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log) "input_format_parquet_max_block_size"}; for (auto const & setting : block_rows_settings) { - auto block_size = getCurrentValue(setting).get(); + auto block_size = get_current_value(setting).get(); if (block_size > max_sane_block_rows_size) { if (log) @@ -128,5 +128,13 @@ void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log) current_settings.set(setting, max_sane_block_rows_size); } } + + if (auto max_block_size = get_current_value("max_block_size").get(); max_block_size == 0) + { + if (log) + LOG_WARNING(log, "Sanity check: 'max_block_size' cannot be 0. Set to default value {}", DEFAULT_BLOCK_SIZE); + current_settings.set("max_block_size", DEFAULT_BLOCK_SIZE); + } } + } diff --git a/src/Processors/QueryPlan/ReadFromSystemNumbersStep.cpp b/src/Processors/QueryPlan/ReadFromSystemNumbersStep.cpp index 759dc7354df..11371578c79 100644 --- a/src/Processors/QueryPlan/ReadFromSystemNumbersStep.cpp +++ b/src/Processors/QueryPlan/ReadFromSystemNumbersStep.cpp @@ -176,9 +176,8 @@ protected: { std::lock_guard lock(ranges_state->mutex); - UInt64 need = base_block_size_; - bool without_block_size_limit = need == 0; + UInt64 need = base_block_size_; UInt64 size = 0; /// how many item found. /// find start @@ -186,21 +185,14 @@ protected: end = start; /// find end - while (without_block_size_limit || need != 0) + while (need != 0) { UInt128 can_provide = end.offset_in_ranges == ranges.size() ? static_cast(0) : ranges[end.offset_in_ranges].size - end.offset_in_range; - if (can_provide == 0) break; - if (without_block_size_limit) - { - end.offset_in_ranges++; - end.offset_in_range = 0; - size += static_cast(can_provide); - } - else if (can_provide > need) + if (can_provide > need) { end.offset_in_range += need; size += need; @@ -535,7 +527,7 @@ Pipe ReadFromSystemNumbersStep::makePipe() checkLimits(size_t(total_size)); - if (max_block_size != 0 && total_size / max_block_size < num_streams) + if (total_size / max_block_size < num_streams) num_streams = static_cast(total_size / max_block_size); if (num_streams == 0) diff --git a/tests/queries/0_stateless/03149_numbers_max_block_size_zero.reference b/tests/queries/0_stateless/03149_numbers_max_block_size_zero.reference index 896f02d1185..d86bac9de59 100644 --- a/tests/queries/0_stateless/03149_numbers_max_block_size_zero.reference +++ b/tests/queries/0_stateless/03149_numbers_max_block_size_zero.reference @@ -1,2 +1 @@ -1320 -1320 +OK diff --git a/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sh b/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sh new file mode 100755 index 00000000000..6f70a0d2536 --- /dev/null +++ b/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +$CLICKHOUSE_CLIENT -q "SELECT count(*) FROM numbers(10) AS a, numbers(11) AS b, numbers(12) AS c SETTINGS max_block_size = 0" 2>&1 | grep -q "Sanity check: 'max_block_size' cannot be 0. Set to default value" && echo "OK" || echo "FAIL" diff --git a/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sql b/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sql deleted file mode 100644 index afc4e4d57a5..00000000000 --- a/tests/queries/0_stateless/03149_numbers_max_block_size_zero.sql +++ /dev/null @@ -1,2 +0,0 @@ -SELECT count(*) FROM numbers(10) AS a, numbers(11) AS b, numbers(12) AS c SETTINGS max_block_size = 0; -SELECT count(*) FROM numbers(10) AS a, numbers(11) AS b, numbers(12) AS c SETTINGS max_block_size = 1;