From 2bdacb75fb73f1f3fbd3315eafc3f7f456f01d4f Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Thu, 22 Oct 2026 17:17:14 +0200 Subject: [PATCH 1/5] Prune Iceberg files on non-null min/max when the predicate cannot match NULL. A clustering column can contain nulls while its manifest bounds still describe the other values. `partner_id = '...'` cannot match those nulls, so the file can be skipped. `IS NULL` still keeps a file that may contain nulls. Co-authored-by: Cursor --- .../Iceberg/ManifestFilesPruning.cpp | 79 ++++++++++++++++++- ..._prunes_null_rejecting_predicate.reference | 8 ++ ..._minmax_prunes_null_rejecting_predicate.sh | 62 +++++++++++++++ 3 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference create mode 100755 tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp index 12ea9d941c91b..6814960e3ea16 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp @@ -368,6 +368,77 @@ std::optional rangeOfPartitionValue(const String & transform_name, const return {}; } +/// `WHERE` keeps a row only when the predicate is true. A comparison with SQL NULL is not true, +/// so the non-null min/max is enough to reject the file. `IS NULL` is true for a null, and an +/// atom the index cannot read may be true for a null, so those stay. +bool mayBeTrueOnNull(const DB::KeyCondition & condition) +{ + enum class Truth : uint8_t + { + IsNotTrue, + IsTrue, + MayBeTrue, + }; + + std::vector stack; + for (const auto & element : condition.getRPN()) + { + switch (element.function) + { + case DB::KeyCondition::RPNElement::FUNCTION_IN_RANGE: + case DB::KeyCondition::RPNElement::FUNCTION_NOT_IN_RANGE: + case DB::KeyCondition::RPNElement::FUNCTION_IN_SET: + case DB::KeyCondition::RPNElement::FUNCTION_NOT_IN_SET: + case DB::KeyCondition::RPNElement::FUNCTION_IS_NOT_NULL: + case DB::KeyCondition::RPNElement::ALWAYS_FALSE: + stack.push_back(Truth::IsNotTrue); + break; + case DB::KeyCondition::RPNElement::FUNCTION_IS_NULL: + case DB::KeyCondition::RPNElement::ALWAYS_TRUE: + stack.push_back(Truth::IsTrue); + break; + case DB::KeyCondition::RPNElement::FUNCTION_UNKNOWN: + case DB::KeyCondition::RPNElement::FUNCTION_ARGS_IN_HYPERRECTANGLE: + case DB::KeyCondition::RPNElement::FUNCTION_POINT_IN_POLYGON: + stack.push_back(Truth::MayBeTrue); + break; + case DB::KeyCondition::RPNElement::FUNCTION_NOT: + { + if (stack.empty()) + return true; + if (stack.back() == Truth::IsNotTrue) + stack.back() = Truth::IsTrue; + else if (stack.back() == Truth::IsTrue) + stack.back() = Truth::IsNotTrue; + break; + } + case DB::KeyCondition::RPNElement::FUNCTION_AND: + case DB::KeyCondition::RPNElement::FUNCTION_OR: + { + if (stack.size() < 2) + return true; + const Truth right = stack.back(); + stack.pop_back(); + const Truth left = stack.back(); + const bool is_and = element.function == DB::KeyCondition::RPNElement::FUNCTION_AND; + if (is_and && (left == Truth::IsNotTrue || right == Truth::IsNotTrue)) + stack.back() = Truth::IsNotTrue; + else if (!is_and && (left == Truth::IsTrue || right == Truth::IsTrue)) + stack.back() = Truth::IsTrue; + else if (left == right) + stack.back() = left; + else + stack.back() = Truth::MayBeTrue; + break; + } + } + } + + if (stack.size() != 1) + return true; + return stack.back() != Truth::IsNotTrue; +} + } PruningReturnStatus ManifestFilesPruner::canBePruned( @@ -433,8 +504,12 @@ PruningReturnStatus ManifestFilesPruner::canBePruned( } auto rect_it = entry_hyperrectangles.find(column_id); - if (has_no_nulls && rect_it != entry_hyperrectangles.end() - && !key_condition.mayBeTrueInRange(1, &rect_it->second.left, &rect_it->second.right, data_types)) + /// A null in the file does not make a null-rejecting predicate true, so the non-null + /// bounds still prune. A predicate that can match null keeps the file unless the + /// manifest says the column has no nulls. + if (rect_it != entry_hyperrectangles.end() + && !key_condition.mayBeTrueInRange(1, &rect_it->second.left, &rect_it->second.right, data_types) + && (has_no_nulls || !mayBeTrueOnNull(key_condition))) { return PruningReturnStatus::MIN_MAX_INDEX_PRUNED; } diff --git a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference new file mode 100644 index 0000000000000..1361664514983 --- /dev/null +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference @@ -0,0 +1,8 @@ +--- a matching row is returned, including null --- +1 +2 +1 +--- min/max prunes a file with nulls when the predicate rejects null --- +2 +2 +1 diff --git a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh new file mode 100755 index 0000000000000..c861cc60492e3 --- /dev/null +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# Tags: no-fasttest +# - no-fasttest: requires `IcebergLocal` (USE_AVRO build option) + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +ROOT="${CLICKHOUSE_USER_FILES}/${CLICKHOUSE_DATABASE}_nullable_minmax" +rm -rf "${ROOT}" +trap 'rm -rf "${ROOT}"' EXIT + +# A Unity liquid-clustering column is an ordinary data column. Its manifest bounds are the +# non-null min/max, and `null_value_counts` counts the nulls separately. `partner_id = 'sg2o27m'` +# is false for null, so a file whose non-null range misses that value cannot hold a match. +# `partner_id IS NULL` is true for those nulls, so that file must stay. +# +# One INSERT per file. `low` is `'aaa'` plus a null (the probe is above its max), `high` is `'zzz'` +# plus a null (the probe is below its min), and `hit` is the probe itself and has no nulls. +# Compaction would merge the files and erase the per-file bounds this checks. +${CLICKHOUSE_CLIENT} --allow_insert_into_iceberg=1 --use_iceberg_metadata_files_cache=0 --query " + CREATE TABLE nullable_minmax (partner_id Nullable(String)) + ENGINE = IcebergLocal('${ROOT}/', 'Parquet') + ORDER BY partner_id + SETTINGS allow_experimental_iceberg_compaction = 0; + + INSERT INTO nullable_minmax VALUES ('aaa'), (NULL); + INSERT INTO nullable_minmax VALUES ('zzz'), (NULL); + INSERT INTO nullable_minmax VALUES ('sg2o27m'); +" < /dev/null + +echo '--- a matching row is returned, including null ---' +${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " + SELECT count() FROM nullable_minmax WHERE partner_id = 'sg2o27m'; + SELECT count() FROM nullable_minmax WHERE partner_id IS NULL; + SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa';" + +echo '--- min/max prunes a file with nulls when the predicate rejects null ---' +${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " + SELECT count() FROM nullable_minmax WHERE partner_id = 'sg2o27m' + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_probe'; + SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa' + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_low'; + SELECT count() FROM nullable_minmax WHERE partner_id IS NULL + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_is_null'; + SYSTEM FLUSH LOGS query_log;" + +${CLICKHOUSE_CLIENT} --query " + SELECT max(ProfileEvents['IcebergMinMaxIndexPrunedFiles']) + FROM system.query_log + WHERE current_database = currentDatabase() AND type = 'QueryFinish' + AND log_comment IN ( + '${CLICKHOUSE_DATABASE}_eq_probe', + '${CLICKHOUSE_DATABASE}_eq_low', + '${CLICKHOUSE_DATABASE}_is_null') + GROUP BY log_comment + ORDER BY indexOf( + ['${CLICKHOUSE_DATABASE}_eq_probe', '${CLICKHOUSE_DATABASE}_eq_low', '${CLICKHOUSE_DATABASE}_is_null'], + log_comment) + SETTINGS enable_parallel_replicas = 0" + +${CLICKHOUSE_CLIENT} --query "DROP TABLE IF EXISTS nullable_minmax SYNC;" From 4efc0847aeaca37b71d42a52eeff387a40656243 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Fri, 23 Oct 2026 14:10:40 +0200 Subject: [PATCH 2/5] Prune Parquet row groups and pages on non-null min/max when the predicate cannot match NULL. `KeyCondition::mayBeTrueOnNull` is the shared check for Iceberg files and Parquet chunks. An equality that rejects NULL still skips a chunk whose other values miss, including an all-null row group. `IS NULL` keeps it. Co-authored-by: Cursor --- .../Formats/Impl/Parquet/Reader.cpp | 24 ++- src/Storages/MergeTree/KeyCondition.cpp | 176 ++++++++++++++++++ src/Storages/MergeTree/KeyCondition.h | 3 + .../Iceberg/ManifestFilesPruning.cpp | 73 +------- ..._prunes_null_rejecting_predicate.reference | 2 + ..._minmax_prunes_null_rejecting_predicate.sh | 11 +- ...t_nullable_minmax_null_rejecting.reference | 8 + ..._parquet_nullable_minmax_null_rejecting.sh | 89 +++++++++ 8 files changed, 309 insertions(+), 77 deletions(-) create mode 100644 tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference create mode 100755 tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh diff --git a/src/Processors/Formats/Impl/Parquet/Reader.cpp b/src/Processors/Formats/Impl/Parquet/Reader.cpp index cbc1bb460c468..fd361188e8b3b 100644 --- a/src/Processors/Formats/Impl/Parquet/Reader.cpp +++ b/src/Processors/Formats/Impl/Parquet/Reader.cpp @@ -345,7 +345,10 @@ void Reader::getHyperrectangleForRowGroup(const parq::RowGroup * meta, Hyperrect if (column_meta.statistics.__isset.max_value) column_info.decoder.decodeField(column_meta.statistics.max_value, /*is_max=*/ true, *column_info.decoded_type, output_block_type, range.right); - adjustRangeFromIndexIfNeeded(range, column_info, can_be_null); + /// Min/max ignores nulls. Widen the range only when SQL NULL can satisfy the filter. + bool include_null = can_be_null && (!column_info.output_nullable || !format_filter_info->key_condition + || format_filter_info->key_condition->mayBeTrueOnNull(column_info.idx_in_output_block)); + adjustRangeFromIndexIfNeeded(range, column_info, include_null); } catch (Exception & e) { @@ -778,6 +781,18 @@ void Reader::prefilterAndInitRowGroups(const std::optionalkey_condition) { + /// An all-null column has no finite min/max. Drop the row group when SQL NULL cannot match. + if (std::any_of(primitive_columns.begin(), primitive_columns.end(), [&](const PrimitiveColumnInfo & column_info) + { + if (!column_info.used_by_key_condition || !column_info.output_nullable) + return false; + const auto & column_meta = meta->columns.at(column_info.column_idx).meta_data; + return column_meta.__isset.statistics && column_meta.statistics.__isset.null_count + && column_meta.statistics.null_count == column_meta.num_values + && !format_filter_info->key_condition->mayBeTrueOnNull(column_info.idx_in_output_block); + })) + continue; + if (!format_filter_info->key_condition->checkInHyperrectangle( hyperrectangle, extended_sample_block_data_types).can_be_true) continue; @@ -2002,7 +2017,9 @@ void Reader::applyColumnIndex(ColumnChunk & column, const PrimitiveColumnInfo & column_info.decoder.decodeField(column_index.min_values[page_idx], /*is_max=*/ false, *column_info.decoded_type, output_block_type, range.left); column_info.decoder.decodeField(column_index.max_values[page_idx], /*is_max=*/ true, *column_info.decoded_type, output_block_type, range.right); - adjustRangeFromIndexIfNeeded(range, column_info, can_be_null); + /// Nullability is checked against each extracted condition below. A nullable + /// ClickHouse output can therefore keep the finite range from the column index. + adjustRangeFromIndexIfNeeded(range, column_info, can_be_null && !column_info.output_nullable); } /// All conjunctive predicates on this column (e.g. two `pointInPolygon` calls sharing @@ -2025,6 +2042,9 @@ void Reader::applyColumnIndex(ColumnChunk & column, const PrimitiveColumnInfo & && (can_be_null || always_null || !spatialBboxStatsHaveNoNulls(*row_group.meta, c.spatial_key_condition_idx))) return true; + if (column_info.output_nullable && can_be_null + && c.condition->mayBeTrueOnNull(column_info.idx_in_output_block)) + return true; return c.condition->checkInHyperrectangle(hyperrectangle, extended_sample_block_data_types).can_be_true; }); diff --git a/src/Storages/MergeTree/KeyCondition.cpp b/src/Storages/MergeTree/KeyCondition.cpp index 1a271a45dd73b..754009a2c0fb4 100644 --- a/src/Storages/MergeTree/KeyCondition.cpp +++ b/src/Storages/MergeTree/KeyCondition.cpp @@ -6812,6 +6812,182 @@ bool KeyCondition::mayBeTrueInRange( return checkInRange(used_key_size, left_keys, right_keys, data_types, BoolMask::consider_only_can_be_true).can_be_true; } +namespace +{ + +/// One SQL result when the column under consideration is NULL. +enum class SqlBool +{ + False, + True, + Unknown, +}; + +/// Every SQL result an expression might still have. Other columns are `any`: false, true, or unknown. +struct TruthOnNull +{ + bool has_false = false; + bool has_true = false; + bool has_unknown = false; + + static TruthOnNull isFalse() + { + TruthOnNull value; + value.has_false = true; + return value; + } + + static TruthOnNull isTrue() + { + TruthOnNull value; + value.has_true = true; + return value; + } + + static TruthOnNull isUnknown() + { + TruthOnNull value; + value.has_unknown = true; + return value; + } + + static TruthOnNull any() + { + TruthOnNull value; + value.has_false = true; + value.has_true = true; + value.has_unknown = true; + return value; + } + + static TruthOnNull from(SqlBool value) + { + if (value == SqlBool::False) + return isFalse(); + if (value == SqlBool::True) + return isTrue(); + return isUnknown(); + } + + bool has(SqlBool value) const + { + if (value == SqlBool::False) + return has_false; + if (value == SqlBool::True) + return has_true; + return has_unknown; + } + + void add(TruthOnNull other) + { + has_false = has_false || other.has_false; + has_true = has_true || other.has_true; + has_unknown = has_unknown || other.has_unknown; + } +}; + +SqlBool sqlAnd(SqlBool left, SqlBool right) +{ + if (left == SqlBool::False || right == SqlBool::False) + return SqlBool::False; + if (left == SqlBool::True && right == SqlBool::True) + return SqlBool::True; + return SqlBool::Unknown; +} + +SqlBool sqlOr(SqlBool left, SqlBool right) +{ + if (left == SqlBool::True || right == SqlBool::True) + return SqlBool::True; + if (left == SqlBool::False && right == SqlBool::False) + return SqlBool::False; + return SqlBool::Unknown; +} + +/// `NOT` swaps false and true. Unknown stays unknown, including when the set is every possible result. +TruthOnNull sqlNot(TruthOnNull value) +{ + std::swap(value.has_false, value.has_true); + return value; +} + +TruthOnNull combineOnNull(TruthOnNull left, TruthOnNull right, bool is_and) +{ + const SqlBool values[] = {SqlBool::False, SqlBool::True, SqlBool::Unknown}; + TruthOnNull result; + for (SqlBool left_value : values) + { + if (!left.has(left_value)) + continue; + for (SqlBool right_value : values) + { + if (!right.has(right_value)) + continue; + result.add(TruthOnNull::from(is_and ? sqlAnd(left_value, right_value) : sqlOr(left_value, right_value))); + } + } + return result; +} + +} + +bool KeyCondition::mayBeTrueOnNull(size_t key_column) const +{ + /// `WHERE` keeps a row only when the predicate is true. A comparison with NULL is unknown. + std::vector stack; + for (const auto & element : rpn) + { + const bool uses_this_column + = std::find(element.key_columns.begin(), element.key_columns.end(), key_column) != element.key_columns.end(); + + switch (element.function) + { + case RPNElement::FUNCTION_IN_RANGE: + case RPNElement::FUNCTION_NOT_IN_RANGE: + case RPNElement::FUNCTION_IN_SET: + case RPNElement::FUNCTION_NOT_IN_SET: + case RPNElement::FUNCTION_ARGS_IN_HYPERRECTANGLE: + case RPNElement::FUNCTION_POINT_IN_POLYGON: + /// `x = 1` and `x != 1` are both unknown when `x` is NULL. + stack.push_back(uses_this_column ? TruthOnNull::isUnknown() : TruthOnNull::any()); + break; + case RPNElement::FUNCTION_IS_NULL: + stack.push_back(uses_this_column ? TruthOnNull::isTrue() : TruthOnNull::any()); + break; + case RPNElement::FUNCTION_IS_NOT_NULL: + stack.push_back(uses_this_column ? TruthOnNull::isFalse() : TruthOnNull::any()); + break; + case RPNElement::FUNCTION_UNKNOWN: + stack.push_back(TruthOnNull::any()); + break; + case RPNElement::ALWAYS_FALSE: + stack.push_back(TruthOnNull::isFalse()); + break; + case RPNElement::ALWAYS_TRUE: + stack.push_back(TruthOnNull::isTrue()); + break; + case RPNElement::FUNCTION_NOT: + if (stack.empty()) + return true; + stack.back() = sqlNot(stack.back()); + break; + case RPNElement::FUNCTION_AND: + case RPNElement::FUNCTION_OR: + { + if (stack.size() < 2) + return true; + const TruthOnNull right = stack.back(); + stack.pop_back(); + stack.back() = combineOnNull(stack.back(), right, element.function == RPNElement::FUNCTION_AND); + break; + } + } + } + + /// A broken expression, or one that can still be true, must not prune. + return stack.size() != 1 || stack.back().has_true; +} + String KeyCondition::RPNElement::toString(const std::vector & key_names) const { WriteBufferFromOwnString buf; diff --git a/src/Storages/MergeTree/KeyCondition.h b/src/Storages/MergeTree/KeyCondition.h index 2de176f3cb068..b406e8bcb7889 100644 --- a/src/Storages/MergeTree/KeyCondition.h +++ b/src/Storages/MergeTree/KeyCondition.h @@ -195,6 +195,9 @@ class KeyCondition const FieldRef * right_keys, const DataTypes & data_types) const; + /// Whether the condition can be true when `key_column` is SQL NULL. Other columns stay arbitrary. + bool mayBeTrueOnNull(size_t key_column) const; + /// Checks that the index can not be used /// FUNCTION_UNKNOWN will be AND'ed (if any). bool alwaysUnknownOrTrue() const; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp index 6814960e3ea16..86ec7297b6961 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp @@ -368,77 +368,6 @@ std::optional rangeOfPartitionValue(const String & transform_name, const return {}; } -/// `WHERE` keeps a row only when the predicate is true. A comparison with SQL NULL is not true, -/// so the non-null min/max is enough to reject the file. `IS NULL` is true for a null, and an -/// atom the index cannot read may be true for a null, so those stay. -bool mayBeTrueOnNull(const DB::KeyCondition & condition) -{ - enum class Truth : uint8_t - { - IsNotTrue, - IsTrue, - MayBeTrue, - }; - - std::vector stack; - for (const auto & element : condition.getRPN()) - { - switch (element.function) - { - case DB::KeyCondition::RPNElement::FUNCTION_IN_RANGE: - case DB::KeyCondition::RPNElement::FUNCTION_NOT_IN_RANGE: - case DB::KeyCondition::RPNElement::FUNCTION_IN_SET: - case DB::KeyCondition::RPNElement::FUNCTION_NOT_IN_SET: - case DB::KeyCondition::RPNElement::FUNCTION_IS_NOT_NULL: - case DB::KeyCondition::RPNElement::ALWAYS_FALSE: - stack.push_back(Truth::IsNotTrue); - break; - case DB::KeyCondition::RPNElement::FUNCTION_IS_NULL: - case DB::KeyCondition::RPNElement::ALWAYS_TRUE: - stack.push_back(Truth::IsTrue); - break; - case DB::KeyCondition::RPNElement::FUNCTION_UNKNOWN: - case DB::KeyCondition::RPNElement::FUNCTION_ARGS_IN_HYPERRECTANGLE: - case DB::KeyCondition::RPNElement::FUNCTION_POINT_IN_POLYGON: - stack.push_back(Truth::MayBeTrue); - break; - case DB::KeyCondition::RPNElement::FUNCTION_NOT: - { - if (stack.empty()) - return true; - if (stack.back() == Truth::IsNotTrue) - stack.back() = Truth::IsTrue; - else if (stack.back() == Truth::IsTrue) - stack.back() = Truth::IsNotTrue; - break; - } - case DB::KeyCondition::RPNElement::FUNCTION_AND: - case DB::KeyCondition::RPNElement::FUNCTION_OR: - { - if (stack.size() < 2) - return true; - const Truth right = stack.back(); - stack.pop_back(); - const Truth left = stack.back(); - const bool is_and = element.function == DB::KeyCondition::RPNElement::FUNCTION_AND; - if (is_and && (left == Truth::IsNotTrue || right == Truth::IsNotTrue)) - stack.back() = Truth::IsNotTrue; - else if (!is_and && (left == Truth::IsTrue || right == Truth::IsTrue)) - stack.back() = Truth::IsTrue; - else if (left == right) - stack.back() = left; - else - stack.back() = Truth::MayBeTrue; - break; - } - } - } - - if (stack.size() != 1) - return true; - return stack.back() != Truth::IsNotTrue; -} - } PruningReturnStatus ManifestFilesPruner::canBePruned( @@ -509,7 +438,7 @@ PruningReturnStatus ManifestFilesPruner::canBePruned( /// manifest says the column has no nulls. if (rect_it != entry_hyperrectangles.end() && !key_condition.mayBeTrueInRange(1, &rect_it->second.left, &rect_it->second.right, data_types) - && (has_no_nulls || !mayBeTrueOnNull(key_condition))) + && (has_no_nulls || !key_condition.mayBeTrueOnNull(0))) { return PruningReturnStatus::MIN_MAX_INDEX_PRUNED; } diff --git a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference index 1361664514983..dc5e28de077a6 100644 --- a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference @@ -2,7 +2,9 @@ 1 2 1 +2 --- min/max prunes a file with nulls when the predicate rejects null --- 2 2 1 +1 diff --git a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh index c861cc60492e3..0926de8bfe06c 100755 --- a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh @@ -33,7 +33,8 @@ echo '--- a matching row is returned, including null ---' ${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " SELECT count() FROM nullable_minmax WHERE partner_id = 'sg2o27m'; SELECT count() FROM nullable_minmax WHERE partner_id IS NULL; - SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa';" + SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa'; + SELECT count() FROM nullable_minmax WHERE NOT partner_id = 'sg2o27m';" echo '--- min/max prunes a file with nulls when the predicate rejects null ---' ${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " @@ -43,6 +44,8 @@ ${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_low'; SELECT count() FROM nullable_minmax WHERE partner_id IS NULL SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_is_null'; + SELECT count() FROM nullable_minmax WHERE NOT partner_id = 'sg2o27m' + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_not_probe'; SYSTEM FLUSH LOGS query_log;" ${CLICKHOUSE_CLIENT} --query " @@ -52,10 +55,12 @@ ${CLICKHOUSE_CLIENT} --query " AND log_comment IN ( '${CLICKHOUSE_DATABASE}_eq_probe', '${CLICKHOUSE_DATABASE}_eq_low', - '${CLICKHOUSE_DATABASE}_is_null') + '${CLICKHOUSE_DATABASE}_is_null', + '${CLICKHOUSE_DATABASE}_not_probe') GROUP BY log_comment ORDER BY indexOf( - ['${CLICKHOUSE_DATABASE}_eq_probe', '${CLICKHOUSE_DATABASE}_eq_low', '${CLICKHOUSE_DATABASE}_is_null'], + ['${CLICKHOUSE_DATABASE}_eq_probe', '${CLICKHOUSE_DATABASE}_eq_low', + '${CLICKHOUSE_DATABASE}_is_null', '${CLICKHOUSE_DATABASE}_not_probe'], log_comment) SETTINGS enable_parallel_replicas = 0" diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference new file mode 100644 index 0000000000000..2bc09d062ce12 --- /dev/null +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference @@ -0,0 +1,8 @@ +--- rows, read row groups, pruned row groups --- +99 1 3 +198 2 2 +103 4 0 +202 4 0 +--- rows, read pages, pruned pages --- +900 2 1 +1200 3 0 diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh new file mode 100755 index 0000000000000..edefd6798b96e --- /dev/null +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Tags: no-fasttest + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +FILE="${CLICKHOUSE_TEST_UNIQUE_NAME}_nullable_minmax.parquet" +PAGE_FILE="${CLICKHOUSE_TEST_UNIQUE_NAME}_nullable_page_minmax.parquet" +rm -f "${FILE}" "${PAGE_FILE}" +trap 'rm -f "${FILE}" "${PAGE_FILE}"' EXIT + +# Model one liquid-clustered Parquet file: three sorted row groups with nulls, followed by an +# all-null group. The file-level range covers `sg2o27m`, while two finite ranges do not. +${CLICKHOUSE_LOCAL} --query " + INSERT INTO FUNCTION file('${FILE}', Parquet, 'partner_id Nullable(String)') + SELECT CAST( + multiIf( + number >= 300, NULL, + number % 100 = 0, NULL, + ['aaa', 'sg2o27m', 'zzz'][intDiv(number, 100) + 1]), + 'Nullable(String)') AS partner_id + FROM numbers(400) + SETTINGS + output_format_parquet_row_group_size = 100, + output_format_parquet_write_page_index = 1, + max_block_size = 1000" + +report() +{ + local predicate=$1 + local output + output=$(${CLICKHOUSE_LOCAL} --print-profile-events --profile-events-delay-ms=-1 --query " + SELECT count() + FROM file('${FILE}', Parquet) + WHERE ${predicate} + SETTINGS + input_format_parquet_bloom_filter_push_down = 0, + input_format_parquet_dictionary_filter_push_down = 0" 2>&1) + echo "${output}" | awk ' + /^[0-9]+$/ { rows = $1 } + /ParquetReadRowGroups:/ { read += $(NF-1) } + /ParquetPrunedRowGroups:/ { pruned += $(NF-1) } + END { print rows, read + 0, pruned + 0 }' +} + +echo '--- rows, read row groups, pruned row groups ---' +report "partner_id = 'sg2o27m'" +report "NOT partner_id = 'sg2o27m'" +report "partner_id IS NULL" +report "partner_id = 'sg2o27m' OR partner_id IS NULL" + +# One row group with three small pages. Every page has nulls, so this also checks that page +# min/max stays usable for a null-rejecting predicate. +${CLICKHOUSE_LOCAL} --query " + INSERT INTO FUNCTION file('${PAGE_FILE}', Parquet, 'partner_id Nullable(String)') + SELECT CAST( + if(number % 10 = 0, NULL, ['aaa', 'sg2o27m', 'zzz'][intDiv(number, 1000) + 1]), + 'Nullable(String)') AS partner_id + FROM numbers(3000) + SETTINGS + output_format_parquet_row_group_size = 3000, + output_format_parquet_data_page_size = 128, + output_format_parquet_max_dictionary_size = 0, + output_format_parquet_write_page_index = 1, + max_block_size = 10000" + +report_pages() +{ + local predicate=$1 + local output + output=$(${CLICKHOUSE_LOCAL} --print-profile-events --profile-events-delay-ms=-1 --query " + SELECT count() + FROM file('${PAGE_FILE}', Parquet) + WHERE ${predicate} + SETTINGS + input_format_parquet_bloom_filter_push_down = 0, + input_format_parquet_dictionary_filter_push_down = 0" 2>&1) + echo "${output}" | awk ' + /^[0-9]+$/ { rows = $1 } + /ParquetReadPages:/ { read += $(NF-1) } + /ParquetPrunedPages:/ { pruned += $(NF-1) } + END { print rows, read + 0, pruned + 0 }' +} + +echo '--- rows, read pages, pruned pages ---' +report_pages "partner_id = 'sg2o27m'" +report_pages "partner_id = 'sg2o27m' OR partner_id IS NULL" + From 9732cac9ee05c5b825f09b061a54a4fdb15337d4 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Fri, 23 Oct 2026 14:51:38 +0200 Subject: [PATCH 3/5] Keep nullable min/max chunks when `assumeNotNull` or `transform_null_in` matches NULL. `mayBeTrueOnNull` treated every comparison and `IN` as unknown on NULL, so a chunk whose other values missed the probe was skipped even though the NULL rows matched. Co-authored-by: Cursor --- src/Processors/Formats/Impl/Parquet/Reader.cpp | 13 ++++++++----- src/Storages/MergeTree/KeyCondition.cpp | 13 +++++++++---- ...parquet_nullable_minmax_null_rejecting.reference | 3 +++ .../05387_parquet_nullable_minmax_null_rejecting.sh | 6 +++++- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/Processors/Formats/Impl/Parquet/Reader.cpp b/src/Processors/Formats/Impl/Parquet/Reader.cpp index fd361188e8b3b..55638251f90fc 100644 --- a/src/Processors/Formats/Impl/Parquet/Reader.cpp +++ b/src/Processors/Formats/Impl/Parquet/Reader.cpp @@ -329,26 +329,29 @@ void Reader::getHyperrectangleForRowGroup(const parq::RowGroup * meta, Hyperrect bool can_be_null = !column_meta.statistics.__isset.null_count || column_meta.statistics.null_count != 0; bool null_as_default = options.format.null_as_default && !column_info.output_nullable; + /// NULL itself can satisfy the filter (`assumeNotNull`, `nullIn`), so bounds cannot reject the row group. + bool null_can_match = can_be_null && column_info.output_nullable && format_filter_info->key_condition + && format_filter_info->key_condition->mayBeTrueOnNull(column_info.idx_in_output_block); if (nullable && always_null) { /// Single-point range containing either the default value or one of the infinities. if (null_as_default) range.right = range.left = output_block_type.getDefault(); - else + else if (!null_can_match) range.right = range.left; continue; } + if (null_can_match) + continue; + if (column_meta.statistics.__isset.min_value) column_info.decoder.decodeField(column_meta.statistics.min_value, /*is_max=*/ false, *column_info.decoded_type, output_block_type, range.left); if (column_meta.statistics.__isset.max_value) column_info.decoder.decodeField(column_meta.statistics.max_value, /*is_max=*/ true, *column_info.decoded_type, output_block_type, range.right); - /// Min/max ignores nulls. Widen the range only when SQL NULL can satisfy the filter. - bool include_null = can_be_null && (!column_info.output_nullable || !format_filter_info->key_condition - || format_filter_info->key_condition->mayBeTrueOnNull(column_info.idx_in_output_block)); - adjustRangeFromIndexIfNeeded(range, column_info, include_null); + adjustRangeFromIndexIfNeeded(range, column_info, can_be_null && !(column_info.output_nullable && format_filter_info->key_condition)); } catch (Exception & e) { diff --git a/src/Storages/MergeTree/KeyCondition.cpp b/src/Storages/MergeTree/KeyCondition.cpp index 754009a2c0fb4..14433a716ed59 100644 --- a/src/Storages/MergeTree/KeyCondition.cpp +++ b/src/Storages/MergeTree/KeyCondition.cpp @@ -6944,12 +6944,17 @@ bool KeyCondition::mayBeTrueOnNull(size_t key_column) const { case RPNElement::FUNCTION_IN_RANGE: case RPNElement::FUNCTION_NOT_IN_RANGE: - case RPNElement::FUNCTION_IN_SET: - case RPNElement::FUNCTION_NOT_IN_SET: case RPNElement::FUNCTION_ARGS_IN_HYPERRECTANGLE: case RPNElement::FUNCTION_POINT_IN_POLYGON: - /// `x = 1` and `x != 1` are both unknown when `x` is NULL. - stack.push_back(uses_this_column ? TruthOnNull::isUnknown() : TruthOnNull::any()); + /// A bare comparison is unknown on NULL. `assumeNotNull` and other wrappers can make it true. + stack.push_back(uses_this_column && element.monotonic_functions_chain.empty() + ? TruthOnNull::isUnknown() : TruthOnNull::any()); + break; + case RPNElement::FUNCTION_IN_SET: + case RPNElement::FUNCTION_NOT_IN_SET: + /// `nullIn` / `notNullIn` (`transform_null_in`) can match NULL. A normal `IN` does not, but the + /// RPN does not record which one this is, so keep the chunk. + stack.push_back(TruthOnNull::any()); break; case RPNElement::FUNCTION_IS_NULL: stack.push_back(uses_this_column ? TruthOnNull::isTrue() : TruthOnNull::any()); diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference index 2bc09d062ce12..98284d3598e37 100644 --- a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference @@ -3,6 +3,9 @@ 198 2 2 103 4 0 202 4 0 +103 4 0 +202 4 0 +301 4 0 --- rows, read pages, pruned pages --- 900 2 1 1200 3 0 diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh index edefd6798b96e..feb0717b45a35 100755 --- a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh @@ -36,7 +36,7 @@ report() WHERE ${predicate} SETTINGS input_format_parquet_bloom_filter_push_down = 0, - input_format_parquet_dictionary_filter_push_down = 0" 2>&1) + input_format_parquet_dictionary_filter_push_down = 0${2:-}" 2>&1) echo "${output}" | awk ' /^[0-9]+$/ { rows = $1 } /ParquetReadRowGroups:/ { read += $(NF-1) } @@ -49,6 +49,10 @@ report "partner_id = 'sg2o27m'" report "NOT partner_id = 'sg2o27m'" report "partner_id IS NULL" report "partner_id = 'sg2o27m' OR partner_id IS NULL" +# NULL itself matches: a monotonic wrapper, and `IN` / `NOT IN` under `transform_null_in`. +report "assumeNotNull(partner_id) = ''" +report "partner_id IN ('sg2o27m', NULL)" ", transform_null_in = 1" +report "partner_id NOT IN ('zzz')" ", transform_null_in = 1" # One row group with three small pages. Every page has nulls, so this also checks that page # min/max stays usable for a null-rejecting predicate. From 4086c266e470ca822316c97ec60d484df531e839 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Fri, 23 Oct 2026 17:03:51 +0200 Subject: [PATCH 4/5] Keep nullable min/max chunks when `NOT (key <=> c)` is true for NULL Co-authored-by: Cursor --- src/Storages/MergeTree/KeyCondition.cpp | 18 +++++++++++++++--- src/Storages/MergeTree/KeyCondition.h | 4 ++++ ...et_nullable_minmax_null_rejecting.reference | 3 +++ ...7_parquet_nullable_minmax_null_rejecting.sh | 4 ++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/Storages/MergeTree/KeyCondition.cpp b/src/Storages/MergeTree/KeyCondition.cpp index 14433a716ed59..34732af6d2106 100644 --- a/src/Storages/MergeTree/KeyCondition.cpp +++ b/src/Storages/MergeTree/KeyCondition.cpp @@ -101,6 +101,8 @@ const KeyCondition::AtomMap KeyCondition::atom_map { out.function = RPNElement::FUNCTION_IN_RANGE; out.range = Range(value); + /// `NULL <=> c` is false, so `NOT (key <=> c)` is true and must keep a chunk that has NULL. + out.false_on_null = true; return true; } }, @@ -6946,9 +6948,19 @@ bool KeyCondition::mayBeTrueOnNull(size_t key_column) const case RPNElement::FUNCTION_NOT_IN_RANGE: case RPNElement::FUNCTION_ARGS_IN_HYPERRECTANGLE: case RPNElement::FUNCTION_POINT_IN_POLYGON: - /// A bare comparison is unknown on NULL. `assumeNotNull` and other wrappers can make it true. - stack.push_back(uses_this_column && element.monotonic_functions_chain.empty() - ? TruthOnNull::isUnknown() : TruthOnNull::any()); + /// A bare comparison is unknown on NULL. `<=>` is false. `assumeNotNull` can make either one true. + if (!uses_this_column || !element.monotonic_functions_chain.empty()) + { + stack.push_back(TruthOnNull::any()); + } + else if (element.false_on_null) + { + stack.push_back(TruthOnNull::isFalse()); + } + else + { + stack.push_back(TruthOnNull::isUnknown()); + } break; case RPNElement::FUNCTION_IN_SET: case RPNElement::FUNCTION_NOT_IN_SET: diff --git a/src/Storages/MergeTree/KeyCondition.h b/src/Storages/MergeTree/KeyCondition.h index b406e8bcb7889..6d04ee8580b67 100644 --- a/src/Storages/MergeTree/KeyCondition.h +++ b/src/Storages/MergeTree/KeyCondition.h @@ -361,6 +361,10 @@ class KeyCondition /// Whether to relax the key condition (e.g., for LIKE queries without a perfect prefix). bool relaxed = false; + /// `isNotDistinctFrom` (`<=>`) with a non-NULL constant is false when the column is NULL. + /// A normal comparison is unknown. `NOT` of this atom is therefore true for NULL. + bool false_on_null = false; + /// For FUNCTION_IN_RANGE and FUNCTION_NOT_IN_RANGE. Range range = Range::createWholeUniverse(); diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference index 98284d3598e37..65f73dd630562 100644 --- a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference @@ -6,6 +6,9 @@ 103 4 0 202 4 0 301 4 0 +99 1 3 +301 4 0 --- rows, read pages, pruned pages --- 900 2 1 1200 3 0 +2100 3 0 diff --git a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh index feb0717b45a35..f50c64206bd7b 100755 --- a/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh @@ -53,6 +53,9 @@ report "partner_id = 'sg2o27m' OR partner_id IS NULL" report "assumeNotNull(partner_id) = ''" report "partner_id IN ('sg2o27m', NULL)" ", transform_null_in = 1" report "partner_id NOT IN ('zzz')" ", transform_null_in = 1" +# `NULL <=> c` is false, so the positive form still prunes. `NOT (NULL <=> c)` is true, so the `zzz` group stays. +report "partner_id <=> 'sg2o27m'" +report "NOT (partner_id <=> 'zzz')" # One row group with three small pages. Every page has nulls, so this also checks that page # min/max stays usable for a null-rejecting predicate. @@ -90,4 +93,5 @@ report_pages() echo '--- rows, read pages, pruned pages ---' report_pages "partner_id = 'sg2o27m'" report_pages "partner_id = 'sg2o27m' OR partner_id IS NULL" +report_pages "NOT (partner_id <=> 'zzz')" From 0a43e582783f826059898db4fbf415c41c43b897 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Tue, 27 Oct 2026 11:34:58 +0100 Subject: [PATCH 5/5] Do not print `05386` `count` rows that only fill `query_log` Co-authored-by: Cursor --- ...rg_nullable_minmax_prunes_null_rejecting_predicate.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh index 0926de8bfe06c..efbd74a5ce2b8 100755 --- a/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh @@ -37,15 +37,16 @@ ${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " SELECT count() FROM nullable_minmax WHERE NOT partner_id = 'sg2o27m';" echo '--- min/max prunes a file with nulls when the predicate rejects null ---' +# `FORMAT Null` keeps these out of the reference. `query_log` still records the prune counters. ${CLICKHOUSE_CLIENT} --use_iceberg_metadata_files_cache=0 --query " SELECT count() FROM nullable_minmax WHERE partner_id = 'sg2o27m' - SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_probe'; + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_probe' FORMAT Null; SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa' - SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_low'; + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_eq_low' FORMAT Null; SELECT count() FROM nullable_minmax WHERE partner_id IS NULL - SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_is_null'; + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_is_null' FORMAT Null; SELECT count() FROM nullable_minmax WHERE NOT partner_id = 'sg2o27m' - SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_not_probe'; + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_not_probe' FORMAT Null; SYSTEM FLUSH LOGS query_log;" ${CLICKHOUSE_CLIENT} --query "