diff --git a/src/Processors/Formats/Impl/Parquet/Reader.cpp b/src/Processors/Formats/Impl/Parquet/Reader.cpp index cbc1bb460c468..55638251f90fc 100644 --- a/src/Processors/Formats/Impl/Parquet/Reader.cpp +++ b/src/Processors/Formats/Impl/Parquet/Reader.cpp @@ -329,23 +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); - adjustRangeFromIndexIfNeeded(range, column_info, can_be_null); + adjustRangeFromIndexIfNeeded(range, column_info, can_be_null && !(column_info.output_nullable && format_filter_info->key_condition)); } catch (Exception & e) { @@ -778,6 +784,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 +2020,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 +2045,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..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; } }, @@ -6812,6 +6814,197 @@ 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_ARGS_IN_HYPERRECTANGLE: + case RPNElement::FUNCTION_POINT_IN_POLYGON: + /// 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: + /// `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()); + 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..6d04ee8580b67 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; @@ -358,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/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp index 6611afc1e6793..f6a00f015d031 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/ManifestFilesPruning.cpp @@ -477,8 +477,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 || !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 new file mode 100644 index 0000000000000..dc5e28de077a6 --- /dev/null +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.reference @@ -0,0 +1,10 @@ +--- a matching row is returned, including null --- +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 new file mode 100755 index 0000000000000..efbd74a5ce2b8 --- /dev/null +++ b/tests/queries/0_stateless/05386_iceberg_nullable_minmax_prunes_null_rejecting_predicate.sh @@ -0,0 +1,68 @@ +#!/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'; + 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' FORMAT Null; + SELECT count() FROM nullable_minmax WHERE partner_id = 'aaa' + 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' FORMAT Null; + SELECT count() FROM nullable_minmax WHERE NOT partner_id = 'sg2o27m' + SETTINGS log_comment = '${CLICKHOUSE_DATABASE}_not_probe' FORMAT 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', + '${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}_not_probe'], + log_comment) + SETTINGS enable_parallel_replicas = 0" + +${CLICKHOUSE_CLIENT} --query "DROP TABLE IF EXISTS nullable_minmax SYNC;" 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..65f73dd630562 --- /dev/null +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.reference @@ -0,0 +1,14 @@ +--- rows, read row groups, pruned row groups --- +99 1 3 +198 2 2 +103 4 0 +202 4 0 +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 new file mode 100755 index 0000000000000..f50c64206bd7b --- /dev/null +++ b/tests/queries/0_stateless/05387_parquet_nullable_minmax_null_rejecting.sh @@ -0,0 +1,97 @@ +#!/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:-}" 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" +# 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" +# `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. +${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" +report_pages "NOT (partner_id <=> 'zzz')" +