alamb commented on a change in pull request #1395:
URL: https://github.com/apache/arrow-datafusion/pull/1395#discussion_r761958621
##########
File path: datafusion/src/physical_plan/sort_preserving_merge.rs
##########
@@ -346,7 +346,7 @@ struct SortPreservingMergeStream {
receivers: Vec<mpsc::Receiver<ArrowResult<RecordBatch>>>,
/// Drop helper for tasks feeding the [`receivers`](Self::receivers)
- drop_helper: AbortOnDropMany<()>,
+ _drop_helper: AbortOnDropMany<()>,
Review comment:
this is used (its `drop()` impl is used) so change its name
##########
File path: datafusion/src/physical_plan/windows/built_in.rs
##########
@@ -33,28 +31,22 @@ use std::sync::Arc;
/// A window expr that takes the form of a built in window function
#[derive(Debug)]
pub struct BuiltInWindowExpr {
- fun: BuiltInWindowFunction,
expr: Arc<dyn BuiltInWindowFunctionExpr>,
partition_by: Vec<Arc<dyn PhysicalExpr>>,
order_by: Vec<PhysicalSortExpr>,
- window_frame: Option<WindowFrame>,
}
impl BuiltInWindowExpr {
/// create a new built-in window function expression
pub(super) fn new(
- fun: BuiltInWindowFunction,
Review comment:
the fun and window_frame are used to construct the
`BuiltInWindowFunctionExpr` (so the don't need to be also encoded on the
`BuildInWindowExpr`
##########
File path: benchmarks/src/bin/tpch.rs
##########
@@ -75,10 +75,9 @@ struct BallistaBenchmarkOpt {
#[structopt(short = "i", long = "iterations", default_value = "3")]
iterations: usize,
- /// Batch size when reading CSV or Parquet files
- #[structopt(short = "s", long = "batch-size", default_value = "8192")]
- batch_size: usize,
-
+ // /// Batch size when reading CSV or Parquet files
Review comment:
These parameters are never read (aka they don't do anything) so removing
them from the CLI seemed like a reasonable thing to do
##########
File path: ballista/rust/core/src/config.rs
##########
@@ -31,22 +31,22 @@ pub const BALLISTA_DEFAULT_SHUFFLE_PARTITIONS: &str =
"ballista.shuffle.partitio
#[derive(Debug, Clone)]
pub struct ConfigEntry {
name: String,
- description: String,
- data_type: DataType,
+ _description: String,
Review comment:
I didn't know if this was important or not to keep in ballista so rather
than removing it I renamed them to start with `_` to satisfy clippy
--
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]
