Copilot commented on code in PR #12810:
URL: https://github.com/apache/gluten/pull/12810#discussion_r3803637507
##########
cpp/velox/shuffle/VeloxHashShuffleWriter.cc:
##########
@@ -893,9 +893,12 @@ inline bool
VeloxHashShuffleWriter::beyondThreshold(uint32_t partitionId, uint32
void VeloxHashShuffleWriter::calculateSimpleColumnBytes() {
fixedWidthBufferBytes_ = 0;
for (size_t col = 0; col < fixedWidthColumnCount_; ++col) {
- auto colIdx = simpleColumnIndices_[col];
- // `bool(1) >> 3` gets 0, so +7
- fixedWidthBufferBytes_ +=
((arrow::bit_width(arrowColumnTypes_[colIdx]->id()) + 7) >> 3);
+ // Reuse the same per-column sizing as the actual buffer allocation,
otherwise this estimate can
+ // drift from it: `arrow::bit_width` mis-counts the types whose Arrow bit
width differs from the
+ // width the partition buffer allocates, i.e. short decimal (allocated as
int64, 8 bytes not 16)
+ // and timestamp (allocated as int128, 16 bytes not 8). Note bool is still
rounded up to one byte
+ // per row.
+ fixedWidthBufferBytes_ +=
valueBufferSizeForFixedWidthArray(static_cast<uint32_t>(col), 1);
Review Comment:
`col` here is the loop index into `fixedWidthColumnCount_`, but previously
sizing was based on `colIdx = simpleColumnIndices_[col]` (the actual column
index in the schema). If `valueBufferSizeForFixedWidthArray` expects the real
column index (not the position within the fixed-width subset), this will size
the wrong type and produce incorrect buffer estimates. Use the same index as
before (i.e., pass `colIdx` derived from `simpleColumnIndices_[col]`) unless
`valueBufferSizeForFixedWidthArray` explicitly documents that it takes the
fixed-width-subset ordinal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
