Skip to content

Commit 84c293b

Browse files
feat: derive ALTER TABLE isBlindAppend from per-op policy
Replaces the hardcoded isBlindAppend=false with a per-op fold over SchemaOperation::is_blind_append(). All current ops (add/drop/rename/ drop-not-null) are blind-append, matching delta-spark. The flag is emitted by apply_schema_operations via SchemaEvolutionResult.is_blind_append alongside the operation string fold, no second iteration.
1 parent 540a036 commit 84c293b

5 files changed

Lines changed: 97 additions & 13 deletions

File tree

kernel/src/transaction/alter_table.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ impl AlterTableTransaction {
3939
effective_table_config: TableConfiguration,
4040
committer: Box<dyn Committer>,
4141
operation: String,
42+
is_blind_append: bool,
4243
) -> DeltaResult<Self> {
4344
let span = tracing::info_span!(
4445
"txn",
@@ -66,10 +67,7 @@ impl AlterTableTransaction {
6667
data_change: false,
6768
shared_write_state: OnceLock::new(),
6869
engine_commit_info: None,
69-
// TODO(#2446): match delta-spark's per-op isBlindAppend policy
70-
// (ADD/DROP/DROP NOT NULL -> true, SET NOT NULL -> false). Hardcoded false for
71-
// now: safe, but misses the true-case optimization delta-spark applies.
72-
is_blind_append: false,
70+
is_blind_append,
7371
dv_matched_files: vec![],
7472
physical_clustering_columns: None,
7573
_state: PhantomData,

kernel/src/transaction/builder/alter_table.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ impl<S: Buildable> AlterTableTransactionBuilder<S> {
224224
schema: evolved_schema,
225225
new_max_column_id,
226226
operation,
227+
is_blind_append,
227228
} = apply_schema_operations(
228229
schema,
229230
self.operations,
@@ -255,6 +256,7 @@ impl<S: Buildable> AlterTableTransactionBuilder<S> {
255256
evolved_table_config,
256257
committer,
257258
operation,
259+
is_blind_append,
258260
)
259261
}
260262
}

kernel/src/transaction/mod.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,20 @@ pub trait SupportsDataFiles {}
197197
impl SupportsDataFiles for ExistingTable {}
198198
impl SupportsDataFiles for CreateTable {}
199199

200+
/// Per-state constants consumed by validation. Implemented by every transaction state marker
201+
/// (`ExistingTable`, `CreateTable`, `AlterTable`); each supplies its own values, and validation
202+
/// reads `S::<CONST>` to branch on the current state.
203+
pub trait TransactionKind {
204+
/// Whether the transaction produces only metadata changes (no data files). Metadata-only
205+
/// commits have no data-file invariants to check during blind-append validation.
206+
const IS_METADATA_ONLY: bool = false;
207+
}
208+
impl TransactionKind for ExistingTable {}
209+
impl TransactionKind for CreateTable {}
210+
impl TransactionKind for AlterTable {
211+
const IS_METADATA_ONLY: bool = true;
212+
}
213+
200214
/// A transaction represents an in-progress write to a table. After creating a transaction, changes
201215
/// to the table may be staged via the transaction methods before calling `commit` to commit the
202216
/// changes to the table.
@@ -340,7 +354,10 @@ impl<S> Transaction<S> {
340354
),
341355
err
342356
)]
343-
pub fn commit(self, engine: &dyn Engine) -> DeltaResult<CommitResult<S>> {
357+
pub fn commit(self, engine: &dyn Engine) -> DeltaResult<CommitResult<S>>
358+
where
359+
S: TransactionKind,
360+
{
344361
info!(
345362
num_add_files = self.add_files_metadata.len(),
346363
num_remove_files = self.remove_files_metadata.len(),
@@ -665,7 +682,10 @@ impl<S> Transaction<S> {
665682
/// Note: Domain metadata additions/removals are allowed; blind append only constrains
666683
/// data-file operations and read predicates. Conflict resolution determines whether
667684
/// metadata changes are problematic.
668-
fn validate_blind_append_semantics(&self) -> DeltaResult<()> {
685+
fn validate_blind_append_semantics(&self) -> DeltaResult<()>
686+
where
687+
S: TransactionKind,
688+
{
669689
if !self.is_blind_append {
670690
return Ok(());
671691
}
@@ -675,6 +695,10 @@ impl<S> Transaction<S> {
675695
"Blind append is not supported for create-table transactions",
676696
)
677697
);
698+
// Metadata-only transactions (e.g. ALTER TABLE) have no data-file operations to check
699+
if S::IS_METADATA_ONLY {
700+
return Ok(());
701+
}
678702
require!(
679703
!self.add_files_metadata.is_empty(),
680704
Error::invalid_transaction_state("Blind append requires at least one added data file")

kernel/src/transaction/schema_evolution.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,17 @@ impl SchemaOperation {
4747
SchemaOperation::SetNullable { .. } => "CHANGE COLUMNS",
4848
}
4949
}
50+
51+
/// Whether this operation is eligible to be committed as a blind append. A commit is a
52+
/// blind append only if every operation in it is eligible.
53+
pub(crate) fn is_blind_append(&self) -> bool {
54+
match self {
55+
SchemaOperation::AddColumn { .. } => true,
56+
SchemaOperation::DropColumn { .. } => true,
57+
SchemaOperation::RenameColumn { .. } => true,
58+
SchemaOperation::SetNullable { .. } => true,
59+
}
60+
}
5061
}
5162

5263
// Helper to transform a nested column. For each component in `remaining`, locates the
@@ -185,6 +196,9 @@ pub(crate) struct SchemaEvolutionResult {
185196
/// `"ADD COLUMNS"`, `"ADD COLUMNS + CHANGE COLUMNS"`). Used as the commit's
186197
/// `operation` field.
187198
pub operation: String,
199+
/// Whether the commit is eligible to be marked `isBlindAppend=true`. True only if every
200+
/// operation in the batch is eligible (see [`SchemaOperation::is_blind_append`]).
201+
pub is_blind_append: bool,
188202
}
189203

190204
/// Applies a sequence of schema operations to the given schema, returning the evolved schema.
@@ -223,12 +237,15 @@ pub(crate) fn apply_schema_operations(
223237
// Distinct operation names in first-occurrence order; joined into the commit's
224238
// operation string at the end.
225239
let mut op_names: Vec<&'static str> = Vec::new();
240+
// Cumulative blind-append eligibility across the batch; false as soon as any op isn't blind.
241+
let mut is_blind_append = true;
226242

227243
for op in operations {
228244
let name = op.operation_name();
229245
if !op_names.contains(&name) {
230246
op_names.push(name);
231247
}
248+
is_blind_append &= op.is_blind_append();
232249
match op {
233250
// Protocol feature checks for the field's data type (e.g. `timestampNtz`) happen
234251
// later when the caller builds a new TableConfiguration from the evolved schema --
@@ -488,6 +505,7 @@ pub(crate) fn apply_schema_operations(
488505
schema: evolved_schema.into(),
489506
new_max_column_id,
490507
operation: op_names.join(" + "),
508+
is_blind_append,
491509
})
492510
}
493511

kernel/tests/alter_table.rs

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -916,23 +916,34 @@ async fn rename_clustering_column_fails() -> Result<(), Box<dyn std::error::Erro
916916
// OPERATION STRING tests
917917
// ============================================================================
918918

919-
// Reads `commitInfo.operation` from a commit JSON written to the local filesystem.
920-
fn read_commit_operation(table_path: &str, version: u64) -> String {
919+
// Reads the `commitInfo` object from a commit JSON written to the local filesystem.
920+
fn read_commit_info(table_path: &str, version: u64) -> serde_json::Value {
921921
let log_file = format!("{table_path}/_delta_log/{version:020}.json");
922922
let contents = std::fs::read_to_string(&log_file).expect("read log file");
923923
for line in contents.lines() {
924924
let action: serde_json::Value = serde_json::from_str(line).expect("parse action");
925925
if let Some(ci) = action.get("commitInfo") {
926-
return ci
927-
.get("operation")
928-
.and_then(|v| v.as_str())
929-
.expect("operation field")
930-
.to_string();
926+
return ci.clone();
931927
}
932928
}
933929
panic!("no commitInfo in commit {version}");
934930
}
935931

932+
fn read_commit_operation(table_path: &str, version: u64) -> String {
933+
read_commit_info(table_path, version)
934+
.get("operation")
935+
.and_then(|v| v.as_str())
936+
.expect("operation field")
937+
.to_string()
938+
}
939+
940+
fn read_commit_is_blind_append(table_path: &str, version: u64) -> bool {
941+
read_commit_info(table_path, version)
942+
.get("isBlindAppend")
943+
.and_then(|v| v.as_bool())
944+
.expect("isBlindAppend field")
945+
}
946+
936947
#[tokio::test]
937948
async fn alter_commit_operation_add_columns() -> DeltaResult<()> {
938949
let (_temp_dir, table_path, engine) = test_table_setup()?;
@@ -981,3 +992,34 @@ async fn alter_commit_operation_mixed_ops_joined_with_plus() -> DeltaResult<()>
981992
);
982993
Ok(())
983994
}
995+
996+
#[tokio::test]
997+
async fn alter_commit_is_blind_append_true_for_add_columns() -> DeltaResult<()> {
998+
let (_temp_dir, table_path, engine) = test_table_setup()?;
999+
let snapshot =
1000+
create_table_and_load_snapshot(&table_path, simple_schema(), engine.as_ref(), &[])?;
1001+
snapshot
1002+
.alter_table()
1003+
.add_column(StructField::nullable("email", DataType::STRING))
1004+
.build(engine.as_ref(), committer())?
1005+
.commit(engine.as_ref())?
1006+
.unwrap_committed();
1007+
assert!(read_commit_is_blind_append(&table_path, 1));
1008+
Ok(())
1009+
}
1010+
1011+
#[tokio::test]
1012+
async fn alter_commit_is_blind_append_true_for_mixed_blind_ops() -> DeltaResult<()> {
1013+
let (_temp_dir, table_path, engine) = test_table_setup()?;
1014+
let snapshot =
1015+
create_table_and_load_snapshot(&table_path, simple_schema(), engine.as_ref(), &[])?;
1016+
snapshot
1017+
.alter_table()
1018+
.add_column(StructField::nullable("email", DataType::STRING))
1019+
.set_nullable(column_name!("id"))
1020+
.build(engine.as_ref(), committer())?
1021+
.commit(engine.as_ref())?
1022+
.unwrap_committed();
1023+
assert!(read_commit_is_blind_append(&table_path, 1));
1024+
Ok(())
1025+
}

0 commit comments

Comments
 (0)