Skip to content

Commit 034ae54

Browse files
tomighitaclaude
andauthored
feat(schema): Add update_schema action to enable table schema updates (#2120)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123. --> - Closes #2119. ## What changes are included in this PR? <!-- Provide a summary of the modifications in this PR. List the main changes such as new features, bug fixes, refactoring, or any other updates. --> This PR creates an `UpdateSchema` which implements the `TransactionAction` allowing users of the crate to add new fields to or delete fields from an iceberg table by updating the table schema. It also checks that the added fields are either optional or have a default value to avoid data corruption downstream. ## Are these changes tested? <!-- Specify what test covers (unit test, integration test, etc.). If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> - [X] Unit tests for `UpdateSchema`. - [X] Adds one integration tests which tests that we can add data, update the table and can still read the data, ensuring backwards compatiblity and one integration test for deleting a schema. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 716394d commit 034ae54

4 files changed

Lines changed: 1441 additions & 0 deletions

File tree

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,284 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
//! Common schema-update behavior across catalogs.
19+
//!
20+
//! These tests assume Docker containers are started externally via `make docker-up`.
21+
22+
mod common;
23+
24+
use std::collections::HashMap;
25+
26+
use common::{CatalogKind, cleanup_namespace_dyn, load_catalog};
27+
use iceberg::spec::{NestedField, PrimitiveType, Schema, StructType, Type};
28+
use iceberg::transaction::{AddColumn, ApplyTransactionAction, Transaction};
29+
use iceberg::{ErrorKind, NamespaceIdent, Result, TableCreation, TableIdent};
30+
use iceberg_test_utils::normalize_test_name_with_parts;
31+
use rstest::rstest;
32+
33+
fn base_schema() -> Schema {
34+
Schema::builder()
35+
.with_fields(vec![
36+
NestedField::optional(1, "foo", Type::Primitive(PrimitiveType::String)).into(),
37+
NestedField::required(2, "bar", Type::Primitive(PrimitiveType::Int)).into(),
38+
NestedField::optional(3, "baz", Type::Primitive(PrimitiveType::Boolean)).into(),
39+
])
40+
.with_identifier_field_ids(vec![2])
41+
.build()
42+
.unwrap()
43+
}
44+
45+
// Common behavior: adding a top-level column appends it to the schema.
46+
// HMS is excluded because update_table is not yet supported.
47+
#[rstest]
48+
#[case::rest_catalog(CatalogKind::Rest)]
49+
#[case::glue_catalog(CatalogKind::Glue)]
50+
#[case::sql_catalog(CatalogKind::Sql)]
51+
#[case::s3tables_catalog(CatalogKind::S3Tables)]
52+
#[case::memory_catalog(CatalogKind::Memory)]
53+
#[tokio::test]
54+
async fn test_catalog_schema_add_column(#[case] kind: CatalogKind) -> Result<()> {
55+
let Some(harness) = load_catalog(kind).await else {
56+
return Ok(());
57+
};
58+
let catalog = harness.catalog;
59+
let namespace = NamespaceIdent::new(normalize_test_name_with_parts!(
60+
"catalog_schema_add_column",
61+
harness.label
62+
));
63+
64+
cleanup_namespace_dyn(catalog.as_ref(), &namespace).await;
65+
catalog.create_namespace(&namespace, HashMap::new()).await?;
66+
67+
let table_name =
68+
normalize_test_name_with_parts!("catalog_schema_add_column", harness.label, "table");
69+
let table = catalog
70+
.create_table(
71+
&namespace,
72+
TableCreation::builder()
73+
.name(table_name)
74+
.schema(base_schema())
75+
.build(),
76+
)
77+
.await?;
78+
79+
let tx = Transaction::new(&table);
80+
let tx = tx
81+
.update_schema()
82+
.add_column(AddColumn::optional(
83+
"a",
84+
Type::Primitive(PrimitiveType::Int),
85+
))
86+
.apply(tx)?;
87+
let updated = tx.commit(catalog.as_ref()).await?;
88+
89+
let schema = updated.metadata().current_schema();
90+
let field_a = schema.field_by_name("a").expect("field 'a' should exist");
91+
assert_eq!(field_a.id, 4);
92+
assert_eq!(*field_a.field_type, Type::Primitive(PrimitiveType::Int));
93+
94+
Ok(())
95+
}
96+
97+
// Common behavior: adding a nested struct column then a sub-field within it,
98+
// and deleting another top-level column, all persist correctly.
99+
// HMS is excluded because update_table is not yet supported.
100+
#[rstest]
101+
#[case::rest_catalog(CatalogKind::Rest)]
102+
#[case::glue_catalog(CatalogKind::Glue)]
103+
#[case::sql_catalog(CatalogKind::Sql)]
104+
#[case::s3tables_catalog(CatalogKind::S3Tables)]
105+
#[case::memory_catalog(CatalogKind::Memory)]
106+
#[tokio::test]
107+
async fn test_catalog_schema_add_nested_and_delete_column(#[case] kind: CatalogKind) -> Result<()> {
108+
let Some(harness) = load_catalog(kind).await else {
109+
return Ok(());
110+
};
111+
let catalog = harness.catalog;
112+
let namespace = NamespaceIdent::new(normalize_test_name_with_parts!(
113+
"catalog_schema_add_nested_and_delete_column",
114+
harness.label
115+
));
116+
117+
cleanup_namespace_dyn(catalog.as_ref(), &namespace).await;
118+
catalog.create_namespace(&namespace, HashMap::new()).await?;
119+
120+
let table_name = normalize_test_name_with_parts!(
121+
"catalog_schema_add_nested_and_delete_column",
122+
harness.label,
123+
"table"
124+
);
125+
let table = catalog
126+
.create_table(
127+
&namespace,
128+
TableCreation::builder()
129+
.name(table_name)
130+
.schema(base_schema())
131+
.build(),
132+
)
133+
.await?;
134+
135+
// First transaction: add a nested struct column.
136+
let tx = Transaction::new(&table);
137+
let tx = tx
138+
.update_schema()
139+
.add_column(AddColumn::optional(
140+
"info",
141+
Type::Struct(StructType::new(vec![
142+
NestedField::optional(0, "city", Type::Primitive(PrimitiveType::String)).into(),
143+
])),
144+
))
145+
.apply(tx)?;
146+
let table = tx.commit(catalog.as_ref()).await?;
147+
148+
// Second transaction: add a sub-field to the nested struct and delete a top-level column.
149+
let tx = Transaction::new(&table);
150+
let tx = tx
151+
.update_schema()
152+
.add_column(
153+
AddColumn::builder()
154+
.name("zip")
155+
.field_type(Type::Primitive(PrimitiveType::String))
156+
.parent("info")
157+
.build(),
158+
)
159+
.delete_column("baz")
160+
.apply(tx)?;
161+
let table = tx.commit(catalog.as_ref()).await?;
162+
163+
let schema = table.metadata().current_schema();
164+
assert!(schema.field_by_name("info").is_some());
165+
assert!(schema.field_by_name("info.city").is_some());
166+
assert!(schema.field_by_name("info.zip").is_some());
167+
assert!(schema.field_by_name("baz").is_none());
168+
169+
Ok(())
170+
}
171+
172+
// Common behavior: deleting an identifier field or a nonexistent field must fail.
173+
// HMS is excluded because update_table is not yet supported.
174+
#[rstest]
175+
#[case::rest_catalog(CatalogKind::Rest)]
176+
#[case::glue_catalog(CatalogKind::Glue)]
177+
#[case::sql_catalog(CatalogKind::Sql)]
178+
#[case::s3tables_catalog(CatalogKind::S3Tables)]
179+
#[case::memory_catalog(CatalogKind::Memory)]
180+
#[tokio::test]
181+
async fn test_catalog_schema_delete_invalid_column_errors(#[case] kind: CatalogKind) -> Result<()> {
182+
let Some(harness) = load_catalog(kind).await else {
183+
return Ok(());
184+
};
185+
let catalog = harness.catalog;
186+
let namespace = NamespaceIdent::new(normalize_test_name_with_parts!(
187+
"catalog_schema_delete_invalid_column_errors",
188+
harness.label
189+
));
190+
191+
cleanup_namespace_dyn(catalog.as_ref(), &namespace).await;
192+
catalog.create_namespace(&namespace, HashMap::new()).await?;
193+
194+
let table_name = normalize_test_name_with_parts!(
195+
"catalog_schema_delete_invalid_column_errors",
196+
harness.label,
197+
"table"
198+
);
199+
let table = catalog
200+
.create_table(
201+
&namespace,
202+
TableCreation::builder()
203+
.name(table_name)
204+
.schema(base_schema())
205+
.build(),
206+
)
207+
.await?;
208+
209+
// Deleting an identifier field must fail.
210+
let tx = Transaction::new(&table);
211+
let tx = tx.update_schema().delete_column("bar").apply(tx)?;
212+
let err = tx.commit(catalog.as_ref()).await.unwrap_err();
213+
assert_eq!(err.kind(), ErrorKind::PreconditionFailed);
214+
215+
// Deleting a nonexistent field must fail.
216+
let tx = Transaction::new(&table);
217+
let tx = tx.update_schema().delete_column("nonexistent").apply(tx)?;
218+
let err = tx.commit(catalog.as_ref()).await.unwrap_err();
219+
assert_eq!(err.kind(), ErrorKind::PreconditionFailed);
220+
221+
Ok(())
222+
}
223+
224+
// Common behavior: loading a table after a schema update reflects the new schema.
225+
// HMS is excluded because update_table is not yet supported.
226+
#[rstest]
227+
#[case::rest_catalog(CatalogKind::Rest)]
228+
#[case::glue_catalog(CatalogKind::Glue)]
229+
#[case::sql_catalog(CatalogKind::Sql)]
230+
#[case::s3tables_catalog(CatalogKind::S3Tables)]
231+
#[case::memory_catalog(CatalogKind::Memory)]
232+
#[tokio::test]
233+
async fn test_catalog_schema_update_persisted_after_reload(
234+
#[case] kind: CatalogKind,
235+
) -> Result<()> {
236+
let Some(harness) = load_catalog(kind).await else {
237+
return Ok(());
238+
};
239+
let catalog = harness.catalog;
240+
let namespace = NamespaceIdent::new(normalize_test_name_with_parts!(
241+
"catalog_schema_update_persisted_after_reload",
242+
harness.label
243+
));
244+
245+
cleanup_namespace_dyn(catalog.as_ref(), &namespace).await;
246+
catalog.create_namespace(&namespace, HashMap::new()).await?;
247+
248+
let table_name = normalize_test_name_with_parts!(
249+
"catalog_schema_update_persisted_after_reload",
250+
harness.label,
251+
"table"
252+
);
253+
let table_ident = TableIdent::new(namespace.clone(), table_name.clone());
254+
let table = catalog
255+
.create_table(
256+
&namespace,
257+
TableCreation::builder()
258+
.name(table_name)
259+
.schema(base_schema())
260+
.build(),
261+
)
262+
.await?;
263+
264+
let tx = Transaction::new(&table);
265+
let tx = tx
266+
.update_schema()
267+
.add_column(AddColumn::optional(
268+
"new_field",
269+
Type::Primitive(PrimitiveType::Long),
270+
))
271+
.apply(tx)?;
272+
tx.commit(catalog.as_ref()).await?;
273+
274+
let reloaded = catalog.load_table(&table_ident).await?;
275+
assert!(
276+
reloaded
277+
.metadata()
278+
.current_schema()
279+
.field_by_name("new_field")
280+
.is_some()
281+
);
282+
283+
Ok(())
284+
}

crates/iceberg/src/spec/schema/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ pub type SchemaId = i32;
5151
pub type SchemaRef = Arc<Schema>;
5252
/// Default schema id.
5353
pub const DEFAULT_SCHEMA_ID: SchemaId = 0;
54+
/// Delimiter for schema name, which denotes a nested struct.
55+
pub const SCHEMA_NAME_DELIMITER: &str = ".";
5456

5557
/// Defines schema in iceberg.
5658
#[derive(Debug, Serialize, Deserialize, Clone)]

crates/iceberg/src/transaction/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,15 @@ mod snapshot;
5858
mod sort_order;
5959
mod update_location;
6060
mod update_properties;
61+
mod update_schema;
6162
mod update_statistics;
6263
mod upgrade_format_version;
6364

6465
use std::sync::Arc;
6566
use std::time::Duration;
6667

6768
use backon::{BackoffBuilder, ExponentialBackoff, ExponentialBuilder, RetryableWithContext};
69+
pub use update_schema::AddColumn;
6870

6971
use crate::error::Result;
7072
use crate::spec::TableProperties;
@@ -74,6 +76,7 @@ use crate::transaction::append::FastAppendAction;
7476
use crate::transaction::sort_order::ReplaceSortOrderAction;
7577
use crate::transaction::update_location::UpdateLocationAction;
7678
use crate::transaction::update_properties::UpdatePropertiesAction;
79+
use crate::transaction::update_schema::UpdateSchemaAction;
7780
use crate::transaction::update_statistics::UpdateStatisticsAction;
7881
use crate::transaction::upgrade_format_version::UpgradeFormatVersionAction;
7982
use crate::{Catalog, TableCommit, TableRequirement, TableUpdate};
@@ -136,6 +139,11 @@ impl Transaction {
136139
UpdatePropertiesAction::new()
137140
}
138141

142+
/// Creates an update schema action.
143+
pub fn update_schema(&self) -> UpdateSchemaAction {
144+
UpdateSchemaAction::new()
145+
}
146+
139147
/// Creates a fast append action.
140148
pub fn fast_append(&self) -> FastAppendAction {
141149
FastAppendAction::new()

0 commit comments

Comments
 (0)