alexanderbianchi commented on PR #27210:
URL: https://github.com/apache/datafusion/pull/27210#issuecomment-6777514534

   I’m wondering whether this exposes an opportunity for a more general 
table-resolution extension point, with time travel as one implementation.
   
   The PR gets an important part right: resolving the selected provider before 
column binding. My concern is the amount of version-specific plumbing across 
`SessionState`, `ContextProvider`, schema providers, and separate table caches.
   
   Could we instead translate parsed modifiers into an extension-defined 
request, then pass that through a common resolution path? Illustratively:
   
   ```rust
   // Proposed preparation hook on the existing RelationPlanner.
   // Runs before async table resolution.
   fn prepare_table(
       &self,
       table: &TableFactor,
       ctx: &mut dyn TablePreparationContext,
   ) -> Result<Option<TableResolutionRequest>>;
   ```
   
   The SQL extension would use the preparation context to evaluate expressions 
and produce a parser-independent request. The schema provider would consume it:
   
   ```rust
   // Proposed SchemaProvider method.
   // An empty request delegates to ordinary table(name) lookup.
   async fn resolve_table(
       &self,
       name: &str,
       request: &TableResolutionRequest,
   ) -> Result<Option<ResolvedTable>>;
   
   struct ResolvedTable {
       provider: Arc<dyn TableProvider>,
       identity: Option<ReadIdentity>,
   }
   ```
   
   These are sketches rather than exact proposed types. 
`TableResolutionRequest` would carry typed extension arguments; `ReadIdentity` 
would distinguish resolved reads for logical-plan equality.
   
   For example:
   
   ```text
   events AT(VERSION => 'release_tag')
     → SQL extension produces TimeTravelRequest::Version("release_tag")
     → core routes it to the schema provider
     → Iceberg resolves the tag and returns a snapshot-bound provider
     → normal planning binds columns using that provider’s schema
   ```
   
   The SQL extension and catalog implementation share the request type. 
`SessionState` orchestrates resolution without interpreting version semantics, 
and the catalog never needs to depend on sqlparser’s AST.
   
   The existing `RelationPlanner` runs after prefetching, so this would require 
a preparation hook or equivalent integration with the earlier resolution phase. 
Unsupported modifiers would still need to fail explicitly.
   
   We’d also need to preserve:
   - Consistent expression evaluation using the query’s planning context.
   - Resolved-table storage that distinguishes requests while allowing reuse, 
without inherently requiring a separate versioned cache.
   - Read identity in scan equality/hashing; separate provider instances alone 
aren’t sufficient.
   - Consistency between the resolved schema and execution, including across 
serialization.
   
   Would this be worth exploring before committing to the version-specific 
APIs? I’m motivated to help make it work, and have outlined related 
query-scoped catalog/provider limitations in 
https://github.com/apache/datafusion-iceberg/issues/24. Thank you for working 
on this.


-- 
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]

Reply via email to