Copilot commented on code in PR #23781:
URL: https://github.com/apache/datafusion/pull/23781#discussion_r3730283343
##########
datafusion/datasource/src/sink.rs:
##########
@@ -24,7 +24,7 @@ use std::sync::Arc;
use arrow::array::{ArrayRef, RecordBatch, UInt64Array};
use arrow::datatypes::{DataType, Field, Schema, SchemaRef};
-use datafusion_common::{Result, assert_eq_or_internal_err};
+use datafusion_common::{Result, assert_eq_or_internal_err,
internal_datafusion_err};
Review Comment:
`internal_datafusion_err` is only used inside `#[cfg(feature = "proto")]`
code (`decode_sort_order`). When `proto` is disabled, this import becomes
unused and can trigger warnings in feature-minimal builds. Consider gating the
import or qualifying the macro at the call site.
##########
datafusion/proto/src/physical_plan/mod.rs:
##########
@@ -782,15 +781,19 @@ pub trait PhysicalPlanNodeExt: Sized {
PhysicalPlanType::Analyze(_) => {
AnalyzeExec::try_from_proto(self.node(), &decode_ctx)
}
- PhysicalPlanType::JsonSink(sink) => {
- self.try_into_json_sink_physical_plan(sink, ctx,
proto_converter)
+ PhysicalPlanType::JsonSink(_) => {
+ JsonSink::try_from_proto(self.node(), &decode_ctx)
}
- PhysicalPlanType::CsvSink(sink) => {
- self.try_into_csv_sink_physical_plan(sink, ctx,
proto_converter)
+ PhysicalPlanType::CsvSink(_) => {
+ CsvSink::try_from_proto(self.node(), &decode_ctx)
}
- #[cfg_attr(not(feature = "parquet"), allow(unused_variables))]
- PhysicalPlanType::ParquetSink(sink) => {
- self.try_into_parquet_sink_physical_plan(sink, ctx,
proto_converter)
+ PhysicalPlanType::ParquetSink(_) => {
+ #[cfg(feature = "parquet")]
+ {
+ ParquetSink::try_from_proto(self.node(), &decode_ctx)
+ }
+ #[cfg(not(feature = "parquet"))]
+ panic!("Trying to use ParquetSink without `parquet` feature
enabled")
Review Comment:
Decoding a Parquet sink currently panics when the `parquet` feature is
disabled. Since protobuf decoding can be fed untrusted input, this should
return a structured `DataFusionError` (e.g. `NotImplemented`) rather than
aborting the process.
--
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]
