Copilot commented on code in PR #12219:
URL: https://github.com/apache/gluten/pull/12219#discussion_r3381660970
##########
cpp/velox/compute/iceberg/IcebergWriter.cc:
##########
@@ -229,7 +236,9 @@ IcebergWriter::IcebergWriter(
pool_.get()),
connectorQueryCtx_.get(),
facebook::velox::connector::CommitStrategy::kNoCommit,
- connectorConfig_);
+ connectorConfig_,
+ icebergConfig);
+ dataSink_.get();
Review Comment:
`dataSink_.get();` is a no-op expression statement (has no side effects) and
can trigger `-Wunused-value`/`-Wextra` warnings. It should be removed.
##########
cpp/velox/compute/iceberg/IcebergWriter.cc:
##########
@@ -238,10 +247,13 @@ void IcebergWriter::write(const VeloxColumnarBatch&
batch) {
if (inputRowType->size() != rowType_->size()) {
const auto& children = inputRowVector->children();
+
+ VELOX_CHECK_GE(children.size(), 1 + rowType_->size());
+
std::vector<VectorPtr> dataColumns(children.begin() + 1, children.begin()
+ 1 + rowType_->size());
- auto filteredRowVector = std::make_shared<RowVector>(
- pool_.get(), rowType_, inputRowVector->nulls(),
inputRowVector->size(), std::move(dataColumns));
+ auto filteredRowVector =
+ std::make_shared<RowVector>(pool_.get(), rowType_, nullptr,
inputRowVector->size(), std::move(dataColumns));
Review Comment:
When constructing `filteredRowVector`, the row-level nulls buffer from
`inputRowVector` is dropped (`nullptr`). If the input has any row-level nulls,
this changes semantics and can write non-null rows where the whole row should
be null. Preserve the original nulls buffer when slicing the children.
--
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]
