Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a43741b
init
NoahKusaba Jul 21, 2026
63e9b61
merge tests
NoahKusaba Jul 21, 2026
5a8e388
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 21, 2026
60b5c1a
cleanup
NoahKusaba Jul 21, 2026
4fe1ebb
make tests assert batch
NoahKusaba Jul 21, 2026
345b0b3
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 22, 2026
b97ce72
fix read-insert-read snapshot
NoahKusaba Jul 23, 2026
f49bf31
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 23, 2026
edb11db
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 24, 2026
3e42a9e
fix schema version error
NoahKusaba Jul 24, 2026
a9215c7
fix test
NoahKusaba Jul 24, 2026
a29b460
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 24, 2026
cb218e5
make snapshot_arrow_schema public
NoahKusaba Jul 25, 2026
9d92b11
reload table in with_snapshot_id, get rid of Table Parameter
NoahKusaba Jul 26, 2026
bd0c036
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 26, 2026
fe72134
simplify tests
NoahKusaba Jul 26, 2026
bfd3aca
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Jul 27, 2026
43e123d
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Aug 1, 2026
4e25485
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Aug 4, 2026
1c71c7d
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Aug 5, 2026
1258792
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Aug 6, 2026
97e24f5
Merge branch 'main' into feature/datafusion-snapshot-id
NoahKusaba Aug 7, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions crates/integrations/datafusion/public-api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ pub fn iceberg_datafusion::IcebergStaticTableProvider::schema(&self) -> arrow_sc
pub fn iceberg_datafusion::IcebergStaticTableProvider::supports_filters_pushdown(&self, filters: &[&datafusion_expr::expr::Expr]) -> datafusion_common::error::Result<alloc::vec::Vec<datafusion_expr::table_source::TableProviderFilterPushDown>>
pub fn iceberg_datafusion::IcebergStaticTableProvider::table_type(&self) -> datafusion_expr::table_source::TableType
pub struct iceberg_datafusion::table::IcebergTableProvider
impl iceberg_datafusion::IcebergTableProvider
pub fn iceberg_datafusion::IcebergTableProvider::snapshot_id(&self) -> core::option::Option<i64>
pub fn iceberg_datafusion::IcebergTableProvider::with_snapshot_id(self, snapshot_id: core::option::Option<i64>) -> Self
impl core::clone::Clone for iceberg_datafusion::IcebergTableProvider
pub fn iceberg_datafusion::IcebergTableProvider::clone(&self) -> iceberg_datafusion::IcebergTableProvider
impl core::fmt::Debug for iceberg_datafusion::IcebergTableProvider
Expand Down Expand Up @@ -114,6 +117,9 @@ pub fn iceberg_datafusion::IcebergStaticTableProvider::schema(&self) -> arrow_sc
pub fn iceberg_datafusion::IcebergStaticTableProvider::supports_filters_pushdown(&self, filters: &[&datafusion_expr::expr::Expr]) -> datafusion_common::error::Result<alloc::vec::Vec<datafusion_expr::table_source::TableProviderFilterPushDown>>
pub fn iceberg_datafusion::IcebergStaticTableProvider::table_type(&self) -> datafusion_expr::table_source::TableType
pub struct iceberg_datafusion::IcebergTableProvider
impl iceberg_datafusion::IcebergTableProvider
pub fn iceberg_datafusion::IcebergTableProvider::snapshot_id(&self) -> core::option::Option<i64>
pub fn iceberg_datafusion::IcebergTableProvider::with_snapshot_id(self, snapshot_id: core::option::Option<i64>) -> Self
impl core::clone::Clone for iceberg_datafusion::IcebergTableProvider
pub fn iceberg_datafusion::IcebergTableProvider::clone(&self) -> iceberg_datafusion::IcebergTableProvider
impl core::fmt::Debug for iceberg_datafusion::IcebergTableProvider
Expand Down
150 changes: 148 additions & 2 deletions crates/integrations/datafusion/src/table/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ pub struct IcebergTableProvider {
table_ident: TableIdent,
/// A reference-counted arrow `Schema` (cached at construction)
schema: ArrowSchemaRef,
/// Optional snapshot to read. `None` reads the current snapshot (refreshed
/// from the catalog on each scan); `Some` pins reads to that snapshot for
/// time-travel. Writes always target the current table state.
snapshot_id: Option<i64>,
}

impl IcebergTableProvider {
Expand All @@ -94,9 +98,23 @@ impl IcebergTableProvider {
catalog,
table_ident,
schema,
snapshot_id: None,
})
}

/// Pins reads to a specific snapshot for time-travel. `None` (the default)
/// reads the current snapshot. The snapshot id is threaded into the scan
/// node, so it is serialized and honored by a distributed engine as well.
pub fn with_snapshot_id(mut self, snapshot_id: Option<i64>) -> Self {
self.snapshot_id = snapshot_id;
self
}
Comment on lines +139 to +144

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reloads table metadata from the catalog purely to validate the snapshot and recompute the schema, and scan() (line 178-183 below) reloads again right after. That's two catalog round trips before a query that needs one, and it makes what reads like a builder setter into a fallible, async network call. Per the comment above it, this cost exists specifically to handle providers the catalog hands out and caches across calls. Is that caching concern this type's to own, or should the reload-for-freshness logic live wherever the caching actually happens (the Ballista scheduler side, by the description upthread)?


/// Returns the snapshot this provider reads, if pinned for time-travel.
pub fn snapshot_id(&self) -> Option<i64> {
self.snapshot_id
}

pub(crate) async fn metadata_table(
&self,
r#type: MetadataTableType,
Expand Down Expand Up @@ -131,10 +149,10 @@ impl TableProvider for IcebergTableProvider {
.await
.map_err(to_datafusion_error)?;

// Create scan with fresh metadata (always use current snapshot)
// Create scan with fresh metadata, honoring a pinned snapshot if set.
Ok(Arc::new(IcebergTableScan::new(
table,
None, // Always use current snapshot for catalog-backed provider
self.snapshot_id,
self.schema.clone(),
projection,
filters,
Expand Down Expand Up @@ -894,4 +912,132 @@ mod tests {
"Limit should be None when not specified"
);
}

/// Runs `SELECT * FROM t` against `provider` and returns the result batches.
async fn scan_rows(
provider: IcebergTableProvider,
) -> Vec<datafusion::arrow::array::RecordBatch> {
let ctx = SessionContext::new();
ctx.register_table("t", Arc::new(provider)).unwrap();
ctx.sql("SELECT * FROM t")
.await
.unwrap()
.collect()
.await
.unwrap()
}

#[tokio::test]
async fn test_pinned_snapshot_reads_historical_data() {
use datafusion::assert_batches_sorted_eq;

let (catalog, namespace, table_name, _temp_dir) = get_test_catalog_and_table().await;

// First append -> snapshot with a single row.
let writer =
IcebergTableProvider::try_new(catalog.clone(), namespace.clone(), table_name.clone())
.await
.unwrap();
let ctx = SessionContext::new();
ctx.register_table("t", Arc::new(writer)).unwrap();
ctx.sql("INSERT INTO t VALUES (1, 'a')")
.await
.unwrap()
.collect()
.await
.unwrap();

// Capture the snapshot produced by the first append.
let table_ident = TableIdent::new(namespace.clone(), table_name.clone());
let first_snapshot = catalog
.load_table(&table_ident)
.await
.unwrap()
.metadata()
.current_snapshot()
.unwrap()
.snapshot_id();

// Second append -> current snapshot now has two rows.
ctx.sql("INSERT INTO t VALUES (2, 'b')")
.await
.unwrap()
.collect()
.await
.unwrap();

// Default (unpinned) provider sees the latest state: both rows.
let current =
IcebergTableProvider::try_new(catalog.clone(), namespace.clone(), table_name.clone())
.await
.unwrap();
assert_batches_sorted_eq!(
[
"+----+------+",
"| id | name |",
"+----+------+",
"| 1 | a |",
"| 2 | b |",
"+----+------+",
],
&scan_rows(current).await
);

// Pinning the first snapshot time-travels: only the first row is visible,
// even though a newer snapshot exists.
let pinned =
IcebergTableProvider::try_new(catalog.clone(), namespace.clone(), table_name.clone())
.await
.unwrap()
.with_snapshot_id(Some(first_snapshot));
assert_eq!(pinned.snapshot_id(), Some(first_snapshot));

// The pin is threaded onto the scan node itself — this is the value a
// distributed engine's codec serializes to reproduce the scan remotely.
let scan_plan = pinned
.scan(&SessionContext::new().state(), None, &[], None)
.await
.unwrap();
let iceberg_scan = scan_plan
.downcast_ref::<IcebergTableScan>()
.expect("Expected IcebergTableScan");
assert_eq!(
iceberg_scan.snapshot_id(),
Some(first_snapshot),
"pinned snapshot should propagate to the scan node"
);

// And it changes what is actually read: only the historical row.
assert_batches_sorted_eq!(
[
"+----+------+",
"| id | name |",
"+----+------+",
"| 1 | a |",
"+----+------+",
],
&scan_rows(pinned).await
);

// Clearing the pin (Some -> None) unpins back to the current snapshot,
// so the newer row becomes visible again.
let unpinned =
IcebergTableProvider::try_new(catalog.clone(), namespace.clone(), table_name.clone())
.await
.unwrap()
.with_snapshot_id(Some(first_snapshot))
.with_snapshot_id(None);
assert_eq!(unpinned.snapshot_id(), None);
assert_batches_sorted_eq!(
[
"+----+------+",
"| id | name |",
"+----+------+",
"| 1 | a |",
"| 2 | b |",
"+----+------+",
],
&scan_rows(unpinned).await
);
}
}
Loading