Add a black-box RTAS behavior test suite

[mkuchenbecker]

Summary

A cohesive set of black-box RTAS tests, driven through Spark SQL against a real embedded OpenHouse
server (RtasBehaviorTest). RTAS (CREATE OR REPLACE TABLE ... AS SELECT) defines a brand new table
body, so it is intentionally allowed schema/partition evolutions the incremental ALTER TABLE path
forbids, while still preserving the table's identity. The suite pins that contract so a future change
can't silently apply the update-path guards to the replace path or lose the table identity.

Tests (RtasBehaviorTest)

• testRtasMayDropColumn — RTAS may drop a column.
• testRtasMayAddColumn — RTAS may add a column.
• testRtasMayChangePartitionSpec — RTAS may repartition by a different column.
• testRtasMayRemovePartitioning — RTAS may replace a partitioned table with an unpartitioned body.
• testRtasReplacesData — the row set after RTAS reflects the SELECT.
• testRtasPreservesTableIdentity — openhouse.tableUUID is preserved across the replace.
• testRtasRejectedWhenReplaceNotEnabled — RTAS is rejected unless replace.enabled is set.

Testing Done

• RtasBehaviorTest (all 7) — green on the spark-3.5 itest. Spotless clean.

[mkuchenbecker]

used in one funciton, not needed ot be a helper.

[mkuchenbecker]

Done (7dbce83) — inlined dataFilePaths (it was only used in this one test).

[mkuchenbecker]

remove the helper and inline.

[mkuchenbecker]

tableProperties is used in 5 tests, so I kept it as a shared helper rather than duplicate the same SHOW TBLPROPERTIES stream in each. I inlined the genuinely single-use helpers (dataFilePaths, tableArg). Happy to inline this one too if you'd rather. (7dbce83)

[mkuchenbecker]

remove the helper and inline.

[mkuchenbecker]

columnsOf is used in 3 tests — same reasoning as the tableProperties thread: kept it shared to avoid duplicating the DESCRIBE TABLE stream. Single-use helpers were inlined. (7dbce83)

[mkuchenbecker]

why is this in 3.5 dir vs 3.1? I would prefer all tests added in 3.5.

[mkuchenbecker]

Moved — all RTAS tests now live in the spark-3.5 itest dir (3.5-only). This file (RtasDmlAfterReplaceTest) is deleted; its coverage is folded into the new RtasDmlMatrixTest. (7dbce83)

[mkuchenbecker]

Test with ORC too. Fix the classpath issue.

[mkuchenbecker]

Fixed the root cause rather than working around it. The itest classpath carried both the shaded ORC bundled in the runtime uber jar (org.apache.iceberg.shaded...orc) and the unshaded iceberg-data/orc-core, so Iceberg's merge-on-read delete loader mixed the two on read-back (the ClassCastException). Excluded iceberg-data from the module (same as apps/spark-3.5), so merge-on-read now reads back on both ORC and Parquet. The new matrix exercises ORC × merge-on-read across the full DML set. (7dbce83)

[mkuchenbecker]

you need to test DML after replace. You have not applied teh lessons of the framework. Write a Set of DML tests that take in a table. That should work normally, after rtas, after rtas+restore, with or without MoR enabled.

The test for dml is "i have a table, do dml tests" and we are preparing different tables for that. Its fine to test the restore behaviour like we are, but DML needs to be tested after the restore.

I see it as (BaseTable, BaseTAble + RTAS, BaseTable + RTAS + Restore, (Basetable + Mor), (Basetable + Mor) +RTAS,...).foreach(table -> runAllDmlTests(Table))

We get to the list of preparations by multiplexing over properties.

[mkuchenbecker]

Rebuilt as RtasDmlMatrixTest. Table preparations are multiplexed over properties — write mode (copy-on-write / merge-on-read) × file format (Parquet / ORC) × partitioning × lifecycle (base, RTAS, RTAS+restore) — and the same DML set (read / insert / delete / update / merge / insert-overwrite) runs against every preparation. Every prep converges to the same canonical seed, so one assertion is valid for all of them: 144 cases (155 RTAS tests total). DML is now tested after RTAS and after RTAS+restore, with and without merge-on-read.

One behavior note: with PARTITIONED BY (data) each seed row is its own single-row file, so a single-row merge-on-read DELETE removes the whole file as a metadata-only delete (no position delete) — correct Iceberg behavior — so the delete-file assertion is scoped to the unpartitioned cases; row-level correctness is asserted everywhere. (7dbce83)

[mkuchenbecker]

map and stream functions, not nested fors.

[mkuchenbecker]

Done — cases() is now a flatMap cartesian product over the axis enums, no nested loops. (621dc27)

[mkuchenbecker]

add block comment explanations

[mkuchenbecker]

Added block comments on each axis enum, the preparation thunks, withTable, and the delete-file gating. (621dc27)

[mkuchenbecker]

rather than switch on prepare seeded table just treat each preparation as a lamba that creates a table. A thunk, this can ensure the table is cleaned up. and manage the lifecycle

withTable(params) {
table -> {
table with scoped lifetime
}
}

[mkuchenbecker]

Done. Each Lifecycle constant is now a preparation thunk (prepare(...) builds+seeds the table), and withTable(axes, table -> { ... }) gives the table a scoped lifetime — it's always dropped in finally. The prepare switch is gone. (621dc27)

[mkuchenbecker]

this CASE seems overly verbose. I would remove this runDml function.

if you have the test tables you could to a testTables.foreach(TestInsert) vs a "run DML" function.

[mkuchenbecker]

Removed runDml. Each DML operation is now a DmlCheck enum constant with its own run(...) that does the op and asserts, so the test body is just "prepare a table, apply the op". (621dc27)

[mkuchenbecker]

Overall I think the structure needs work. Think of it more funcitonal than data driven.

Build up the set of tables as preparations. For each preparaation, run each DML test (each can be their own test). The whole point is that we have a list of preparations that can be plugged into each dml test and it should work.

[mkuchenbecker]

this doesn't need to be one giant statement. reduce nesting

[mkuchenbecker]

The giant nested statement is gone: with each DML operation now a composable check run in sequence on one prepared table, the parameter source is just preparations() (the 4 preparation axes), and the per-op DmlCheck/runDml dispatch was removed entirely. (982bc1f)

[mkuchenbecker]

just inline and remove dmlcheck

[mkuchenbecker]

Removed DmlCheck. Each DML operation is now an inline composable check (readCheck/insertCheck/deleteCheck/updateCheck/mergeCheck/insertOverwriteCheck) applied directly to the prepared table. (982bc1f)

[mkuchenbecker]

get rid of this pattern. This is logic embedded in a test. The test construciton should do the assertion even if we end up duplicating some setup.

[mkuchenbecker]

Removed. There's no expectDeleteFiles flag / conditional-assert pattern anymore — merge-on-read delete-file production is asserted only in the dedicated mergeOnRead*WritesDeleteFiles tests, which are constructed for exactly the case (unpartitioned merge-on-read) that produces position delete files. (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

remove helper

[mkuchenbecker]

Addressed in the rewrite. The composable-check redesign removed the shared helper this referred to (the preparation is now a plain switch calling the composed steps, and the DML checks are inline). (982bc1f)

[mkuchenbecker]

don't assert if you don;t expect delete files.

[mkuchenbecker]

Fixed. The general DML checks no longer assert on delete files at all; only the dedicated unpartitioned merge-on-read tests — which are built specifically to produce position delete files — assert they exist. (982bc1f)

[mkuchenbecker]

make props kv

[mkuchenbecker]

Done. Table properties are now authored as a key/value Map<String,String> (tableProps) and rendered into the TBLPROPERTIES (...) body at the SQL boundary (tblProperties), instead of a hand-concatenated string. (982bc1f)

[mkuchenbecker]

override is a smell here. you have over-abstracted. This should just be a function call.

[mkuchenbecker]

Done. Removed the enum abstract-method override; Lifecycle is now a plain enum and prepare(...) dispatches with a simple switch that calls the composed steps directly — just a function call, no anonymous-subclass hierarchy. (982bc1f)

[mkuchenbecker]

Assersions should be delta-based not absolute. This is too brittle. Tests should compose.

[mkuchenbecker]

Done. The DML checks are now delta-based and compose: each check asserts its own effect relative to the current table state (a count delta, or a row it just wrote), so they're order-independent and repeatable — insertCheck(t); insertCheck(t) both pass — and the whole sequence runs against a single prepared table. (982bc1f)