Skip to content

Test : experiment load page index with column#7624

Draft
zhuqi-lucas wants to merge 3 commits intoapache:mainfrom
zhuqi-lucas:load_page_index_with_column_id
Draft

Test : experiment load page index with column#7624
zhuqi-lucas wants to merge 3 commits intoapache:mainfrom
zhuqi-lucas:load_page_index_with_column_id

Conversation

@zhuqi-lucas
Copy link
Contributor

@zhuqi-lucas zhuqi-lucas commented Jun 7, 2025

Which issue does this PR close?

Test : experiment load page index with column

Closes #NNN.

Rationale for this change

Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.

What changes are included in this PR?

There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.

Are there any user-facing changes?

If there are user-facing changes then we may require documentation to be updated before approving the PR.

If there are any breaking changes to public APIs, please call them out.

@etseidl
Copy link
Contributor

etseidl commented Jun 9, 2025

Thanks for all your recent work on query improvement @zhuqi-lucas. Great stuff! 😄

I know this is draft, but if I could offer one suggestion early on it would be to not pass the column_ids around, but instead add an Option<Vec<usize>> to ParquetMetaDataReader as well as a with_column_ids function. That will allow for less internal API thrash.

@zhuqi-lucas
Copy link
Contributor Author

Thanks for all your recent work on query improvement @zhuqi-lucas. Great stuff! 😄

I know this is draft, but if I could offer one suggestion early on it would be to not pass the column_ids around, but instead add an Option<Vec<usize>> to ParquetMetaDataReader as well as a with_column_ids function. That will allow for less internal API thrash.

Thank you @etseidl for review, it makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants