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]

Reply via email to