wangyong9999 commented on code in PR #284:
URL: https://github.com/apache/paimon-cpp/pull/284#discussion_r3939644631
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -86,6 +104,12 @@ Result<bool> AppendOnlyFileStoreScan::FilterByStats(const
ManifestEntry& entry)
if (!predicates_) {
return true;
}
+ // A historical file may use a different schema or bucket count after a
rescale.
+ // Keep the inferred bucket separate from the caller's explicit bucket
filter.
+ if (predicate_bucket_ && entry.TotalBuckets() == core_options_.GetBucket()
&&
+ entry.File()->schema_id == table_schema_->Id() && entry.Bucket() !=
*predicate_bucket_) {
+ return false;
Review Comment:
Please skip inferred pruning for NaN bucket-key literals.
`Literal::CompareTo()` treats all NaNs as equal, but the default bucket hash
uses raw bits. With 17 buckets, DOUBLE payloads `0x7ff8000000000000` and
`0x7ff8000000000001` map to buckets 11 and 14. Querying the former now drops a
matching-schema file containing the latter, even though its stats match. I
reproduced this against both versions: the previous scan retains the file.
Canonicalizing only the query cannot fix existing stored keys; retain all
buckets for NaN and add a regression.
##########
src/paimon/core/operation/append_only_file_store_scan.cpp:
##########
@@ -66,6 +69,21 @@ Result<std::unique_ptr<AppendOnlyFileStoreScan>>
AppendOnlyFileStoreScan::Create
table_schema, arrow_schema, core_options,
executor, pool));
PAIMON_RETURN_NOT_OK(
scan->SplitAndSetFilter(table_schema->PartitionKeys(), arrow_schema,
scan_filters));
+ const auto& bucket_keys = table_schema->BucketKeys();
+ int32_t num_buckets = core_options.GetBucket();
+ if (scan->predicates_ && !scan_filters->GetBucketFilter().has_value() &&
num_buckets > 0 &&
+ !bucket_keys.empty()) {
+ std::vector<std::shared_ptr<arrow::DataType>> bucket_key_types;
+ bucket_key_types.reserve(bucket_keys.size());
+ for (const auto& key : bucket_keys) {
+ PAIMON_ASSIGN_OR_RAISE(DataField field,
table_schema->GetField(key));
+ bucket_key_types.push_back(field.Type());
+ }
+ PAIMON_ASSIGN_OR_RAISE(scan->predicate_bucket_,
+ BucketSelectConverter::Convert(
+ scan->predicates_, bucket_keys,
bucket_key_types,
+ core_options.GetBucketFunctionType(),
num_buckets, pool.get()));
Review Comment:
Please rescale DECIMAL literals to the bucket field's scale before inferring
the bucket, or skip pruning if exact conversion is not possible. With
`DECIMAL(10,2)` and 17 buckets, stored `1.20` hashes to bucket 9, but
`Decimal::FromUnscaledLong(12, 10, 1)` (`1.2`) infers bucket 0. This predicate
passes validation and matches the file stats, yet the matching file is
discarded. A cross-scale regression reproduces this on the PR and passes with
the previous scan implementation.
--
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]
