Skip to content

[SPARK-37928][SQL][TESTS] Add Parquet Data Page V2 test scenario to DataSourceReadBenchmark #35226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
LuciferYang wants to merge 2 commits into from
Closed

[SPARK-37928][SQL][TESTS] Add Parquet Data Page V2 test scenario to DataSourceReadBenchmark #35226

LuciferYang wants to merge 2 commits into from

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jan 17, 2022

What changes were proposed in this pull request?

This PR adds a corresponding Parquet Data Page V2 test scenario for each Parquet Data Page V1 test scenario to DataSourceReadBenchmark.

Why are the changes needed?

Add micro benchmark for Parquet Data Page V2.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GA

@github-actions github-actions bot added the SQL label Jan 17, 2022
@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jan 17, 2022

cc @dongjoon-hyun @sunchao @viirya I give this pr to adds a corresponding Parquet Data Page V2 test scenario for each Parquet Data Page V1 test scenario to DataSourceReadBenchmark, if @parthchandra plans to do this part of work in SPARK-37912, I will close this pr.

@parthchandra
Copy link
Contributor

parthchandra commented Jan 18, 2022

I was planning to add these in subsequent PR)s) which includes implementation of the remaining encodings for Parquet V2.
But since this is done, and supersedes my PR, let's merge this one and I can close my PR (or rename it to just address the review comments for the original delta binary PR).

@parthchandra parthchandra mentioned this pull request Jan 18, 2022
Closed
sunchao
sunchao approved these changes Jan 18, 2022
View reviewed changes
Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

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

LGTM

sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/DataSourceReadBenchmark.scala
val enableOffHeapColumnVector = spark.sessionState.conf.offHeapColumnVectorEnabled
val vectorizedReaderBatchSize = spark.sessionState.conf.parquetVectorizedReaderBatchSize
// Decoding in vectorized but having the reader return rows.
parquetReaderBenchmark
Copy link
Member

@sunchao sunchao Jan 18, 2022

Choose a reason for hiding this comment

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

nit: I think we can just put this in the first withParquetVersions above?

Copy link
Contributor Author

@LuciferYang LuciferYang Jan 19, 2022

Choose a reason for hiding this comment

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

4611bbd fix this

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jan 19, 2022

I was planning to add these in subsequent PR)s) which includes implementation of the remaining encodings for Parquet V2.
But since this is done, and supersedes my PR, let's merge this one and I can close my PR (or rename it to just address the review comments for the original delta binary PR).

thanks ~ @parthchandra

@sunchao sunchao closed this in c13d019 Jan 19, 2022
@sunchao
Copy link
Member

sunchao commented Jan 19, 2022

Thanks @LuciferYang , merged to master!

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jan 20, 2022

thanks all

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jan 20, 2022

+1, late LGTM. Thank you all!

@LuciferYang LuciferYang deleted the SPARK-37928 branch October 22, 2023 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunchao sunchao sunchao approved these changes

Assignees

No one assigned

Labels

SQL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LuciferYang @parthchandra @sunchao @dongjoon-hyun