Skip to content

minor: Move PreparedAccessPlan to same module as ParquetAccessPlan#20929

Merged
Dandandan merged 1 commit intoapache:mainfrom
alamb:alamb/consolidate_prepared_plan
Mar 14, 2026
Merged

minor: Move PreparedAccessPlan to same module as ParquetAccessPlan#20929
Dandandan merged 1 commit intoapache:mainfrom
alamb:alamb/consolidate_prepared_plan

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Mar 13, 2026

Which issue does this PR close?

Rationale for this change

While working on morselization, I found that the PreparedAccessPlan was in a different module which made it harder to understand the relationship between the two. Thus, I think it would be better to move PreparedAccessPlan to the same module as ParquetAccessPlan.

What changes are included in this PR?

  1. Move PreparedAccessPlan: Relocated the PreparedAccessPlan struct and its implementation from datafusion/datasource-parquet/src/opener.rs to datafusion/datasource-parquet/src/access_plan.rs.
  2. Unify Logic: Added a prepare method to ParquetAccessPlan to create a PreparedAccessPlan, replacing the PreparedAccessPlan::from_access_plan static method.

Are these changes tested?

Yes by CI

Are there any user-facing changes?

No

@github-actions github-actions bot added the datasource Changes to the datasource crate label Mar 13, 2026
@alamb alamb force-pushed the alamb/consolidate_prepared_plan branch from 9e03577 to 105131b Compare March 13, 2026 17:40
@alamb alamb requested a review from zhuqi-lucas March 13, 2026 17:41
@alamb alamb marked this pull request as ready for review March 13, 2026 17:41
@alamb
Copy link
Contributor Author

alamb commented Mar 13, 2026

Hi @zhuqi-lucas -- I wonder if you are interested in reviewing this PR

Copy link
Contributor

@zhuqi-lucas zhuqi-lucas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @alamb !

}

/// Prepare this plan and resolve to the final `PreparedAccessPlan`
pub(crate) fn prepare(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice addition

@Dandandan Dandandan added this pull request to the merge queue Mar 14, 2026
Merged via the queue into apache:main with commit c74976f Mar 14, 2026
30 checks passed
@alamb alamb deleted the alamb/consolidate_prepared_plan branch March 15, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants