Copilot commented on code in PR #12810:
URL: https://github.com/apache/gluten/pull/12810#discussion_r3806174904
##########
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:
This loop previously mapped `col` to the underlying column index via
`simpleColumnIndices_[col]` (used to look up `arrowColumnTypes_[colIdx]`). The
new call passes `col` directly, which can change which column/type is sized if
`valueBufferSizeForFixedWidthArray` expects the original column index (the one
used for type lookup / buffer allocation). If
`valueBufferSizeForFixedWidthArray` is keyed by the underlying column index,
pass `simpleColumnIndices_[col]` instead; if it’s keyed by the fixed-width
ordinal, consider renaming the parameter/API to make that contract explicit to
avoid silent mis-sizing.
--
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]
