feat(blob): support blob-write-null-on-missing-file and blob-write-null-on-fetch-failure options#421
Open
SteNicholas wants to merge 4 commits into
Open
feat(blob): support blob-write-null-on-missing-file and blob-write-null-on-fetch-failure options#421SteNicholas wants to merge 4 commits into
SteNicholas wants to merge 4 commits into
Conversation
SteNicholas
force-pushed
the
blob-write-null-options
branch
2 times, most recently
from
6a95a3b to
13156e5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds two new table options to make descriptor-based BLOB writes tolerant to missing objects and/or fetch/open failures, aligning paimon-cpp writer behavior with Java BlobFormatWriter semantics. The change also tightens jindo error-code mapping so “missing file” is classified as Status::NotExist, enabling correct option gating on OSS.
Changes:
- Introduce
blob-write-null-on-missing-fileandblob-write-null-on-fetch-failureoption keys, wire them through the blob writer builder intoBlobFormatWriter. - Update
BlobFormatWriter::WriteBlobto open/validate descriptor streams before emitting any bytes, enabling NULL emission on open-time failures without partial output. - Map jindo file-not-found (
JDO_FILE_NOT_FOUND_ERROR) toStatus::NotExist(including lazy-not-found surfaced viagetFileLength), and add/extend unit tests for option propagation and null-write behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/paimon/testing/utils/test_helper.h | Add helper to build a one-row Arrow struct array containing a serialized blob descriptor for tests. |
| src/paimon/fs/jindo/jindo_utils.h | Add a jindo-status macro that maps file-not-found to Status::NotExist. |
| src/paimon/fs/jindo/jindo_utils_test.cpp | New unit tests for the new jindo status conversion macro (currently excluded from default build). |
| src/paimon/fs/jindo/jindo_file_system.cpp | Use the new macro in Open() and Length() to preserve NotExist classification (incl. lazy not-found). |
| src/paimon/format/blob/blob_writer_builder.h | Parse/write-null options from the options map and pass them into BlobFormatWriter::Create. |
| src/paimon/format/blob/blob_writer_builder_test.cpp | Add coverage for option parsing defaults/invalid values and basic behavior expectations. |
| src/paimon/format/blob/blob_format_writer.h | Extend Create()/ctor to accept the new booleans; add logger and a helper to open descriptor streams. |
| src/paimon/format/blob/blob_format_writer.cpp | Implement open-before-write + NULL-on-open-failure behavior gated by the two new options. |
| src/paimon/format/blob/blob_format_writer_test.cpp | Add tests validating NULL emission semantics, writer usability after rejected rows, and status-code-based classification via a mock FS. |
| src/paimon/format/blob/blob_file_format_factory_test.cpp | Add end-to-end test proving option propagation through FileFormatFactory::Get(...). |
| src/paimon/common/defs.cpp | Define the two new option string constants. |
| src/paimon/CMakeLists.txt | Register (commented-out) jindo utils test alongside other jindo tests. |
| include/paimon/defs.h | Public API: document and declare the two new option constants on Options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SteNicholas
force-pushed
the
blob-write-null-options
branch
from
0871da3 to
0425348
Compare
lxy-9602
reviewed
Jul 16, 2026
SteNicholas
force-pushed
the
blob-write-null-options
branch
from
0425348 to
da5454a
Compare
lxy-9602
reviewed
Jul 16, 2026
…ll-on-fetch-failure options Introduce two table options for descriptor BLOB writes and thread them from the table options map through BlobFileFormat/BlobWriterBuilder into BlobFormatWriter: - blob-write-null-on-missing-file: write NULL instead of failing the write when the file referenced by a blob descriptor does not exist. - blob-write-null-on-fetch-failure: write NULL when the referenced data cannot be fetched for other reasons, e.g. an invalid offset. BlobFormatWriter::WriteBlob now opens the descriptor input stream before writing any bytes, so a rejected row leaves no partial data in the output stream, while failures during the streaming copy still fail the write. JindoFileSystem maps JDO_FILE_NOT_FOUND_ERROR to Status::NotExist in Open() and JindoInputStream::Length() so that a missing OSS object is classified as a missing file rather than a generic IO error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Reorder the write-null parameters before fs/pool in BlobFormatWriter to match the Java constructor order, and document them on Create(). - Map JDO_FILE_NOT_FOUND_ERROR to Status::NotExist directly in PAIMON_RETURN_NOT_OK_FROM_JINDO and drop the _WITH_NOT_EXIST variant, making all jindo call sites consistent with LocalFileSystem. - Enable jindo_utils_test in fs_test when PAIMON_ENABLE_JINDO is ON; it only checks in-memory status conversion and needs no OSS cluster. - Add end-to-end write-null tests in blob_table_inte_test.cpp covering the NULL element merge with data files and the failure paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bad offset Add TestWriteNullOnFetchFailure to blob_table_inte_test.cpp: with blob-write-null-on-fetch-failure=true, a descriptor whose offset points beyond the end of the target file (a fetch failure that is not a missing file) is written as a NULL blob element, the row itself is kept in both the data file and the .blob file, and the read path merges them back with the blob column NULL for that row. Introduce a CorruptDescriptorOffset test helper that rewrites the serialized BlobDescriptor of a single row to reference an out-of-range offset, reusing the TransformBlobFields framework. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SteNicholas
force-pushed
the
blob-write-null-options
branch
from
b5be8ad to
991db0b
Compare
|
|
libarrow.a calls dlsym but the arrow imported target did not declare
${CMAKE_DL_LIBS} in its link interface, so -ldl could end up before
libarrow.a in the final link line. Linkers that resolve symbols strictly
left-to-right (gcc8-test in CI) then fail linking paimon-fs-test with
"undefined reference to dlsym / DSO missing from command line" once
fs_test started linking the jindo static libraries. Declare
${CMAKE_DL_LIBS} on the arrow interface, matching the lucene and
jindosdk::nextarch targets.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SteNicholas
force-pushed
the
blob-write-null-options
branch
from
d37e1da to
67f049d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: close #420
Align with the Java
BlobFormatWritersemantics for descriptor BLOB writes:blob-write-null-on-missing-file/blob-write-null-on-fetch-failureoption keys and thread them from the table options map throughBlobFileFormat/BlobWriterBuilderintoBlobFormatWriter.BlobFormatWriter::WriteBlobnow opens the descriptor input stream before writing any bytes, so an open-time fetch failure can be converted into a NULL element (kNullBinLength) without leaving partial data in the output stream. A missing file (Status::NotExist) is converted only byblob-write-null-on-missing-file; other open failures only byblob-write-null-on-fetch-failure; failures during the streaming copy keep failing the write, matching Java.JDO_FILE_NOT_FOUND_ERRORtoStatus::NotExistat the jindo boundary (JindoFileSystem::Open, andJindoInputStream::Lengthfor lazily-surfaced not-found) so the missing-file classification also holds on OSS, consistent withLocalFileSystem::Open. Note: this makes a missing consumer file on OSS take the existingIsNotExistbranch inConsumerManager::GetNextSnapshotId(returning "no next snapshot" like the local file system and Java) instead of surfacing anIOError.Tests
BlobFormatWriterWriteNullTest.TestWriteNullOnMissingFile/TestWriteNullOnFetchFailure: each option converts only its own failure class to NULL (with read-back verification), the other class still fails, and a rejected row leaves the writer usable.BlobFormatWriterWriteNullTest.TestWriteNullOnBothOptionsEnabled: both options together produce a mixed NULL/NULL/data file, verified by read-back.BlobFormatWriterWriteNullTest.TestWriteNullClassifiesByStatusCode: classification contract pinned on the open status code via a mock file system (NotExist/IOError× both option combinations), independent of the file system implementation.BlobWriterBuilderTest.TestWriteNullOptions: option parsing and wiring (absent keys default to false,"true"/"false"values, invalid value failsBuild).BlobFileFormatFactoryTest.TestWriteNullOptionPropagation: end-to-end option propagation throughFileFormatFactory::Get("blob", options).JindoUtilsTest: unit tests for the jindo status conversion (JDO_FILE_NOT_FOUND_ERROR→NotExist, other codes →IOError, ok pass-through, and the plain macro keepingIOError); registered commented-out infs_testalongside the other jindo tests since jindo is not built by default.Local validation:
paimon-blob-format-test39/39 passed;paimon-fs-testmatches the HEAD baseline. The jindo sources compile only with-DPAIMON_ENABLE_JINDO=ON(CI build entry).API and Format
Optionsstruct ininclude/paimon/defs.h(BLOB_WRITE_NULL_ON_MISSING_FILE,BLOB_WRITE_NULL_ON_FETCH_FAILURE), both defaulting to false, so behavior is unchanged unless opted in.kNullBinLengthencoding that readers already understand.Documentation
Option semantics are documented on the new constants in
include/paimon/defs.h.Generative AI tooling
Generated-by: Claude Code (Claude Fable 5)
🤖 Generated with Claude Code