-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add empty columns to chunked node group if needed during COPY #4882
Conversation
c2c35b9
to
043ef90
Compare
Benchmark ResultMaster commit hash:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4882 +/- ##
=======================================
Coverage 86.50% 86.51%
=======================================
Files 1403 1403
Lines 60629 60661 +32
Branches 7460 7461 +1
=======================================
+ Hits 52447 52480 +33
+ Misses 8013 8012 -1
Partials 169 169 ☔ View full report in Codecov by Sentry. |
Benchmark ResultMaster commit hash:
|
6ab993c
to
eb895fb
Compare
Benchmark ResultMaster commit hash:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks!
|
||
for (column_id_t i = 0; i < columnTypes.size(); ++i) { | ||
if (chunks[i] == nullptr) { | ||
chunks[i] = std::make_unique<ColumnChunk>(mm, columnTypes[i].copy(), capacity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not set capacity to 0 here for padding chunks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
78c925e
to
eba79fa
Compare
Benchmark ResultMaster commit hash:
|
Description
Follow-up to #4786
After dropping a column (and before checkpointing), if we are directly appending newly-flushed chunked groups to a node group (during copy) the newly inserted node group may not have as many columns as the parent node group, which would cause the next checkpoint to fail.
This PR adds empty columns to the newly inserted node group so that its number of columns is as expected
Also fixes another bug where warning data was being flushed to disk for rel batch insert.
Contributor agreement