github-actions[bot] commented on code in PR #65676:
URL: https://github.com/apache/doris/pull/65676#discussion_r3588971637
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergDeleteFileFilter.java:
##########
@@ -54,16 +57,52 @@ public static PositionDelete
createPositionDelete(DeleteFile deleteFile) {
String deleteFilePath = deleteFile.path().toString();
if (deleteFile.format() == FileFormat.PUFFIN) {
+ long fileSize = deleteFile.fileSizeInBytes();
+ Long contentOffset = deleteFile.contentOffset();
+ Long contentLength = deleteFile.contentSizeInBytes();
+ validateDeletionVectorMetadata(deleteFilePath, fileSize,
contentOffset, contentLength);
// The content_offset and content_size_in_bytes fields are used to
reference
// a specific blob for direct access to a deletion vector.
return new DeletionVector(deleteFilePath,
positionLowerBound.orElse(-1L), positionUpperBound.orElse(-1L),
- deleteFile.fileSizeInBytes(), deleteFile.contentOffset(),
deleteFile.contentSizeInBytes());
+ fileSize, contentOffset, contentLength);
} else {
return new PositionDelete(deleteFilePath,
positionLowerBound.orElse(-1L), positionUpperBound.orElse(-1L),
deleteFile.fileSizeInBytes(), deleteFile.format());
}
}
+ static void validateDeletionVectorMetadata(
+ String deleteFilePath, long fileSize, Long contentOffset, Long
contentLength) {
+ if (contentOffset == null || contentLength == null) {
Review Comment:
This new hard cap is not matched by the Doris writer, so Doris can produce a
valid Puffin DV that it later refuses to read.
`VIcebergDeleteSink::_write_deletion_vector_files` builds
`content_size_in_bytes` from `4 + 4 + bitmap_size + 4` and commits it without
this 1 GiB guard or any split policy, while the Puffin DV format only records
the blob length/CRC and does not make 1 GiB a format limit. A writer-produced
Puffin DV above this cap will therefore fail planning here (and BE has the same
cap) even though the metadata is valid. Please either enforce/split the same
limit on the write path, or avoid making this a reader-side compatibility limit
without a clear unsupported-size contract and writer-to-reader coverage.
##########
be/src/format/table/deletion_vector_reader.cpp:
##########
@@ -17,11 +17,62 @@
#include "format/table/deletion_vector_reader.h"
+#include <limits>
+
#include "rapidjson/document.h"
#include "rapidjson/stringbuffer.h"
#include "util/block_compression.h"
namespace doris {
+namespace {
+
+constexpr int64_t ICEBERG_DELETION_VECTOR_MIN_BYTES = 12;
+constexpr int64_t PAIMON_DELETION_VECTOR_MIN_BYTES = 8;
+constexpr int64_t PAIMON_LENGTH_PREFIX_BYTES = 4;
+
+Status validate_deletion_vector_read_range(const char* description, int64_t
offset, int64_t size,
+ int64_t min_size, size_t&
bytes_read) {
+ if (offset < 0) {
+ return Status::DataQualityError("{} offset must be non-negative: {}",
description, offset);
+ }
+ if (size < min_size) {
+ return Status::DataQualityError("{} size too small: {}, minimum: {}",
description, size,
+ min_size);
+ }
+ if (size > MAX_DELETION_VECTOR_BYTES) {
Review Comment:
This shared cap also rejects valid Paimon deletion-vector files before the
native readers can read them. Doris forwards Paimon's external
`DeletionFile.length()` into this validator, and the check uses `length + 4`,
so any Paimon DV just over 1 GiB now fails here with `DataQualityError`. But
Doris pins Paimon 1.3.1, where `DeletionFile.length` is a `long` and the 64-bit
DV writer only rejects serialized blobs above `Integer.MAX_VALUE` rather than
at 1 GiB. A table with a valid Paimon DV between those limits is still readable
by Paimon's Java/JNI path but is rejected by Doris native/format_v2 before
read. Please align the native limit with Paimon's supported serialized size,
add a fallback/explicit unsupported-native error, or enforce the smaller
contract before native selection.
--
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]
