Skip to content

Conversation

@gabotechs
Copy link
Collaborator

@gabotechs gabotechs commented Aug 8, 2025

While integrating TPCH benchmarks with the codebase, I realize that the SessionBuilder trait needs to allow more flexibility to users for configuring their session.

In order to understand what's needed, it's convenient to know how users create DataFusion sessions:

  1. They create their SessionStateBuilder, and start building their session. ✅ we allow configuring this
  2. They call .build() and get a SessionState, which can be further configured. ❌ we don't allow configuring this
  3. They create a new SessionContext out of the state, which can be further configured. ❌ we don't allow configuring this

This PR adds two more methods to the SessionBuilder trait, and it now looks like this:

pub trait SessionBuilder {
    fn session_state_builder(&self, builder: SessionStateBuilder) -> Result<SessionStateBuilder, DataFusionError>;
    async fn session_state(&self, state: SessionState) -> Result<SessionState, DataFusionError>;
    async fn session_context(&self, ctx: SessionContext) -> Result<SessionContext, DataFusionError>;
}

This allows the user to configure:

  1. SessionStateBuilder
  2. SessionState
  3. SessionContext

This unlocks wiring up the datafusion-distributed project with the TPCH benchmarks from #73

@gabotechs gabotechs force-pushed the gabrielmusat/move-common-test-utils-to-one-place branch from 6766309 to 6a8b5b0 Compare August 8, 2025 11:34
…ture. This way, other crates and integration tests as well can use it
@gabotechs gabotechs force-pushed the gabrielmusat/extend-session-builder-trait branch from b371273 to 82a2c82 Compare August 8, 2025 11:35
…he `SessionStateBuilder` level, but also on `SessionState` and `SessionContext`
@gabotechs gabotechs force-pushed the gabrielmusat/extend-session-builder-trait branch from 82a2c82 to b7b62be Compare August 8, 2025 11:46
* Add benchmarks crate by copying upstream DataFusion code

* Wire-up distributed execution with tpch benchmarks (#86)
@robtandy robtandy merged commit 7809267 into gabrielmusat/move-common-test-utils-to-one-place Aug 8, 2025
3 checks passed
@robtandy robtandy deleted the gabrielmusat/extend-session-builder-trait branch August 8, 2025 15:38
robtandy pushed a commit that referenced this pull request Aug 8, 2025
…ture (#84)

* Move all test utils to src/ and hide them behind an "integration" feature. This way, other crates and integration tests as well can use it

* Extend SessionBuilder trait to operate on SessionState and SessionContext (#85)

* Move all test utils to src/ and hide them behind an "integration" feature. This way, other crates and integration tests as well can use it

* Extend the `SessionBuilder` trait to be able to operate not only at the `SessionStateBuilder` level, but also on `SessionState` and `SessionContext`

* Add benchmarks crate by copying upstream DataFusion code (#73)

* Add benchmarks crate by copying upstream DataFusion code

* Wire-up distributed execution with tpch benchmarks (#86)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants