lxy-9602 commented on code in PR #392:
URL: https://github.com/apache/paimon-cpp/pull/392#discussion_r4129973993
##########
src/paimon/format/blob/blob_format_writer.cpp:
##########
@@ -262,38 +355,68 @@ Result<std::unique_ptr<InputStream>>
BlobFormatWriter::OpenDescriptorInputStream
if (write_null_on_missing_file_) {
Result<bool> exists = fs_->Exists(blob->Uri());
if (exists.ok() && !exists.value()) {
- return HandleMissingFile(blob->Uri());
+ return HandleMissingFile(blob->Uri(), element_index);
}
}
- return HandleFetchFailure(blob->Uri(), opened.status());
+ return HandleFetchFailure(blob->Uri(), element_index, opened.status());
}
return std::move(opened).value();
}
-std::unique_ptr<InputStream> BlobFormatWriter::HandleMissingFile(const
std::string& blob_uri) {
- PAIMON_LOG_WARN(logger_, "Blob file %s does not exist, writing NULL for
BLOB field %s into %s",
- blob_uri.c_str(), blob_field_name_.c_str(), uri_.c_str());
+std::unique_ptr<InputStream> BlobFormatWriter::HandleMissingFile(
+ const std::string& blob_uri, std::optional<int32_t> element_index) {
+ PAIMON_LOG_WARN(logger_, "Blob file %s does not exist, writing NULL for
%s", blob_uri.c_str(),
+ DescribeValue(element_index).c_str());
++null_on_missing_file_count_;
return std::unique_ptr<InputStream>();
}
Result<std::unique_ptr<InputStream>> BlobFormatWriter::HandleFetchFailure(
- const std::string& blob_uri, const Status& status) {
+ const std::string& blob_uri, std::optional<int32_t> element_index, const
Status& status) {
if (!write_null_on_fetch_failure_) {
- return status;
+ return AddFailureContext(status, fmt::format("failed to fetch blob {}
for", blob_uri),
+ element_index);
}
- PAIMON_LOG_WARN(logger_, "Failed to fetch blob %s, writing NULL for BLOB
field %s into %s: %s",
- blob_uri.c_str(), blob_field_name_.c_str(), uri_.c_str(),
- status.ToString().c_str());
+ PAIMON_LOG_WARN(logger_, "Failed to fetch blob %s, writing NULL for %s:
%s", blob_uri.c_str(),
+ DescribeValue(element_index).c_str(),
status.ToString().c_str());
++null_on_fetch_failure_count_;
return std::unique_ptr<InputStream>();
}
+Status BlobFormatWriter::AddFailureContext(const Status& status, const
std::string& action,
+ std::optional<int32_t>
element_index) const {
+ return status.WithMessage(action, " ", DescribeValue(element_index), ": ",
status.message());
+}
+
+std::string BlobFormatWriter::DescribeValue(std::optional<int32_t>
element_index) const {
+ // The entry of the row being written is appended to bin_lengths_ once the
row is done.
+ const size_t row = bin_lengths_.size();
+ if (!element_index) {
+ return fmt::format("BLOB field {} in row {} of blob file {}",
blob_field_name_, row, uri_);
+ }
+ return fmt::format("element {} of ARRAY<BLOB> field {} in row {} of blob
file {}",
+ *element_index, blob_field_name_, row, uri_);
+}
+
+Result<int32_t> BlobFormatWriter::ToIndexLength(size_t index_size) const {
+ if (index_size > static_cast<size_t>(std::numeric_limits<int32_t>::max()))
{
+ return Status::Invalid(
+ fmt::format("compressed index too large: {} bytes in blob file
{}", index_size, uri_));
+ }
+ return static_cast<int32_t>(index_size);
+}
+
Status BlobFormatWriter::WriteBytes(const char* data, int64_t length) {
- PAIMON_ASSIGN_OR_RAISE(int64_t actual, out_->Write(data, length));
- if (actual != length) {
+ Result<int64_t> actual = out_->Write(data, length);
+ if (!actual.ok()) {
+ const Status& status = actual.status();
+ return status.WithMessage("failed to write blob file ", uri_, ": ",
status.message());
+ }
+ if (actual.value() != length) {
return Status::Invalid(
- fmt::format("unexpected actual length {} not match with expect
{}", actual, length));
+ fmt::format("failed to write blob file {}: unexpected actual
length {} not match with "
+ "expect {}",
+ uri_, actual.value(), length));
}
return Status::OK();
}
Review Comment:
Could we align this case with Java? For `bad_length_descriptor`, Java opens
a bounded stream and fails in `copyExactly` after reaching EOF. Copy-stage
failures are propagated even when `blob-write-null-on-fetch-failure` is
enabled, so this element should fail the write instead of becoming NULL.
--
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]
