-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update rust vesion to 1.57 #1395
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ homepage = "https://github.com/apache/arrow-datafusion" | |
repository = "https://github.com/apache/arrow-datafusion" | ||
authors = ["Apache Arrow <[email protected]>"] | ||
edition = "2021" | ||
rust-version = "1.56" | ||
rust-version = "1.57" | ||
|
||
[dependencies] | ||
ballista-core = { path = "../core", version = "0.6.0" } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These parameters are never read (aka they don't do anything) so removing them from the CLI seemed like a reasonable thing to do |
||
// #[structopt(short = "s", long = "batch-size", default_value = "8192")] | ||
// batch_size: usize, | ||
/// Path to data files | ||
#[structopt(parse(from_os_str), required = true, short = "p", long = "path")] | ||
path: PathBuf, | ||
|
@@ -87,10 +86,9 @@ struct BallistaBenchmarkOpt { | |
#[structopt(short = "f", long = "format", default_value = "csv")] | ||
file_format: String, | ||
|
||
/// Load the data into a MemTable before executing the query | ||
#[structopt(short = "m", long = "mem-table")] | ||
mem_table: bool, | ||
|
||
// /// Load the data into a MemTable before executing the query | ||
// #[structopt(short = "m", long = "mem-table")] | ||
// mem_table: bool, | ||
/// Number of partitions to process in parallel | ||
#[structopt(short = "p", long = "partitions", default_value = "2")] | ||
partitions: usize, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,7 @@ use arrow::{ | |
}; | ||
use chrono::prelude::*; | ||
use chrono::Duration; | ||
use std::borrow::Borrow; | ||
|
||
/// given a function `op` that maps a `&str` to a Result of an arrow native type, | ||
/// returns a `PrimitiveArray` after the application | ||
|
@@ -77,7 +78,10 @@ where | |
})?; | ||
|
||
// first map is the iterator, second is for the `Option<_>` | ||
array.iter().map(|x| x.map(|x| op(x)).transpose()).collect() | ||
array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
.iter() | ||
.map(|x| x.map(op.borrow()).transpose()) | ||
.collect() | ||
} | ||
|
||
// given an function that maps a `&str` to a arrow native type, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,8 +37,6 @@ use super::{format_state_name, sum}; | |
#[derive(Debug)] | ||
pub struct Avg { | ||
name: String, | ||
data_type: DataType, | ||
nullable: bool, | ||
expr: Arc<dyn PhysicalExpr>, | ||
} | ||
|
||
|
@@ -69,11 +67,14 @@ impl Avg { | |
name: impl Into<String>, | ||
data_type: DataType, | ||
) -> Self { | ||
// Average is always Float64, but Avg::new() has a data_type | ||
// parameter to keep a consistent signature with the other | ||
// Aggregate expressions. | ||
assert_eq!(data_type, DataType::Float64); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
Self { | ||
name: name.into(), | ||
expr, | ||
data_type, | ||
nullable: true, | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<()>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is used (its |
||
|
||
/// For each input stream maintain a dequeue of SortKeyCursor | ||
/// | ||
|
@@ -379,7 +379,7 @@ struct SortPreservingMergeStream { | |
impl SortPreservingMergeStream { | ||
fn new( | ||
receivers: Vec<mpsc::Receiver<ArrowResult<RecordBatch>>>, | ||
drop_helper: AbortOnDropMany<()>, | ||
_drop_helper: AbortOnDropMany<()>, | ||
schema: SchemaRef, | ||
expressions: &[PhysicalSortExpr], | ||
target_batch_size: usize, | ||
|
@@ -394,7 +394,7 @@ impl SortPreservingMergeStream { | |
schema, | ||
cursors, | ||
receivers, | ||
drop_helper, | ||
_drop_helper, | ||
column_expressions: expressions.iter().map(|x| x.expr.clone()).collect(), | ||
sort_options: expressions.iter().map(|x| x.options).collect(), | ||
target_batch_size, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,8 @@ | |
//! Physical exec for built-in window function expressions. | ||
|
||
use crate::error::{DataFusionError, Result}; | ||
use crate::logical_plan::window_frames::WindowFrame; | ||
use crate::physical_plan::{ | ||
expressions::PhysicalSortExpr, | ||
window_functions::{BuiltInWindowFunction, BuiltInWindowFunctionExpr}, | ||
expressions::PhysicalSortExpr, window_functions::BuiltInWindowFunctionExpr, | ||
PhysicalExpr, WindowExpr, | ||
}; | ||
use arrow::compute::concat; | ||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the fun and window_frame are used to construct the |
||
expr: Arc<dyn BuiltInWindowFunctionExpr>, | ||
partition_by: &[Arc<dyn PhysicalExpr>], | ||
order_by: &[PhysicalSortExpr], | ||
window_frame: Option<WindowFrame>, | ||
) -> Self { | ||
Self { | ||
fun, | ||
expr, | ||
partition_by: partition_by.to_vec(), | ||
order_by: order_by.to_vec(), | ||
window_frame, | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,7 +68,7 @@ pub enum ScalarValue { | |
/// large binary | ||
LargeBinary(Option<Vec<u8>>), | ||
/// list of nested ScalarValue (boxed to reduce size_of(ScalarValue)) | ||
#[allow(clippy::box_vec)] | ||
#[allow(clippy::box_collection)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh clippy, how wrong you are :) the benefit is that the overall enum is smaller (as in the size of |
||
List(Option<Box<Vec<ScalarValue>>>, Box<DataType>), | ||
/// Date stored as a signed 32bit int | ||
Date32(Option<i32>), | ||
|
@@ -87,7 +87,7 @@ pub enum ScalarValue { | |
/// Interval with DayTime unit | ||
IntervalDayTime(Option<i64>), | ||
/// struct of nested ScalarValue (boxed to reduce size_of(ScalarValue)) | ||
#[allow(clippy::box_vec)] | ||
#[allow(clippy::box_collection)] | ||
Struct(Option<Box<Vec<ScalarValue>>>, Box<Vec<Field>>), | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,7 +85,7 @@ pub struct CreateExternalTable { | |
#[derive(Debug, Clone, PartialEq)] | ||
pub enum Statement { | ||
/// ANSI SQL AST node | ||
Statement(SQLStatement), | ||
Statement(Box<SQLStatement>), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
/// Extension: `CREATE EXTERNAL TABLE` | ||
CreateExternalTable(CreateExternalTable), | ||
} | ||
|
@@ -167,13 +167,17 @@ impl<'a> DFParser<'a> { | |
} | ||
_ => { | ||
// use the native parser | ||
Ok(Statement::Statement(self.parser.parse_statement()?)) | ||
Ok(Statement::Statement(Box::from( | ||
self.parser.parse_statement()?, | ||
))) | ||
} | ||
} | ||
} | ||
_ => { | ||
// use the native parser | ||
Ok(Statement::Statement(self.parser.parse_statement()?)) | ||
Ok(Statement::Statement(Box::from( | ||
self.parser.parse_statement()?, | ||
))) | ||
} | ||
} | ||
} | ||
|
@@ -183,7 +187,7 @@ impl<'a> DFParser<'a> { | |
if self.parser.parse_keyword(Keyword::EXTERNAL) { | ||
self.parse_create_external_table() | ||
} else { | ||
Ok(Statement::Statement(self.parser.parse_create()?)) | ||
Ok(Statement::Statement(Box::from(self.parser.parse_create()?))) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 clippyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, very much agree with you,
_
is a conservative but useful way!