Skip to content

Commit 3ce07d5

Browse files
committed
feat(datafusion): reconstruct IcebergTableScan from a predicate
Add a `new_with_tasks_from_predicate` constructor plus `table_schema()` and `projection_indices()` getters so a downstream consumer can rebuild the node without the original DataFusion `Expr`s — e.g. a distributed-plan codec that ships a serialized `Predicate` across workers. The node now keeps the pre-projection schema and the numeric projection indices verbatim: the provider caches its schema while reloading the table separately, so re-deriving them would be incorrect. All struct construction still funnels through `new_inner`, now parameterized by `Option<Predicate>`; `new`/`new_with_tasks` keep their `&[Expr]` signatures and pre-convert. The catalog-backed provider passes its already-computed predicate straight to the constructor, dropping the duplicate `filters -> Predicate` conversion.
1 parent 79b97fd commit 3ce07d5

2 files changed

Lines changed: 190 additions & 12 deletions

File tree

  • crates/integrations/datafusion/src

crates/integrations/datafusion/src/physical_plan/scan.rs

Lines changed: 186 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,12 @@ use crate::to_datafusion_error;
3939

4040
/// Iceberg [`Table`] scan as a DataFusion [`ExecutionPlan`].
4141
///
42-
/// Has two construction modes: [`IcebergTableScan::new`] for a lazy
43-
/// single-partition scan, and [`IcebergTableScan::new_with_tasks`] for an
44-
/// eager multi-partition scan over pre-planned [`FileScanTask`] buckets.
42+
/// Has three construction modes: [`new`][Self::new] (lazy, single-partition),
43+
/// [`new_with_tasks`][Self::new_with_tasks] (eager, over pre-planned
44+
/// [`FileScanTask`] buckets), and
45+
/// [`new_with_tasks_from_predicate`][Self::new_with_tasks_from_predicate]
46+
/// (eager, from a [`Predicate`]) — the last lets a node be rebuilt from its
47+
/// getters, e.g. by a distributed-plan codec.
4548
///
4649
/// Note: in eager mode the underlying `TableScan` is rebuilt on every
4750
/// `execute(partition)` call. The per-build cost is bounded (no I/O) and
@@ -57,6 +60,12 @@ pub struct IcebergTableScan {
5760
plan_properties: Arc<PlanProperties>,
5861
/// Projection column names, None means all columns.
5962
projection: Option<Vec<String>>,
63+
/// Full schema before projection. Kept verbatim, not re-derived: the
64+
/// provider caches it while reloading the table, so it can diverge from the
65+
/// table's current metadata.
66+
table_schema: ArrowSchemaRef,
67+
/// Projection indices as received by `scan`; `projection` keeps only names.
68+
projection_indices: Option<Vec<usize>>,
6069
/// Filters to apply to the table scan.
6170
predicates: Option<Predicate>,
6271
/// Pre-planned file scan tasks per partition (eager mode), or `None` (lazy mode).
@@ -84,7 +93,7 @@ impl IcebergTableScan {
8493
snapshot_id,
8594
schema,
8695
projection,
87-
filters,
96+
convert_filters_to_predicate(filters),
8897
limit,
8998
Partitioning::UnknownPartitioning(1),
9099
None,
@@ -111,7 +120,34 @@ impl IcebergTableScan {
111120
snapshot_id,
112121
schema,
113122
projection,
114-
filters,
123+
convert_filters_to_predicate(filters),
124+
limit,
125+
partitioning,
126+
Some(buckets),
127+
)
128+
}
129+
130+
/// Eager variant taking a [`Predicate`] instead of [`Expr`] filters, so a
131+
/// node can be rebuilt from its getters. The predicate is unbound; the scan
132+
/// builder binds it at `execute` time.
133+
// Arity mirrors `new_with_tasks`; an args struct is deferred.
134+
#[allow(clippy::too_many_arguments)]
135+
pub fn new_with_tasks_from_predicate(
136+
table: Table,
137+
snapshot_id: Option<i64>,
138+
schema: ArrowSchemaRef,
139+
projection: Option<&Vec<usize>>,
140+
predicate: Option<Predicate>,
141+
limit: Option<usize>,
142+
buckets: Vec<Vec<FileScanTask>>,
143+
partitioning: Partitioning,
144+
) -> Self {
145+
Self::new_inner(
146+
table,
147+
snapshot_id,
148+
schema,
149+
projection,
150+
predicate,
115151
limit,
116152
partitioning,
117153
Some(buckets),
@@ -124,7 +160,7 @@ impl IcebergTableScan {
124160
snapshot_id: Option<i64>,
125161
schema: ArrowSchemaRef,
126162
projection: Option<&Vec<usize>>,
127-
filters: &[Expr],
163+
predicate: Option<Predicate>,
128164
limit: Option<usize>,
129165
partitioning: Partitioning,
130166
buckets: Option<Vec<Vec<FileScanTask>>>,
@@ -139,15 +175,18 @@ impl IcebergTableScan {
139175
EmissionType::Incremental,
140176
Boundedness::Bounded,
141177
));
178+
let table_schema = Arc::clone(&schema);
179+
let projection_indices = projection.cloned();
142180
let projection = get_column_names(schema, projection);
143-
let predicates = convert_filters_to_predicate(filters);
144181

145182
Self {
146183
table,
147184
snapshot_id,
148185
plan_properties,
149186
projection,
150-
predicates,
187+
table_schema,
188+
projection_indices,
189+
predicates: predicate,
151190
buckets,
152191
partition_keys_kind: None,
153192
limit,
@@ -166,6 +205,10 @@ impl IcebergTableScan {
166205
&self.table
167206
}
168207

208+
pub fn table_schema(&self) -> &ArrowSchemaRef {
209+
&self.table_schema
210+
}
211+
169212
pub fn snapshot_id(&self) -> Option<i64> {
170213
self.snapshot_id
171214
}
@@ -174,6 +217,10 @@ impl IcebergTableScan {
174217
self.projection.as_deref()
175218
}
176219

220+
pub fn projection_indices(&self) -> Option<&[usize]> {
221+
self.projection_indices.as_deref()
222+
}
223+
177224
pub fn predicates(&self) -> Option<&Predicate> {
178225
self.predicates.as_ref()
179226
}
@@ -383,3 +430,134 @@ pub(super) fn get_column_names(
383430
.collect::<Vec<String>>()
384431
})
385432
}
433+
434+
#[cfg(test)]
435+
mod tests {
436+
use std::sync::Arc;
437+
438+
use datafusion::arrow::datatypes::{DataType, Field, Schema as ArrowSchema};
439+
use datafusion::physical_plan::ExecutionPlan;
440+
use datafusion::prelude::{Expr, col, lit};
441+
use iceberg::TableIdent;
442+
use iceberg::expr::Reference;
443+
use iceberg::io::FileIO;
444+
use iceberg::spec::Datum;
445+
use iceberg::table::{StaticTable, Table};
446+
447+
use super::*;
448+
449+
async fn test_table() -> Table {
450+
let metadata_path = format!(
451+
"{}/tests/test_data/TableMetadataV2Valid.json",
452+
env!("CARGO_MANIFEST_DIR")
453+
);
454+
let ident = TableIdent::from_strs(["ns", "scan_table"]).unwrap();
455+
StaticTable::from_metadata_file(&metadata_path, ident, FileIO::new_with_fs())
456+
.await
457+
.unwrap()
458+
.into_table()
459+
}
460+
461+
fn arrow_schema() -> ArrowSchemaRef {
462+
Arc::new(ArrowSchema::new(vec![
463+
Field::new("x", DataType::Int64, false),
464+
Field::new("y", DataType::Int64, false),
465+
Field::new("z", DataType::Int64, false),
466+
]))
467+
}
468+
469+
fn filters() -> Vec<Expr> {
470+
vec![col("x").gt(lit(5i64))]
471+
}
472+
473+
// The Expr and predicate constructors must agree: `new_with_tasks` only
474+
// pre-converts the filters that `new_with_tasks_from_predicate` takes raw.
475+
#[tokio::test]
476+
async fn expr_and_predicate_constructors_agree() {
477+
let table = test_table().await;
478+
let projection = vec![0usize, 2];
479+
let from_filters = IcebergTableScan::new_with_tasks(
480+
table.clone(),
481+
None,
482+
arrow_schema(),
483+
Some(&projection),
484+
&filters(),
485+
Some(100),
486+
vec![vec![], vec![]],
487+
Partitioning::UnknownPartitioning(2),
488+
);
489+
let from_predicate = IcebergTableScan::new_with_tasks_from_predicate(
490+
table,
491+
None,
492+
arrow_schema(),
493+
Some(&projection),
494+
convert_filters_to_predicate(&filters()),
495+
Some(100),
496+
vec![vec![], vec![]],
497+
Partitioning::UnknownPartitioning(2),
498+
);
499+
500+
assert_eq!(from_filters.predicates(), from_predicate.predicates());
501+
assert_eq!(from_filters.projection(), from_predicate.projection());
502+
assert_eq!(
503+
from_filters.schema().fields(),
504+
from_predicate.schema().fields()
505+
);
506+
assert_eq!(
507+
format!("{:?}", from_filters.properties().output_partitioning()),
508+
format!("{:?}", from_predicate.properties().output_partitioning()),
509+
);
510+
}
511+
512+
// table_schema() exposes the full pre-projection schema; schema() is projected.
513+
#[tokio::test]
514+
async fn getters_expose_full_schema_and_indices() {
515+
let projection = vec![0usize, 2];
516+
let scan = IcebergTableScan::new(
517+
test_table().await,
518+
None,
519+
arrow_schema(),
520+
Some(&projection),
521+
&[],
522+
None,
523+
);
524+
525+
assert_eq!(scan.table_schema().fields(), arrow_schema().fields());
526+
assert_eq!(scan.table_schema().fields().len(), 3);
527+
assert_eq!(scan.schema().fields().len(), 2);
528+
assert_ne!(scan.schema().fields(), scan.table_schema().fields());
529+
530+
assert_eq!(scan.projection_indices(), Some(projection.as_slice()));
531+
let names = ["x".to_string(), "z".to_string()];
532+
assert_eq!(scan.projection(), Some(names.as_slice()));
533+
}
534+
535+
// Without a projection, indices/names are None and the full schema is kept.
536+
#[tokio::test]
537+
async fn no_projection_keeps_full_schema() {
538+
let scan = IcebergTableScan::new(test_table().await, None, arrow_schema(), None, &[], None);
539+
540+
assert_eq!(scan.projection_indices(), None);
541+
assert_eq!(scan.projection(), None);
542+
assert_eq!(scan.schema().fields(), scan.table_schema().fields());
543+
assert_eq!(scan.table_schema().fields(), arrow_schema().fields());
544+
}
545+
546+
// A predicate passed to the new constructor round-trips unchanged.
547+
#[tokio::test]
548+
async fn predicate_round_trips() {
549+
let predicate = Reference::new("x").greater_than(Datum::long(5));
550+
let scan = IcebergTableScan::new_with_tasks_from_predicate(
551+
test_table().await,
552+
None,
553+
arrow_schema(),
554+
None,
555+
Some(predicate.clone()),
556+
None,
557+
vec![vec![]],
558+
Partitioning::UnknownPartitioning(1),
559+
);
560+
561+
assert_eq!(scan.predicates(), Some(&predicate));
562+
}
563+
}

crates/integrations/datafusion/src/table/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ impl TableProvider for IcebergTableProvider {
208208
Some(names) => builder.select(names),
209209
None => builder.select_all(),
210210
};
211-
if let Some(pred) = predicate {
212-
builder = builder.with_filter(pred);
211+
if let Some(pred) = &predicate {
212+
builder = builder.with_filter(pred.clone());
213213
}
214214

215215
let tasks: Vec<FileScanTask> = builder
@@ -258,12 +258,12 @@ impl TableProvider for IcebergTableProvider {
258258
};
259259

260260
Ok(Arc::new(
261-
IcebergTableScan::new_with_tasks(
261+
IcebergTableScan::new_with_tasks_from_predicate(
262262
table,
263263
None, // Always use current snapshot for catalog-backed provider
264264
self.schema.clone(),
265265
projection,
266-
filters,
266+
predicate,
267267
limit,
268268
buckets,
269269
partitioning,

0 commit comments

Comments
 (0)