wangyong9999 commented on code in PR #245:
URL: https://github.com/apache/paimon-cpp/pull/245#discussion_r3853285256
##########
src/paimon/common/utils/fields_comparator.cpp:
##########
@@ -128,21 +144,24 @@ Result<FieldsComparator::FieldComparatorFunc>
FieldsComparator::CompareField(
return lvalue == rvalue ? 0 : (lvalue < rvalue ? -1 : 1);
});
case arrow::Type::type::FLOAT:
- // TODO(xinyu.lxy):
- // currently in java KeyComparatorSupplier: -inf < -0.0 == +0.0 <
+inf = nan
- // paimon-cpp: -inf < -0.0 == +0.0 < +inf and nan cannot be
compared
return FieldsComparator::FieldComparatorFunc(
- [field_idx](const InternalRow& lhs, const InternalRow& rhs) ->
int32_t {
+ [field_idx, use_java_floating_point_order](const InternalRow&
lhs,
+ const InternalRow&
rhs) -> int32_t {
float lvalue = lhs.GetFloat(field_idx);
float rvalue = rhs.GetFloat(field_idx);
- return lvalue == rvalue ? 0 : (lvalue < rvalue ? -1 : 1);
+ return use_java_floating_point_order
+ ? CompareFloatingPoint(lvalue, rvalue)
+ : (lvalue == rvalue ? 0 : (lvalue < rvalue ? -1
: 1));
Review Comment:
Restored the note. PK BTree construction continues to opt into the Java
floating-point order.
##########
src/paimon/core/operation/expire_snapshots.cpp:
##########
@@ -144,7 +156,33 @@ Result<int32_t> ExpireSnapshots::ExpireUntil(int64_t
earliest_snapshot_id,
continue;
}
PAIMON_ASSIGN_OR_RAISE(Snapshot snapshot,
snapshot_manager_->LoadSnapshot(id));
-
PAIMON_RETURN_NOT_OK(CleanUnusedDataFiles(snapshot.DeltaManifestList()));
+ bool tag_changed = false;
+ while (next_tag != tagged_snapshots.end() && next_tag->Id() < id) {
+ previous_tag = &*next_tag;
+ ++next_tag;
+ tag_changed = true;
+ }
+ if (tag_changed) {
+ Result<std::set<std::string>> tagged_data_files_result =
+ GetTaggedDataFiles(*previous_tag);
+ if (!tagged_data_files_result.ok()) {
+ PAIMON_LOG_WARN(logger_,
+ "Skip cleaning data files of snapshot #%ld
because the data files "
+ "referenced by tag snapshot #%ld could not be
loaded. %s",
+ id, previous_tag->Id(),
+
tagged_data_files_result.status().ToString().c_str());
+ tagged_data_files.reset();
+ } else {
+ tagged_data_files =
std::move(tagged_data_files_result).value();
+ }
+ }
+ if (previous_tag != nullptr && !tagged_data_files) {
Review Comment:
Current Java also builds one skipper per tag per expiration, so the failure
is sticky there as well. Updated the warning to state that metadata expiration
continues and skipped files may remain for orphan cleanup.
##########
src/paimon/core/schema/schema_validation.cpp:
##########
@@ -341,6 +402,64 @@ Status SchemaValidation::ValidateForDeletionVectors(const
CoreOptions& options)
"no deletion of old data in this merge engine.");
}
+Status SchemaValidation::ValidatePrimaryKeyBTreeIndexes(const TableSchema&
schema,
+ const CoreOptions&
options) {
+ std::vector<std::string> index_columns =
PrimaryKeyBTreeIndexColumns(schema.Options());
+ if (index_columns.empty()) {
+ return Status::OK();
+ }
+
+ PAIMON_ASSIGN_OR_RAISE(PrimaryKeyIndexDefinitions definitions,
+ PrimaryKeyIndexDefinitions::Create(schema));
+ if (!options.DeletionVectorsEnabled()) {
+ return Status::Invalid(
+ "Primary-key BTree indexes require deletion-vectors.enabled =
true.");
+ }
+ if (schema.PrimaryKeys().empty()) {
+ return Status::Invalid("Primary-key BTree indexes require a
primary-key table.");
+ }
+ if (options.GetBucket() <= 0 && !IsPostponeBucketTable(schema,
options.GetBucket())) {
+ return Status::Invalid(
+ fmt::format("Primary-key BTree indexes require fixed or postpone
bucket mode "
+ "(bucket > 0 or bucket = -2), but bucket is {}.",
+ options.GetBucket()));
+ }
+ PAIMON_ASSIGN_OR_RAISE(
+ bool deletion_vectors_merge_on_read,
+ OptionsUtils::GetValueFromMap<bool>(schema.Options(),
kDeletionVectorsMergeOnRead, false));
+ if (deletion_vectors_merge_on_read) {
+ return Status::Invalid(
+ "Primary-key BTree indexes require deletion-vectors.merge-on-read
= false.");
+ }
+ PAIMON_ASSIGN_OR_RAISE(bool pk_clustering_override,
+ OptionsUtils::GetValueFromMap<bool>(
+ schema.Options(),
Options::PK_CLUSTERING_OVERRIDE, false));
+ if (pk_clustering_override) {
Review Comment:
Updated the message to point to the global C++ commit-path restriction while
retaining early schema validation.
--
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]
