Skip to content

Commit ae5429a

Browse files
SteNicholasclaude
andcommitted
fix(test): let the fetch-failure inte test expect a converted missing file
TestWriteNullOnFetchFailureKeepsMissingFileFailing still asserted that a missing descriptor file fails the write when only 'blob-write-null-on-fetch-failure' is enabled. That is the behavior the previous commit deliberately removed: the FileSystem::Exists check that tells a missing file apart runs only when 'blob-write-null-on-missing-file' needs the classification, so with just the fetch-failure option a missing file is an ordinary open failure and is converted to a NULL element. Only the comment and the expected message were updated, so the test failed on CI in every build job. Rename it to TestWriteNullOnFetchFailureCoversMissingFile and assert the conversion the way the neighbouring write-null tests do: the write and commit succeed, both files keep the full row count, and the read merges them back into a row whose blob is NULL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ef3da8c commit ae5429a

1 file changed

Lines changed: 40 additions & 6 deletions

File tree

test/inte/blob_table_inte_test.cpp

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -766,9 +766,11 @@ TEST_P(BlobTableInteTest, TestWriteNullOnFetchFailure) {
766766
<< "expected:" << expected_with_rk->ToString();
767767
}
768768

769-
TEST_P(BlobTableInteTest, TestWriteNullOnFetchFailureKeepsMissingFileFailing) {
770-
// blob-write-null-on-fetch-failure does not cover a missing file, which is identified with
771-
// FileSystem::Exists and left to blob-write-null-on-missing-file.
769+
TEST_P(BlobTableInteTest, TestWriteNullOnFetchFailureCoversMissingFile) {
770+
// The existence check that tells a missing file apart runs only when
771+
// blob-write-null-on-missing-file needs the classification. With only
772+
// blob-write-null-on-fetch-failure enabled a missing file is just an open failure, so it is
773+
// converted to a NULL blob element like any other fetch failure.
772774
arrow::FieldVector fields = {arrow::field("f0", arrow::utf8()),
773775
arrow::field("f1", arrow::int32()),
774776
BlobUtils::ToArrowField("blob", true)};
@@ -788,16 +790,48 @@ TEST_P(BlobTableInteTest, TestWriteNullOnFetchFailureKeepsMissingFileFailing) {
788790

789791
std::string raw_json = R"([
790792
["str_0", 0, "blob_data_0"],
791-
["str_1", 1, "blob_data_1"]
793+
["str_1", 1, "blob_data_1"],
794+
["str_2", 2, "blob_data_2"]
792795
])";
793796
auto raw_array = std::dynamic_pointer_cast<arrow::StructArray>(
794797
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields), raw_json).ValueOrDie());
795798
ASSERT_OK_AND_ASSIGN(auto desc_array, ConvertRawBlobToDescriptor(raw_array, {"blob"}));
799+
800+
// Remove the file behind row 1's descriptor so the write observes a missing file.
796801
ASSERT_OK(DeleteDescriptorTarget(desc_array, "blob", /*row=*/1));
797802

798803
auto schema = arrow::schema(fields);
799-
ASSERT_NOK_WITH_MSG(WriteArray(table_path, {}, schema->field_names(), {desc_array}),
800-
"does not exist");
804+
ASSERT_OK_AND_ASSIGN(auto commit_msgs,
805+
WriteArray(table_path, {}, schema->field_names(), {desc_array}));
806+
ASSERT_OK(Commit(table_path, commit_msgs));
807+
808+
// Both the main data file and the .blob file keep the full row count; the read path
809+
// merges them back into rows where row 1's blob is NULL.
810+
ASSERT_OK_AND_ASSIGN(auto plan, ScanTable(table_path));
811+
VerifyDataFileMetas(plan, /*expected_file_count=*/2, /*expected_row_counts=*/{3, 3},
812+
/*expected_min_seqs=*/{1, 1}, /*expected_max_seqs=*/{1, 1},
813+
/*expected_first_row_ids=*/{0, 0},
814+
/*expected_write_cols=*/
815+
{std::vector<std::string>{"f0", "f1"}, std::vector<std::string>{"blob"}});
816+
817+
ASSERT_OK_AND_ASSIGN(auto result, ReadTable(table_path, schema->field_names(), plan));
818+
ASSERT_TRUE(result.chunked_array);
819+
auto read_concat = arrow::Concatenate(result.chunked_array->chunks()).ValueOrDie();
820+
auto read_struct = std::dynamic_pointer_cast<arrow::StructArray>(read_concat);
821+
ASSERT_OK_AND_ASSIGN(auto resolved, ConvertDescriptorToRawBlob(read_struct, {"blob"}));
822+
823+
std::string expected_json = R"([
824+
["str_0", 0, "blob_data_0"],
825+
["str_1", 1, null],
826+
["str_2", 2, "blob_data_2"]
827+
])";
828+
auto expected_array = std::dynamic_pointer_cast<arrow::StructArray>(
829+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields), expected_json)
830+
.ValueOrDie());
831+
ASSERT_OK_AND_ASSIGN(auto expected_with_rk, PrependRowKindColumn(expected_array));
832+
ASSERT_TRUE(resolved->Equals(expected_with_rk))
833+
<< "result:" << resolved->ToString() << std::endl
834+
<< "expected:" << expected_with_rk->ToString();
801835
}
802836

803837
TEST_P(BlobTableInteTest, TestBasic) {

0 commit comments

Comments
 (0)