Copilot commented on code in PR #11712:
URL: https://github.com/apache/gluten/pull/11712#discussion_r3480373496
##########
cpp/velox/utils/VeloxWriterUtils.h:
##########
@@ -23,7 +23,7 @@
namespace gluten {
-std::unique_ptr<facebook::velox::parquet::WriterOptions>
makeParquetWriteOption(
+std::unique_ptr<facebook::velox::dwio::common::WriterOptions>
makeParquetWriteOption(
const std::unordered_map<std::string, std::string>& sparkConfs);
Review Comment:
The declaration of makeParquetWriteOption now returns
dwio::common::WriterOptions, but the definition and call sites in the repo
still use parquet::WriterOptions (e.g., utils/VeloxWriterUtils.cc:41 returns
WriterOptions from the parquet namespace; substrait/SubstraitToVeloxPlan.cc:881
stores the result in shared_ptr<parquet::WriterOptions>). This will fail to
compile and also mismatches makeHiveInsertTableHandle(), which expects
shared_ptr<dwio::common::WriterOptions> (SubstraitToVeloxPlan.cc:714). Please
update the function definition and all callers to use a single consistent
WriterOptions type (likely dwio::common::WriterOptions for the newer Velox
writer API), and align ownership semantics (unique_ptr vs shared_ptr) across
the call sites.
--
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]