alexanderbianchi commented on code in PR #67:
URL: https://github.com/apache/datafusion-iceberg/pull/67#discussion_r4383788885
##########
crates/datafusion/src/read/scan.rs:
##########
@@ -145,28 +221,88 @@ impl IcebergDataSourceBuilder {
self
}
- /// Builds the source, which reads the columns of its projected schema by
name.
+ /// Select files during [`Self::build`] when enabled. Defaults to `false`.
+ pub fn with_eager_planning(mut self, enabled: bool) -> Self {
+ self.eager_planning = enabled;
+ self
+ }
+
+ /// Builds the source and its shared reader. When eager planning is
enabled,
+ /// discovers files using `context` for partition count and memory
accounting.
+ /// Lazy builds perform no file-discovery I/O.
///
/// # Errors
///
- /// Returns an error if the projection is out of bounds for the schema.
- pub fn build(self) -> Result<IcebergDataSource> {
- let schema = match &self.projection {
- None => self.schema,
- Some(projection) => Arc::new(self.schema.project(projection)?),
+ /// Returns an error if the projection is invalid, file planning fails, or
+ /// retained tasks exceed the query memory pool.
+ pub async fn build(self, context: Arc<TaskContext>) ->
Result<IcebergDataSource> {
Review Comment:
With [[DFD
#803](https://github.com/datafusion-contrib/datafusion-distributed/pull/803)](https://github.com/datafusion-contrib/datafusion-distributed/pull/803),
I’m reconsidering whether eager execution should have a separate data source
here.
The useful boundary seems to be discovery versus execution of
already-assigned files. An assigned source would own explicit file groups and
read them without needing the discovery request state or knowing whether those
files came from eager planning, a catalog, or a decoded worker plan.
That could give us a source both projects use directly: local planning
constructs it here, while DFD assigns files to tasks, handles placement, and
supplies a codec that reconstructs those exact groups on workers. DFD’s lazy
work-unit-feed lifecycle could remain separate, without becoming part of the
upstream source’s API.
This isn’t required for distributed support—a single source could expose the
same capabilities—but it seems like a useful ownership boundary, beyond just
avoiding conditionals. It could also let improvements such as ordered reading
live in one shared implementation.
Would it make sense to establish that separation in #67, while we’re still
shaping the API?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
