ReemaAlzaid commented on code in PR #13067:
URL: https://github.com/apache/gluten/pull/13067#discussion_r4219906455
##########
cpp/velox/operators/plannodes/CudfVectorStream.h:
##########
@@ -115,11 +116,30 @@ class CudfVectorStream : public CudfVectorStreamBase {
VELOX_DCHECK(vp != nullptr);
auto cudfVector =
std::dynamic_pointer_cast<facebook::velox::cudf_velox::CudfVector>(vp);
if (cudfVector == nullptr) {
- // The vector may comes from BroadcastExchange, in this case, it's not a
CudfVector.
- vp->setType(outputType_);
- return vp;
+ // BroadcastExchange may return a host RowVector; upload it for GPU
operators.
+ auto stream =
facebook::velox::cudf_velox::cudfGlobalStreamPool().get_stream();
+ if (vp->childrenSize() == 0 || outputType_->size() == 0) {
+ // Preserve row count because zero-column cuDF tables cannot store it.
+ return std::make_shared<facebook::velox::cudf_velox::CudfVector>(
+ vp->pool(), outputType_, vp->size(),
std::make_unique<cudf::table>(), stream);
+ }
+ // Drop extra trailing columns added by broadcast exchange.
Review Comment:
Thanks, I checked this. With an expression key broadcast join and that
option enabled, I still saw `col_0` added with cuDF. The CPU hash-table build
is skipped, but the projection still runs.
The query passed without trimming too, so I haven’t reproduced a failure
caused by that column. The q3/q10/q18 failures were fixed by uploading the host
batches. The trimming is defensive schema handling, and I’ll make that clearer
in the comment
--
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]
