Skip to content

[feature-wip](parquet-reader) parquert scanner can read data#11970

Merged
dataroaring merged 1 commit intoapache:masterfrom
wsjz:parquet_scanner_pass
Aug 26, 2022
Merged

[feature-wip](parquet-reader) parquert scanner can read data#11970
dataroaring merged 1 commit intoapache:masterfrom
wsjz:parquet_scanner_pass

Conversation

@wsjz
Copy link
Contributor

@wsjz wsjz commented Aug 22, 2022

Proposed changes

Issue Number: close #xxx

Problem summary

finished first version of the new parquet scanner and reader, this version just support primary type

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@wsjz wsjz marked this pull request as ready for review August 24, 2022 02:22
@cambyzju
Copy link
Contributor

please add some description about what this pr doing.

@wsjz
Copy link
Contributor Author

wsjz commented Aug 24, 2022

please add some description about what this pr doing.

use new parquet reader instead of old arrow reader WIP

: HdfsFileScanner(state, profile, params, ranges, pre_filter_texprs, counter) {}

ParquetFileHdfsScanner::~ParquetFileHdfsScanner() {
ParquetFileHdfsScanner::close();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should move this call to FileScanNode when this FileScanner is ready to close.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think FileScanNode in this version we can keep the same with before

@Gabriel39
Copy link
Contributor

Hi @wsjz I have fixed a failure regression case and you need rebase master and push again to resolve this.

@wsjz
Copy link
Contributor Author

wsjz commented Aug 24, 2022

Hi @wsjz I have fixed a failure regression case and you need rebase master and push again to resolve this.

OK

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 25, 2022
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@dataroaring dataroaring merged commit 0c16740 into apache:master Aug 26, 2022
GoGoWen pushed a commit to GoGoWen/incubator-doris that referenced this pull request Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. area/multi-catalog area/vectorization reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants