diff --git a/cpp/src/io/parquet/experimental/page_index_filter.cu b/cpp/src/io/parquet/experimental/page_index_filter.cu index 7ec4aa859f0..aea3be06f84 100644 --- a/cpp/src/io/parquet/experimental/page_index_filter.cu +++ b/cpp/src/io/parquet/experimental/page_index_filter.cu @@ -277,13 +277,13 @@ struct page_stats_caster : public stats_caster_base { /** * @brief Computes host side data including page row offsets, column chunk page offsets, and host - * columns containing page-level min, max and (optional) is_null statistics for a column + * columns containing page-level min, max and (optional) all-null statistics for a column * * @param schema_idx Column schema index * @param dtype Column data type * @param stream CUDA stream * @return A tuple of page row offsets, column chunk page offsets, and host columns containing - * page-level min, max and (optional) is_null statistics + * page-level min, max and (optional) all-null statistics */ template [[nodiscard]] auto compute_host_data(cudf::size_type schema_idx, @@ -300,11 +300,13 @@ struct page_stats_caster : public stats_caster_base { auto const total_pages = col_chunk_page_offsets.back(); - // Create host columns with page-level min, max and optionally is_null statistics + // Create host columns with page-level min, max and optionally all-null statistics. The + // all-null column is true only when every value in the page is null, false when none are, and + // null when only some are, which is what lets it answer both IS_NULL and IS NOT NULL. host_column min(total_pages, stream); host_column max(total_pages, stream); - std::optional> is_null; - if (has_is_null_operator) { is_null = host_column(total_pages, stream); } + std::optional> all_null; + if (has_is_null_operator) { all_null = host_column(total_pages, stream); } // Compute timestamp scale factor for precision conversion auto const ts_scale = [&] { @@ -353,22 +355,24 @@ struct page_stats_caster : public stats_caster_base { if (has_is_null_operator) { // Check if the page is completely null if (column_index.null_pages[page_idx]) { - is_null->val[column_page_idx] = true; + all_null->val[column_page_idx] = true; return; } // Check if the page doesn't have a null count if (not column_index.null_counts.has_value()) { - is_null->set_index(column_page_idx, std::nullopt, {}); + all_null->set_index(column_page_idx, std::nullopt, {}); return; } // Use the null count to determine if the page is completely null auto const page_row_count = page_row_offsets[column_page_idx + 1] - page_row_offsets[column_page_idx]; auto const& null_count = column_index.null_counts.value()[page_idx]; - if (null_count == page_row_count) { - is_null->val[column_page_idx] = false; - } else if (null_count > 0 and null_count < page_row_count) { - is_null->set_index(column_page_idx, std::nullopt, {}); + if (null_count == 0) { + all_null->val[column_page_idx] = false; + } else if (null_count < page_row_count) { + all_null->set_index(column_page_idx, std::nullopt, {}); + } else if (null_count == page_row_count) { + all_null->val[column_page_idx] = true; } else { CUDF_FAIL("Invalid null count"); } @@ -381,7 +385,7 @@ struct page_stats_caster : public stats_caster_base { std::move(col_chunk_page_offsets), std::move(min), std::move(max), - std::move(is_null)}; + std::move(all_null)}; } /** @@ -564,7 +568,7 @@ struct page_stats_to_row_mask_converter : public page_stats_caster { // Converts AST to StatsAST with reference to min, max columns in above `stats_table`. auto constexpr num_columns = 1; parquet::detail::stats_expression_converter const stats_expr{ - filter.get(), num_columns, has_is_null_operator, stream}; + filter.get(), num_columns, stream}; // Filter the input table using AST expression and return the (BOOL8) predicate column. auto const page_mask = cudf::detail::compute_column(page_stats_table, @@ -865,7 +869,7 @@ std::unique_ptr aggregate_reader_metadata::build_row_mask_with_pag auto const num_columns = output_dtypes.size(); // Get a boolean mask indicating which columns will participate in stats based filtering - auto const [stats_columns_mask, has_is_null_operator] = + auto const stats_columns_mask = parquet::detail::stats_columns_collector{filter.get(), static_cast(output_dtypes.size())} .get_stats_columns_mask(); @@ -900,10 +904,8 @@ std::unique_ptr aggregate_reader_metadata::build_row_mask_with_pag // Optimization for single column filter: Directly build the row mask from page statistics if (num_columns == 1) { - page_stats_to_row_mask_converter const stats_col{static_cast(total_rows), - per_file_metadata, - row_group_indices, - has_is_null_operator}; + page_stats_to_row_mask_converter const stats_col{ + static_cast(total_rows), per_file_metadata, row_group_indices, true}; return cudf::type_dispatcher(output_dtypes.front(), stats_col, output_column_schemas.front(), @@ -919,7 +921,7 @@ std::unique_ptr aggregate_reader_metadata::build_row_mask_with_pag page_stats_caster const stats_col{.total_rows = static_cast(total_rows), .per_file_metadata = per_file_metadata, .row_group_indices = row_group_indices, - .has_is_null_operator = has_is_null_operator}; + .has_is_null_operator = true}; std::vector> page_stats_columns; std::for_each( @@ -946,32 +948,28 @@ std::unique_ptr aggregate_reader_metadata::build_row_mask_with_pag 0, stream, cudf::get_current_device_resource_ref())); - if (has_is_null_operator) { - page_stats_columns.push_back(cudf::make_numeric_column( - data_type{cudf::type_id::BOOL8}, - total_rows, - rmm::device_buffer{0, stream, cudf::get_current_device_resource_ref()}, - 0, - stream, - cudf::get_current_device_resource_ref())); - } + page_stats_columns.push_back(cudf::make_numeric_column( + data_type{cudf::type_id::BOOL8}, + total_rows, + rmm::device_buffer{0, stream, cudf::get_current_device_resource_ref()}, + 0, + stream, + cudf::get_current_device_resource_ref())); return; } auto [min_col, max_col, is_null_col] = cudf::type_dispatcher( dtype, stats_col, schema_idx, dtype, stream, cudf::get_current_device_resource_ref()); page_stats_columns.push_back(std::move(min_col)); page_stats_columns.push_back(std::move(max_col)); - if (has_is_null_operator) { - CUDF_EXPECTS(is_null_col.has_value(), "is_null host column must be present"); - page_stats_columns.push_back(std::move(is_null_col.value())); - } + CUDF_EXPECTS(is_null_col.has_value(), "is_null host column must be present"); + page_stats_columns.push_back(std::move(is_null_col.value())); }); auto page_stats_table = cudf::table(std::move(page_stats_columns)); // Converts AST to StatsAST with reference to min, max columns in above `stats_table`. parquet::detail::stats_expression_converter const stats_expr{ - filter.get(), static_cast(output_dtypes.size()), has_is_null_operator, stream}; + filter.get(), static_cast(output_dtypes.size()), stream}; // Filter the input table using AST expression and return the (BOOL8) predicate column. return cudf::detail::compute_column( diff --git a/cpp/src/io/parquet/predicate_pushdown.cpp b/cpp/src/io/parquet/predicate_pushdown.cpp index 44fa83badc7..55dbc5f856a 100644 --- a/cpp/src/io/parquet/predicate_pushdown.cpp +++ b/cpp/src/io/parquet/predicate_pushdown.cpp @@ -68,7 +68,7 @@ std::optional>> aggregate_reader_metadata::ap auto mr = cudf::get_current_device_resource_ref(); // Get a boolean mask indicating which columns can participate in stats based filtering - auto const [stats_columns_mask, has_is_null_operator] = + auto const stats_columns_mask = stats_columns_collector{filter.get(), static_cast(output_dtypes.size())} .get_stats_columns_mask(); @@ -91,14 +91,14 @@ std::optional>> aggregate_reader_metadata::ap } // Converts Column chunk statistics to a table - // where min(col[i]) = columns[i*2], max(col[i])=columns[i*2+1] - // For each column, it contains #sources * #column_chunks_per_src rows + // where min(col[i]) = columns[i*3], max(col[i]) = columns[i*3+1], is_null(col[i]) = + // columns[i*3+2] For each column, it contains #sources * #column_chunks_per_src rows std::vector> columns; row_group_stats_caster const stats_col{ .total_row_groups = static_cast(total_row_groups), .per_file_metadata = per_file_metadata, .row_group_indices = input_row_group_indices, - .has_is_null_operator = has_is_null_operator}; + .has_is_null_operator = true}; for (size_t col_idx = 0; col_idx < output_dtypes.size(); col_idx++) { auto const schema_idx = output_column_schemas[col_idx]; @@ -119,14 +119,12 @@ std::optional>> aggregate_reader_metadata::ap 0, stream, mr)); - if (has_is_null_operator) { - columns.push_back(cudf::make_numeric_column(data_type{cudf::type_id::BOOL8}, - total_row_groups, - rmm::device_buffer{0, stream, mr}, - 0, - stream, - mr)); - } + columns.push_back(cudf::make_numeric_column(data_type{cudf::type_id::BOOL8}, + total_row_groups, + rmm::device_buffer{0, stream, mr}, + 0, + stream, + mr)); continue; } // Map each filter column's zeroth-source schema index into every source's schema tree. @@ -141,16 +139,14 @@ std::optional>> aggregate_reader_metadata::ap dtype, stats_col, per_source_schema_indices, dtype, stream, mr); columns.push_back(std::move(min_col)); columns.push_back(std::move(max_col)); - if (has_is_null_operator) { - CUDF_EXPECTS(is_null_col.has_value(), "is_null column must be present"); - columns.push_back(std::move(is_null_col.value())); - } + CUDF_EXPECTS(is_null_col.has_value(), "is_null column must be present"); + columns.push_back(std::move(is_null_col.value())); } auto stats_table = cudf::table(std::move(columns)); // Converts AST to StatsAST with reference to min, max columns in above `stats_table`. stats_expression_converter const stats_expr{ - filter.get(), static_cast(output_dtypes.size()), has_is_null_operator, stream}; + filter.get(), static_cast(output_dtypes.size()), stream}; // Filter stats table with StatsAST expression and collect filtered row group indices return collect_filtered_row_group_indices( diff --git a/cpp/src/io/parquet/row_group_stats_helpers.hpp b/cpp/src/io/parquet/row_group_stats_helpers.hpp index ab7b968e8a2..a2419b9e981 100644 --- a/cpp/src/io/parquet/row_group_stats_helpers.hpp +++ b/cpp/src/io/parquet/row_group_stats_helpers.hpp @@ -138,6 +138,12 @@ struct row_group_stats_caster : public stats_caster_base { } else { CUDF_FAIL("Invalid null count"); } + } else { + // Statistics without a null count say nothing about this chunk's nullability. The + // value array is allocated uninitialized and the null mask starts out all valid, so + // this entry has to be marked null; leaving it alone would let an uninitialized + // byte be read as an answer. + is_null->set_index(stats_idx, std::nullopt, {}); } } } else { diff --git a/cpp/src/io/parquet/stats_filter_helpers.cpp b/cpp/src/io/parquet/stats_filter_helpers.cpp index fb5dbd3e1e1..f41b7d7c7c4 100644 --- a/cpp/src/io/parquet/stats_filter_helpers.cpp +++ b/cpp/src/io/parquet/stats_filter_helpers.cpp @@ -14,6 +14,27 @@ namespace cudf::io::parquet::detail { +namespace { + +/** + * @brief Maps a logical connective to its null-aware equivalent, returning any other operator as is + * + * A null in a statistics column means the writer did not record the statistic, so it reads as + * "unknown, keep this chunk". The null-aware connectives keep a decisive verdict decisive + * (`false AND unknown` is false); the plain ones return null if either side is null, letting one + * absent statistic switch off pruning for the whole expression. + */ +[[nodiscard]] ast::ast_operator null_aware_operator(ast::ast_operator op) +{ + switch (op) { + case ast::ast_operator::LOGICAL_AND: return ast::ast_operator::NULL_LOGICAL_AND; + case ast::ast_operator::LOGICAL_OR: return ast::ast_operator::NULL_LOGICAL_OR; + default: return op; + } +} + +} // namespace + stats_columns_collector::stats_columns_collector(ast::expression const& expr, cudf::size_type num_columns) : _num_columns(num_columns) @@ -57,10 +78,7 @@ std::reference_wrapper stats_columns_collector::visit( if (kind == operand_kind::COLUMN_REF) { col_ref->accept(*this); - if (input_op == ast_operator::IS_NULL) { - _columns_mask[col_ref->get_column_index()] = true; - _has_is_null_operator = true; - } + if (input_op == ast_operator::IS_NULL) { _columns_mask[col_ref->get_column_index()] = true; } } else { std::ignore = visit_operands(expr.get_operands()); } @@ -84,23 +102,42 @@ std::reference_wrapper stats_columns_collector::visit( return expr; } -std::pair, bool> stats_columns_collector::get_stats_columns_mask() && +thrust::host_vector stats_columns_collector::get_stats_columns_mask() && { - return {std::move(_columns_mask), _has_is_null_operator}; + return std::move(_columns_mask); } stats_expression_converter::stats_expression_converter(ast::expression const& expr, size_type num_columns, - bool has_is_null_operator, cuda::stream_ref stream) : _always_true_scalar{std::make_unique>(true, true, stream)}, _always_true{std::make_unique(*_always_true_scalar)} { - _stats_cols_per_column = has_is_null_operator ? 3 : 2; + _stats_cols_per_column = 3; _num_columns = num_columns; expr.accept(*this); } +void stats_expression_converter::push_non_null_guard(size_type col_index, + ast::expression const& stats_expr) +{ + using cudf::ast::ast_operator; + + auto const& all_null = + _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column + 2}); + // Answering "not entirely null" takes all three of the column's states, so a plain NOT will not + // do: its null state says the chunk holds both nulls and values, or that the writer recorded no + // null count, and both of those answer this question true. NOT alone answers it null and hands an + // unknown to a comparison that is in fact decisive. + auto const& not_all_null = _stats_expr.push( + ast::operation{ast_operator::NULL_LOGICAL_OR, + _stats_expr.push(ast::operation{ast_operator::IS_NULL, all_null}), + _stats_expr.push(ast::operation{ast_operator::NOT, all_null})}); + // Null-aware so that the false this side pushes for an all-null chunk prunes it even though the + // min and max it lacks leave `stats_expr` unknown. + _stats_expr.push(ast::operation{ast_operator::NULL_LOGICAL_AND, not_all_null, stats_expr}); +} + std::reference_wrapper stats_expression_converter::visit( ast::operation const& expr) { @@ -203,10 +240,15 @@ std::reference_wrapper stats_expression_converter::visit( _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column}); auto const& vmax = _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column + 1}); - _stats_expr.push(ast::operation{ - ast::ast_operator::LOGICAL_AND, + // The two halves are separately optional in the statistics, so they are combined null-aware + // to keep whichever one is present decisive. + auto const& in_range = _stats_expr.push(ast::operation{ + ast::ast_operator::NULL_LOGICAL_AND, _stats_expr.push(ast::operation{ast_operator::GREATER_EQUAL, vmax, literal}), _stats_expr.push(ast::operation{ast_operator::LESS_EQUAL, vmin, literal})}); + // An all-null chunk has no min or max, so this range test is unknown there and would keep + // the chunk. The guard makes it prune instead. + push_non_null_guard(col_index, in_range); break; } case ast_operator::NOT_EQUAL: { @@ -214,24 +256,31 @@ std::reference_wrapper stats_expression_converter::visit( _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column}); auto const& vmax = _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column + 1}); - _stats_expr.push( - ast::operation{ast_operator::LOGICAL_OR, + // Null-aware for the same reason as the range test above: either half can be the one the + // statistics carry. + auto const& outside_range = _stats_expr.push( + ast::operation{ast_operator::NULL_LOGICAL_OR, _stats_expr.push(ast::operation{ast_operator::NOT_EQUAL, vmin, vmax}), _stats_expr.push(ast::operation{ast_operator::NOT_EQUAL, vmax, literal})}); + // A null does not satisfy `!=` either, and an all-null chunk has no min or max to make this + // test decisive, so the guard prunes it. + push_non_null_guard(col_index, outside_range); break; } case ast_operator::LESS: [[fallthrough]]; case ast_operator::LESS_EQUAL: { auto const& vmin = _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column}); - _stats_expr.push(ast::operation{op, vmin, literal}); + // An all-null chunk has no min, leaving this test unknown, so the guard prunes it. + push_non_null_guard(col_index, _stats_expr.push(ast::operation{op, vmin, literal})); break; } case ast_operator::GREATER: [[fallthrough]]; case ast_operator::GREATER_EQUAL: { auto const& vmax = _stats_expr.push(ast::column_reference{col_index * _stats_cols_per_column + 1}); - _stats_expr.push(ast::operation{op, vmax, literal}); + // An all-null chunk has no max, leaving this test unknown, so the guard prunes it. + push_non_null_guard(col_index, _stats_expr.push(ast::operation{op, vmax, literal})); break; } default: { @@ -242,7 +291,8 @@ std::reference_wrapper stats_expression_converter::visit( } // Visit operands and push expression for `expr op expr` form else if (lhs_kind == operand_kind::EXPRESSION and rhs_kind == operand_kind::EXPRESSION) { auto new_operands = visit_operands(expr.get_operands()); - _stats_expr.push(ast::operation{op, new_operands.front(), new_operands.back()}); + _stats_expr.push( + ast::operation{null_aware_operator(op), new_operands.front(), new_operands.back()}); } // Push _always_true for `col op col`, `expr op col`, `expr op lit` forms else { _stats_expr.push(ast::operation{ast_operator::IDENTITY, *_always_true}); diff --git a/cpp/src/io/parquet/stats_filter_helpers.hpp b/cpp/src/io/parquet/stats_filter_helpers.hpp index ed7ac756bd2..1042e2b3b49 100644 --- a/cpp/src/io/parquet/stats_filter_helpers.hpp +++ b/cpp/src/io/parquet/stats_filter_helpers.hpp @@ -332,19 +332,18 @@ class stats_columns_collector : public ast::detail::expression_transformer { std::reference_wrapper visit(ast::operation const& expr) override; /** - * @brief Return a boolean vector indicating input columns that can participate in stats based + * @brief Return a boolean vector indicating which input columns can participate in stats based * filtering * * @return Boolean vector indicating input columns that can participate in stats based filtering */ - std::pair, bool> get_stats_columns_mask() &&; + thrust::host_vector get_stats_columns_mask() &&; protected: size_type _num_columns; private: thrust::host_vector _columns_mask; - bool _has_is_null_operator = false; }; /** @@ -353,13 +352,12 @@ class stats_columns_collector : public ast::detail::expression_transformer { * This is used in row group filtering based on predicate. * statistics min value of a column is referenced by column_index*3 * statistics max value of a column is referenced by column_index*3+1 - * statistics is_null value of a column is referenced by column_index*3+2 + * statistics all_nulls value of a column is referenced by column_index*3+2 */ class stats_expression_converter : public stats_columns_collector { public: stats_expression_converter(ast::expression const& expr, size_type num_columns, - bool has_is_null_operator, cuda::stream_ref stream); // Bring all overrides of `visit` from stats_columns_collector into scope @@ -383,6 +381,16 @@ class stats_expression_converter : public stats_columns_collector { thrust::host_vector get_stats_columns_mask() && = delete; private: + /** + * @brief Push `not_all_null AND stats_expr` for a column, so that a chunk holding nothing but + * nulls is pruned by a predicate needing a non-null value to match, rather than kept because its + * absent min and max leave the comparison null + * + * @param col_index Index of the column in the input table + * @param stats_expr Statistics expression to guard, already pushed onto the tree + */ + void push_non_null_guard(size_type col_index, ast::expression const& stats_expr); + ast::tree _stats_expr; cudf::size_type _stats_cols_per_column; std::unique_ptr> _always_true_scalar; diff --git a/cpp/tests/io/parquet_reader_test.cpp b/cpp/tests/io/parquet_reader_test.cpp index 6e55ccbd5ca..9e75cb80ab6 100644 --- a/cpp/tests/io/parquet_reader_test.cpp +++ b/cpp/tests/io/parquet_reader_test.cpp @@ -2898,6 +2898,128 @@ TEST_F(ParquetReaderTest, FilterNoStats) CUDF_TEST_EXPECT_TABLES_EQUAL(expected->view(), result); } +TEST_F(ParquetReaderTest, FilterNullableStats) +{ + // Filter on a column whose row groups differ in nullability, which is what makes a statistic + // indecisive: a chunk of nothing but nulls has no min or max at all, and a chunk holding both + // nulls and values has a null count that says neither of those things. + auto constexpr num_input_row_groups = 3; + + auto const filepath = temp_env->get_temp_filepath("FilterNullableStats.parquet"); + + // Three row groups of three rows. Column `a` holds some nulls in the first, nothing but nulls in + // the second and none in the third, so its nullability statistic takes each of its three states. + // Column `b` is never null and holds values far below the literal compared against it below. + { + auto const a0 = + cudf::test::fixed_width_column_wrapper({10, 0, 20}, {true, false, true}); + auto const a1 = + cudf::test::fixed_width_column_wrapper({0, 0, 0}, {false, false, false}); + auto const a2 = cudf::test::fixed_width_column_wrapper({100, 200, 300}); + auto const b0 = cudf::test::fixed_width_column_wrapper({1, 1, 1}); + auto const b1 = cudf::test::fixed_width_column_wrapper({2, 2, 2}); + auto const b2 = cudf::test::fixed_width_column_wrapper({3, 3, 3}); + auto const t0 = cudf::table_view{{a0, b0}}; + auto const t1 = cudf::table_view{{a1, b1}}; + auto const t2 = cudf::table_view{{a2, b2}}; + + auto const options = + cudf::io::chunked_parquet_writer_options::builder(cudf::io::sink_info{filepath}) + .metadata(cudf::io::table_input_metadata(t0)) + .build(); + + cudf::io::chunked_parquet_writer writer(options); + writer.write(t0); + writer.write(t1); + writer.write(t2); + writer.close(); + } + + auto const test_predicate_pushdown = [&](cudf::ast::operation const& filter, + cudf::size_type expected_filtered_row_groups, + cudf::size_type expected_num_rows) { + auto const options = cudf::io::parquet_reader_options::builder(cudf::io::source_info{filepath}) + .filter(filter) + .build(); + + auto const result = cudf::io::read_parquet(options); + + EXPECT_EQ(result.metadata.num_input_row_groups, num_input_row_groups); + EXPECT_TRUE(result.metadata.num_row_groups_after_stats_filter.has_value()); + EXPECT_EQ(result.metadata.num_row_groups_after_stats_filter.value(), + expected_filtered_row_groups); + EXPECT_EQ(result.tbl->num_rows(), expected_num_rows); + }; + + auto const a_ref = cudf::ast::column_reference(0); + auto const b_ref = cudf::ast::column_reference(1); + + auto scalar_10 = cudf::numeric_scalar(10, true); + auto scalar_20 = cudf::numeric_scalar(20, true); + auto scalar_50 = cudf::numeric_scalar(50, true); + auto scalar_60 = cudf::numeric_scalar(60, true); + auto scalar_5 = cudf::numeric_scalar(5, true); + auto const literal_10 = cudf::ast::literal(scalar_10); + auto const literal_20 = cudf::ast::literal(scalar_20); + auto const literal_50 = cudf::ast::literal(scalar_50); + auto const literal_60 = cudf::ast::literal(scalar_60); + auto const literal_5 = cudf::ast::literal(scalar_5); + + auto const a_is_null = cudf::ast::operation(cudf::ast::ast_operator::IS_NULL, a_ref); + auto const a_ge_10 = + cudf::ast::operation(cudf::ast::ast_operator::GREATER_EQUAL, a_ref, literal_10); + auto const a_le_20 = cudf::ast::operation(cudf::ast::ast_operator::LESS_EQUAL, a_ref, literal_20); + auto const a_ge_50 = + cudf::ast::operation(cudf::ast::ast_operator::GREATER_EQUAL, a_ref, literal_50); + auto const a_le_60 = cudf::ast::operation(cudf::ast::ast_operator::LESS_EQUAL, a_ref, literal_60); + auto const b_gt_5 = cudf::ast::operation(cudf::ast::ast_operator::GREATER, b_ref, literal_5); + + { + // Filter: IS_NULL(a). The all-null row group answers this yes and the partly null one cannot + // answer it at all, so both are kept and only the row group with no nulls is ruled out. + test_predicate_pushdown(a_is_null, 2, 4); + } + + { + // Filter: a >= 10 AND a <= 20. RG 0 passes on its values, which the nulls it also holds must + // not count against; RG 1 holds nothing a comparison can match; RG 2's min of 100 rules it out. + auto const filter = + cudf::ast::operation(cudf::ast::ast_operator::LOGICAL_AND, a_ge_10, a_le_20); + test_predicate_pushdown(filter, 1, 2); + } + + { + // Filter: a == 50 — matches no row group. RG 0's max of 20 and RG 2's min of 100 rule those + // out, the guard rules out the all-null RG 1. + auto const filter = cudf::ast::operation(cudf::ast::ast_operator::EQUAL, a_ref, literal_50); + test_predicate_pushdown(filter, 0, 0); + } + + { + // Filter: a != 50 — only the all-null RG 1 is ruled out, by the guard. + auto const filter = cudf::ast::operation(cudf::ast::ast_operator::NOT_EQUAL, a_ref, literal_50); + test_predicate_pushdown(filter, 2, 5); + } + + { + // Filter: a >= 50 AND a <= 60 — matches no row group. RG 0's max of 20 rules it out even though + // its other conjunct is indecisive there, which is the case a conjunction that is not + // null-aware gets wrong: it would carry the indecisive side up and keep the row group. + auto const filter = + cudf::ast::operation(cudf::ast::ast_operator::LOGICAL_AND, a_ge_50, a_le_60); + test_predicate_pushdown(filter, 0, 0); + } + + { + // Filter: IS_NULL(a) AND b > 5 — matches no row group, since `b` reaches only 3. The `IS_NULL` + // side is indecisive on RG 0 and decides nothing on its own anywhere, so this pins that one + // decisive conjunct is enough to prune whatever the other side says. + auto const filter = + cudf::ast::operation(cudf::ast::ast_operator::LOGICAL_AND, a_is_null, b_gt_5); + test_predicate_pushdown(filter, 0, 0); + } +} + // Filter for float column with NaN values TEST_F(ParquetReaderTest, FilterFloatNAN) { @@ -4399,12 +4521,14 @@ void filter_unary_operation_typed_test() auto const ref_not_expr1 = cudf::ast::operation(cudf::ast::ast_operator::NOT, ref_expr1); auto const ref_expr2 = cudf::ast::operation(cudf::ast::ast_operator::IS_NULL, col_ref_0); - // col0 < 100 AND IS_NULL(col0) + // col0 < 100 AND IS_NULL(col0). No row satisfies this, since a null is not less than anything, + // so every row group is ruled out: the all-null one by the comparison, which needs a non-null + // value to match, and the rest by `IS_NULL` against statistics that count no nulls. auto filter_expression = cudf::ast::operation(cudf::ast::ast_operator::LOGICAL_AND, expr1, expr2); auto ref_filter = cudf::ast::operation(cudf::ast::ast_operator::LOGICAL_AND, ref_expr1, ref_expr2); - auto constexpr expected_filtered_row_groups_with_unary_and = 1; + auto constexpr expected_filtered_row_groups_with_unary_and = 0; test_predicate_pushdown(filter_expression, ref_filter, expected_total_row_groups,