Copilot commented on code in PR #11712:
URL: https://github.com/apache/gluten/pull/11712#discussion_r3480121204
##########
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:
`makeParquetWriteOption` now advertises
`facebook::velox::dwio::common::WriterOptions`, but there are still in-tree
uses/assumptions of `facebook::velox::parquet::WriterOptions` (e.g.
`SubstraitToVeloxPlan.cc` assigns the return value to a
`std::shared_ptr<facebook::velox::parquet::WriterOptions>`). Depending on the
Velox version, this can become a hard compile error and is also inconsistent
API usage. Please update the remaining call sites and the implementation to
consistently use the new options type (or provide a typedef/using that keeps
the public signature stable).
##########
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(
Review Comment:
This header is not self-contained: it uses `std::unique_ptr` and
`std::unordered_map` but does not include `<memory>` / `<unordered_map>`.
Relying on transitive includes from Velox headers is brittle and can break with
upstream header refactors (especially during Velox rebases).
--
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]
