Skip to content

Commit 48b2d2d

Browse files
committed
fix
1 parent 176d94a commit 48b2d2d

3 files changed

Lines changed: 64 additions & 1 deletion

File tree

src/paimon/common/table/special_fields.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include "arrow/type_fwd.h"
2424
#include "paimon/common/types/data_field.h"
25+
#include "paimon/common/utils/string_utils.h"
2526
#include "paimon/utils/special_field_ids.h"
2627

2728
namespace paimon {
@@ -63,6 +64,15 @@ struct SpecialFields {
6364
return data_field;
6465
}
6566

67+
static bool IsSystemField(const std::string& field_name) {
68+
if (StringUtils::StartsWith(field_name, KEY_FIELD_PREFIX)) {
69+
return true;
70+
}
71+
return field_name == SequenceNumber().Name() || field_name == ValueKind().Name() ||
72+
field_name == RowKind().Name() || field_name == RowId().Name() ||
73+
field_name == IndexScore().Name();
74+
}
75+
6676
static bool IsSpecialFieldName(const std::string& field_name) {
6777
if (field_name == SequenceNumber().Name() || field_name == ValueKind().Name() ||
6878
field_name == RowKind().Name() || field_name == RowId().Name() ||

src/paimon/core/operation/commit/row_id_column_conflict_checker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ Result<std::optional<int32_t>> RowIdColumnConflictChecker::FieldId(
189189
return std::optional<int32_t>(it->second);
190190
}
191191

192-
if (SpecialFields::IsSpecialFieldName(write_col)) {
192+
if (SpecialFields::IsSystemField(write_col)) {
193193
return std::optional<int32_t>();
194194
}
195195

test/inte/data_evolution_table_test.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,19 @@ class DataEvolutionTableTest : public ::testing::Test,
130130
return file_store_commit->Commit(commit_msgs);
131131
}
132132

133+
Status CommitWithRowIdCheckFromSnapshot(
134+
const std::string& table_path,
135+
const std::vector<std::shared_ptr<CommitMessage>>& commit_msgs,
136+
std::optional<int64_t> row_id_check_from_snapshot) const {
137+
CommitContextBuilder commit_builder(table_path, "commit_user_1");
138+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<CommitContext> commit_context,
139+
commit_builder.Finish());
140+
PAIMON_ASSIGN_OR_RAISE(std::unique_ptr<FileStoreCommit> file_store_commit,
141+
FileStoreCommit::Create(std::move(commit_context)));
142+
file_store_commit->RowIdCheckConflict(row_id_check_from_snapshot);
143+
return file_store_commit->Commit(commit_msgs);
144+
}
145+
133146
Status ScanAndRead(const std::string& table_path, const std::vector<std::string>& read_schema,
134147
const std::shared_ptr<arrow::StructArray>& expected_array,
135148
const std::shared_ptr<Predicate>& predicate = nullptr,
@@ -316,6 +329,46 @@ TEST_P(DataEvolutionTableTest, TestBasic) {
316329
}
317330
}
318331

332+
TEST_P(DataEvolutionTableTest, TestCommitConflictOnOverlappedRowIdAndWriteColumns) {
333+
CreateTable();
334+
std::string table_path = PathUtil::JoinPath(dir_->Str(), "foo.db/bar");
335+
336+
// Snapshot 1: initialize row id range [0, 0].
337+
std::vector<std::string> init_write_cols = {"f0", "f1", "f2"};
338+
auto init_array = std::dynamic_pointer_cast<arrow::StructArray>(
339+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_(fields_), R"([
340+
[1, "a", "b"]
341+
])")
342+
.ValueOrDie());
343+
ASSERT_OK_AND_ASSIGN(auto init_msgs, WriteArray(table_path, init_write_cols, init_array));
344+
ASSERT_OK(Commit(table_path, init_msgs));
345+
346+
// Snapshot 2: update f2 at row id 0.
347+
std::vector<std::string> write_cols = {"f2"};
348+
auto src_array_1 = std::dynamic_pointer_cast<arrow::StructArray>(
349+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[2]}), R"([
350+
["c"]
351+
])")
352+
.ValueOrDie());
353+
ASSERT_OK_AND_ASSIGN(auto commit_msgs_1, WriteArray(table_path, write_cols, src_array_1));
354+
SetFirstRowId(/*reset_first_row_id=*/0, commit_msgs_1);
355+
ASSERT_OK(Commit(table_path, commit_msgs_1));
356+
357+
// Snapshot 3 attempt: update f2 at row id 0 again, and check history from snapshot 1.
358+
// This should conflict with snapshot 2 because row-id range and write columns overlap.
359+
auto src_array_2 = std::dynamic_pointer_cast<arrow::StructArray>(
360+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({fields_[2]}), R"([
361+
["d"]
362+
])")
363+
.ValueOrDie());
364+
ASSERT_OK_AND_ASSIGN(auto commit_msgs_2, WriteArray(table_path, write_cols, src_array_2));
365+
SetFirstRowId(/*reset_first_row_id=*/0, commit_msgs_2);
366+
ASSERT_NOK_WITH_MSG(
367+
CommitWithRowIdCheckFromSnapshot(table_path, commit_msgs_2,
368+
/*row_id_check_from_snapshot=*/1),
369+
"multiple MERGE INTO operations have encountered conflicts while checking row-id history");
370+
}
371+
319372
TEST_P(DataEvolutionTableTest, TestMultipleAppends) {
320373
CreateTable();
321374
std::string table_path = PathUtil::JoinPath(dir_->Str(), "foo.db/bar");

0 commit comments

Comments
 (0)