github-actions[bot] commented on code in PR #66422:
URL: https://github.com/apache/doris/pull/66422#discussion_r3710988386
##########
be/test/format_v2/parquet/parquet_statistics_test.cpp:
##########
@@ -386,6 +423,130 @@ TEST(ParquetBloomFilterPruningTest,
NativeUint32BloomUsesPhysicalInt32Hash) {
bloom_filter));
}
+TEST(ParquetBloomFilterPruningTest, NativeFloatingBloomPreservesDorisEquality)
{
+ const auto check_type = []<PrimitiveType Type, typename DataType>(
+ tparquet::Type::type physical_type,
+ typename
PrimitiveTypeTraits<Type>::CppType stored_value,
+ typename
PrimitiveTypeTraits<Type>::CppType predicate_value) {
+ format::parquet::ParquetColumnSchema column_schema;
+ column_schema.type = std::make_shared<DataType>();
+ column_schema.type_descriptor.doris_type = column_schema.type;
+ column_schema.type_descriptor.physical_type = physical_type;
+
+ format::parquet::native::BlockSplitBloomFilter bloom_filter;
+ ASSERT_TRUE(bloom_filter
+ .init(segment_v2::BloomFilter::MINIMUM_BYTES,
+ segment_v2::HashStrategyPB::XX_HASH_64)
+ .ok());
+ bloom_filter.add_bytes(reinterpret_cast<const char*>(&stored_value),
sizeof(stored_value));
+ ASSERT_FALSE(bloom_filter.test_bytes(reinterpret_cast<const
char*>(&predicate_value),
+ sizeof(predicate_value)));
+ const auto field = Field::create_field<Type>(predicate_value);
+
+
EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::NativeBloomFilterExcludes(
+ column_schema, 0, bloom_eq_conjunct(column_schema.type,
field), bloom_filter));
+
EXPECT_FALSE(format::parquet::ParquetStatisticsUtils::NativeBloomFilterExcludes(
+ column_schema, 0, bloom_conjuncts(column_schema.type,
{field}), bloom_filter));
+ };
+
+ check_type.template operator()<TYPE_FLOAT,
DataTypeFloat32>(tparquet::Type::FLOAT, -0.0F, 0.0F);
+ check_type.template operator()<TYPE_FLOAT,
DataTypeFloat32>(tparquet::Type::FLOAT, 0.0F, -0.0F);
+ check_type.template operator()<TYPE_DOUBLE,
DataTypeFloat64>(tparquet::Type::DOUBLE, -0.0, 0.0);
+ check_type.template operator()<TYPE_DOUBLE,
DataTypeFloat64>(tparquet::Type::DOUBLE, 0.0, -0.0);
+ check_type.template operator()<TYPE_FLOAT, DataTypeFloat32>(
+ tparquet::Type::FLOAT, std::bit_cast<float>(uint32_t
{0x7fc00001U}),
+ std::bit_cast<float>(uint32_t {0x7fc00002U}));
+ check_type.template operator()<TYPE_DOUBLE, DataTypeFloat64>(
+ tparquet::Type::DOUBLE, std::bit_cast<double>(uint64_t
{0x7ff8000000000001ULL}),
+ std::bit_cast<double>(uint64_t {0x7ff8000000000002ULL}));
+}
+
+TEST(ParquetBloomFilterPruningTest,
NativeRowGroupKeepsDorisEqualFloatingValues) {
+ const auto check_type = []<PrimitiveType Type, typename DataType>(
+ tparquet::Type::type physical_type,
+ typename
PrimitiveTypeTraits<Type>::CppType stored_value,
+ typename
PrimitiveTypeTraits<Type>::CppType predicate_value) {
+ format::parquet::native::BlockSplitBloomFilter bloom_filter;
+ ASSERT_TRUE(bloom_filter
+ .init(segment_v2::BloomFilter::MINIMUM_BYTES,
+ segment_v2::HashStrategyPB::XX_HASH_64)
+ .ok());
+ bloom_filter.add_bytes(reinterpret_cast<const char*>(&stored_value),
sizeof(stored_value));
+
+ tparquet::BloomFilterAlgorithm algorithm;
+ algorithm.__set_BLOCK(tparquet::SplitBlockAlgorithm());
+ tparquet::BloomFilterHash hash;
+ hash.__set_XXHASH(tparquet::XxHash());
+ tparquet::BloomFilterCompression compression;
+ compression.__set_UNCOMPRESSED(tparquet::Uncompressed());
+ tparquet::BloomFilterHeader bloom_header;
+ bloom_header.__set_numBytes(static_cast<int32_t>(bloom_filter.size()));
+ bloom_header.__set_algorithm(algorithm);
+ bloom_header.__set_hash(hash);
+ bloom_header.__set_compression(compression);
+ std::vector<uint8_t> bloom_bytes;
+ ThriftSerializer serializer(/*compact=*/true, 64);
+ ASSERT_TRUE(serializer.serialize(&bloom_header, &bloom_bytes).ok());
+ bloom_bytes.insert(bloom_bytes.end(), bloom_filter.data(),
+ bloom_filter.data() + bloom_filter.size());
+
+ tparquet::ColumnMetaData column_metadata;
+ column_metadata.__set_type(physical_type);
+ column_metadata.__set_codec(tparquet::CompressionCodec::UNCOMPRESSED);
+ column_metadata.__set_num_values(1);
+ column_metadata.__set_total_compressed_size(0);
+ column_metadata.__set_data_page_offset(0);
+ column_metadata.__set_bloom_filter_offset(0);
+
column_metadata.__set_bloom_filter_length(static_cast<int32_t>(bloom_bytes.size()));
+ tparquet::ColumnChunk chunk;
+ chunk.__set_meta_data(column_metadata);
+ tparquet::RowGroup row_group;
+ row_group.__set_columns({chunk});
+ row_group.__set_total_byte_size(0);
+ row_group.__set_num_rows(1);
+ tparquet::FileMetaData metadata;
+ metadata.__set_version(1);
+ metadata.__set_num_rows(1);
+ metadata.__set_row_groups({row_group});
+
+ const auto field = Field::create_field<Type>(predicate_value);
+ for (const bool use_eq : {true, false}) {
+ auto column_schema =
std::make_unique<format::parquet::ParquetColumnSchema>();
+ column_schema->type = std::make_shared<DataType>();
+ column_schema->type_descriptor.doris_type = column_schema->type;
+ column_schema->type_descriptor.physical_type = physical_type;
+ column_schema->local_id = 0;
+ column_schema->leaf_column_id = 0;
+
+ format::FileScanRequest request;
+ request.local_positions.emplace(format::LocalColumnId(0),
format::LocalIndex(0));
+ request.conjuncts = use_eq ?
bloom_eq_conjunct(column_schema->type, field)
+ : bloom_conjuncts(column_schema->type,
{field});
+ std::vector<std::unique_ptr<format::parquet::ParquetColumnSchema>>
schema;
+ schema.push_back(std::move(column_schema));
+ format::parquet::ParquetFileContext file_context;
+ file_context.native_file =
std::make_shared<StatisticsMemoryFileReader>(bloom_bytes);
+ std::vector<int> selected_row_groups;
+ format::parquet::ParquetPruningStats pruning_stats;
+ ASSERT_TRUE(format::parquet::select_row_groups_by_metadata(
+ metadata, schema, request, nullptr,
&selected_row_groups, true,
+ &pruning_stats, nullptr, nullptr,
&file_context)
+ .ok());
+ EXPECT_EQ(selected_row_groups, std::vector<int>({0}));
+ EXPECT_EQ(pruning_stats.filtered_row_groups_by_bloom_filter, 0);
+ }
+ };
+
+ check_type.template operator()<TYPE_FLOAT,
DataTypeFloat32>(tparquet::Type::FLOAT, -0.0F, 0.0F);
+ check_type.template operator()<TYPE_DOUBLE,
DataTypeFloat64>(tparquet::Type::DOUBLE, 0.0, -0.0);
+ check_type.template operator()<TYPE_FLOAT, DataTypeFloat32>(
+ tparquet::Type::FLOAT, std::bit_cast<float>(uint32_t
{0x7fc00001U}),
Review Comment:
[P1] Preserve NaN equality through the residual IN set
These tests stop at Bloom/row-group retention, but an IN list larger than
eight selects `DynamicContainer<float/double>`. That container uses Doris
`EqualTo` (which equates every NaN payload) with phmap's raw-bit floating hash,
so a file NaN payload A queried by `IN (NaN_payload_B, 1, ..., 8)` can hash to
a different probe and be rejected after this Bloom fix correctly retains the
group; both ordinary residual evaluation and V2's raw fixed-value path use the
same set. This is downstream of and distinct from the Bloom threads. Please
normalize NaNs consistently for hashing/insertion/lookup (or provide a hash
compatible with `EqualTo`) and extend the end-to-end test to execute a >8-value
IN residual and assert the returned row.
##########
be/src/format/parquet/parquet_block_split_bloom_filter.h:
##########
@@ -40,6 +43,23 @@ class ParquetBlockSplitBloomFilter : public
segment_v2::BloomFilter {
Status init(const char* buf, size_t size, segment_v2::HashStrategyPB
strategy) override;
void add_bytes(const char* buf, size_t size) override;
bool test_bytes(const char* buf, size_t size) const override;
+
+ template <typename T>
+ bool test_floating_point(T value) const {
+ static_assert(std::is_floating_point_v<T>);
+ // Doris equality collapses NaN payloads and signed zeros, so raw
Parquet Bloom bytes may
+ // disprove membership only after every representable equivalent has
been covered.
+ if (std::isnan(value)) {
Review Comment:
[P1] Let NaN predicates bypass earlier Parquet range pruning
This conservative Bloom result is never reached when a mixed finite/NaN
chunk publishes finite min/max, which is valid because Parquet requires writers
to exclude NaNs from those bounds and readers to ignore them when searching for
NaN. V1 evaluates the EQ/IN `camp_field` checks before loading Bloom (and
repeats them for page indexes), while V2 runs `check_native_statistics` before
`native_bloom_filter_prune_reason`; with bounds `[0, 0]`, both can classify
`col = NaN` or `col IN (NaN)` as no-match and drop rows that residual
`Compare::equal(NaN, NaN)` accepts. This is upstream of and distinct from the
existing raw-Bloom threads. Please bypass row-group/page range pruning for
FLOAT/DOUBLE EQ/IN containing NaN when no trustworthy NaN count exists, in both
readers, and add mixed finite+NaN tests with statistics present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
