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

Implement parquet-reader #2076

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Implement parquet-reader #2076

merged 1 commit into from
Sep 25, 2023

Conversation

acquamarin
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Attention: 385 lines in your changes are missing coverage. Please review.

Comparison is base (da22d9e) 90.21% compared to head (b43a63c) 89.00%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2076      +/-   ##
==========================================
- Coverage   90.21%   89.00%   -1.22%     
==========================================
  Files         966      987      +21     
  Lines       35019    36057    +1038     
==========================================
+ Hits        31593    32092     +499     
- Misses       3426     3965     +539     
Files Coverage Δ
src/binder/bind/bind_copy.cpp 91.39% <100.00%> (-0.67%) ⬇️
src/binder/bind/bind_reading_clause.cpp 94.36% <ø> (ø)
src/common/vector/value_vector.cpp 95.39% <100.00%> (+0.13%) ⬆️
src/include/common/types/ku_string.h 100.00% <100.00%> (ø)
src/include/common/types/types.h 100.00% <ø> (ø)
src/include/common/vector/value_vector.h 100.00% <ø> (ø)
src/include/processor/operator/persistent/reader.h 100.00% <ø> (ø)
...cessor/operator/persistent/reader/csv/csv_reader.h 100.00% <ø> (ø)
...persistent/reader/parquet/callback_column_reader.h 100.00% <100.00%> (ø)
...perator/persistent/reader/parquet/parquet_reader.h 100.00% <100.00%> (ø)
... and 22 more

... and 40 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ray6080 ray6080 left a comment

Choose a reason for hiding this comment

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

👍 I didn't really get into details for this PR. Only peeked into parts our ValueVector gets used. It's hard to figure out the whole logic right away, but I feel we can trust a bit on duck's code path.

One high level comment: we should try to improve the codecov for this PR by two options: 1) guard code path not enabled at all with LCOV_EXEC_START and LCOV_EXEC_STOP, for example: prefetch mode; 2) see if there are easy cases we can add tests, maybe the following code block?



if (thisOutputChunkRows == 0) {1
--
+         state.finished = true;
+         return false; // end of last group, we are done
+     }


third_party/miniparquet/.Rbuildignore Outdated Show resolved Hide resolved
third_party/miniparquet/.travis.yml Outdated Show resolved Hide resolved
third_party/miniparquet/LICENSE Outdated Show resolved Hide resolved
test/runner/e2e_ddl_test.cpp Outdated Show resolved Hide resolved
@acquamarin acquamarin merged commit 4628776 into master Sep 25, 2023
10 of 11 checks passed
@acquamarin acquamarin deleted the parquet-reader branch September 25, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants