Skip to content
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.

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

Extract common utils from ColumnReader and create DwrfColumnReader #1678

Conversation

liushengxuan
Copy link
Contributor

@liushengxuan liushengxuan commented May 23, 2022

This PR resolves the first part of issue #1620.

Previously, ColumnReader and the derived XXXColumnReader were Dwrf specific. However, some utils can also be applied to Parquet and ColumnReader should only keep the common features. So this PR decouples the DwrfColumnReader from ColumnReader and adjusted the other class structures accordiingly.

  1. ColumnReader is moved to dwio::common::reader as the common util for both Dwrf and Parquet
  2. The previous ColumnReader is replaced by DwrfColumnReader, which still remains at the same directory and inherits ColumnReader.
  3. All the derived XXXColumnReader are renamed to XXXDwrfColumnReader and inheriting DwrfColumnReader.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 23, 2022
@liushengxuan liushengxuan force-pushed the shengxuan/interface_refactor branch 2 times, most recently from 074e897 to 86347e2 Compare May 24, 2022 04:49
@liushengxuan liushengxuan changed the title WIP: Shengxuan/interface refactor Extract ColumnReader Jun 6, 2022
@liushengxuan liushengxuan changed the title Extract ColumnReader Extract common utils from ColumnReader and create DwrfColumnReader Jun 6, 2022
@stale
Copy link

stale bot commented Sep 6, 2022

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!

@stale stale bot added the stale label Sep 6, 2022
@stale stale bot closed this Sep 20, 2022
marin-ma added a commit to marin-ma/velox-oap that referenced this pull request Dec 15, 2023
Add shuffle benchmark to velox shuffle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants