Fix COPY; rework Reader op #1963
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1963 +/- ##
=========================================
Coverage ? 89.74%
=========================================
Files ? 868
Lines ? 32027
Branches ? 0
=========================================
Hits ? 28743
Misses ? 3284
Partials ? 0
☔ View full report in Codecov by Sentry. |
| ColumnChunkMetadata() = default; | ||
| ColumnChunkMetadata(common::page_idx_t pageIdx, common::page_idx_t numPages) | ||
| struct BaseColumnChunkMetadata { | ||
| BaseColumnChunkMetadata() : pageIdx{common::INVALID_PAGE_IDX}, numPages{0} {} |
There was a problem hiding this comment.
I think we always put constructor and deconstructor below the fields for structs.
There was a problem hiding this comment.
hmm ok. Will make the change.
Was this style coming from some standard? I wasn't aware of that. why do we have to make struct so different from class?
| return false; | ||
| } | ||
| for (auto i = 0u; i < readerInfo.dataColumnPoses.size(); i++) { | ||
| ArrowColumnVector::setArrowColumn( |
There was a problem hiding this comment.
I think it is better to store the readerVectors in the Reader operators, so you don't need to dynamically call getValueVector() during reading.
resultSet->getValueVector(readerInfo.dataColumnPoses[i]).get()
There was a problem hiding this comment.
I'm changing this as well. Let me do this.
There was a problem hiding this comment.
OK. I will leave this to @andyfengHKU .
But i think it's better that we just store either column pos or value vector, as these two are redundant. I would prefer storing column pos, and dynamically fetch the value vector. We can discuss a bit more.
ray6080
force-pushed
the
morsel
branch
2 times, most recently
from
42bf4ec
to
c222939
Compare
No description provided.