Skip to content

Commit 3d5af47

Browse files
authored
fix: return error for reserved property keys in UpdateProperties::Set() (#817)
Previously, reserved table properties (except `format-version`) were silently ignored when set via `UpdateProperties::Set()`. This caused undetected configuration drift compared to Java, which throws exceptions for such attempts. Now attempts to set reserved properties accumulate a validation error via `ICEBERGER_BUILDER_CHECK`, surfaced at `Apply()` time. This matches Java's loud-failure behavior.
1 parent a45bce6 commit 3d5af47

3 files changed

Lines changed: 54 additions & 6 deletions

File tree

src/iceberg/test/update_properties_test.cc

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,53 @@ TEST_F(UpdatePropertiesTest, UpgradeFormatVersionUnsupported) {
129129
EXPECT_THAT(result, HasErrorMessage("unsupported format version"));
130130
}
131131

132+
TEST_F(UpdatePropertiesTest, SetReservedPropertyUuid) {
133+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
134+
update->Set("uuid", "some-uuid");
135+
136+
auto result = update->Apply();
137+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
138+
EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property"));
139+
}
140+
141+
TEST_F(UpdatePropertiesTest, SetReservedPropertyCurrentSchema) {
142+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
143+
update->Set("current-schema", R"({"type": "struct"})");
144+
145+
auto result = update->Apply();
146+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
147+
EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property"));
148+
}
149+
150+
TEST_F(UpdatePropertiesTest, SetReservedPropertyCurrentSnapshotId) {
151+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
152+
update->Set("current-snapshot-id", "12345");
153+
154+
auto result = update->Apply();
155+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
156+
EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property"));
157+
}
158+
159+
TEST_F(UpdatePropertiesTest, SetFormatVersionStillAllowed) {
160+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
161+
update->Set("format-version", "3");
162+
163+
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
164+
EXPECT_TRUE(result.updates.empty());
165+
ASSERT_TRUE(result.format_version.has_value());
166+
EXPECT_EQ(result.format_version.value(), 3);
167+
}
168+
169+
TEST_F(UpdatePropertiesTest, SetValidAndReservedProperties) {
170+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateProperties());
171+
update->Set("valid.key", "valid.value");
172+
update->Set("snapshot-count", "10");
173+
174+
auto result = update->Apply();
175+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
176+
EXPECT_THAT(result, HasErrorMessage("Cannot set reserved property"));
177+
}
178+
132179
TEST_F(UpdatePropertiesTest, CommitSuccess) {
133180
ICEBERG_UNWRAP_OR_FAIL(auto empty_update, table_->NewUpdateProperties());
134181
EXPECT_THAT(empty_update->Commit(), IsOk());

src/iceberg/update/update_properties.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ UpdateProperties& UpdateProperties::Set(const std::string& key,
5050
"Cannot set property '{}' that is already marked for removal",
5151
key);
5252

53-
if (!TableProperties::reserved_properties().contains(key) ||
54-
key == TableProperties::kFormatVersion.key()) {
55-
updates_.insert_or_assign(key, value);
56-
}
53+
ICEBERG_BUILDER_CHECK(!TableProperties::reserved_properties().contains(key) ||
54+
key == TableProperties::kFormatVersion.key(),
55+
"Cannot set reserved property: '{}'", key);
56+
updates_.insert_or_assign(key, value);
5757

5858
return *this;
5959
}

src/iceberg/update/update_properties.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ class ICEBERG_EXPORT UpdateProperties : public PendingUpdate {
5151

5252
/// \brief Sets a property key to a specified value.
5353
///
54-
/// The key must not have been previously marked for removal and reserved property keys
55-
/// will be ignored.
54+
/// The key must not have been previously marked for removal and must not be a
55+
/// reserved property key (except `format-version`). Setting a reserved property
56+
/// will result in a validation error at Apply() time.
5657
///
5758
/// \param key The property key to set
5859
/// \param value The property value to set

0 commit comments

Comments
 (0)