Skip to content

Commit e86f8e9

Browse files
author
QP Hou
authored
use Into<String> as argument type wherever applicable (#615)
* use Into<String> as argument type wherever applicable * switch from Into<String> to AsRef<str> for write_csv and write_parquet
1 parent 4068f8b commit e86f8e9

File tree

17 files changed

+97
-65
lines changed

17 files changed

+97
-65
lines changed

‎datafusion/src/datasource/csv.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,12 @@ pub struct CsvFile {
5959

6060
impl CsvFile {
6161
/// Attempt to initialize a new `CsvFile` from a file path
62-
pub fn try_new(path: &str, options: CsvReadOptions) -> Result<Self> {
62+
pub fn try_new(path: impl Into<String>, options: CsvReadOptions) -> Result<Self> {
63+
let path = path.into();
6364
let schema = Arc::new(match options.schema {
6465
Some(s) => s.clone(),
6566
None => {
66-
let filenames = common::build_file_list(path, options.file_extension)?;
67+
let filenames = common::build_file_list(&path, options.file_extension)?;
6768
if filenames.is_empty() {
6869
return Err(DataFusionError::Plan(format!(
6970
"No files found at {path} with file extension {file_extension}",
@@ -76,7 +77,7 @@ impl CsvFile {
7677
});
7778

7879
Ok(Self {
79-
source: Source::Path(path.to_string()),
80+
source: Source::Path(path),
8081
schema,
8182
has_header: options.has_header,
8283
delimiter: options.delimiter,

‎datafusion/src/datasource/parquet.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@ pub struct ParquetTable {
4242

4343
impl ParquetTable {
4444
/// Attempt to initialize a new `ParquetTable` from a file path.
45-
pub fn try_new(path: &str, max_concurrency: usize) -> Result<Self> {
46-
let parquet_exec = ParquetExec::try_from_path(path, None, None, 0, 1, None)?;
45+
pub fn try_new(path: impl Into<String>, max_concurrency: usize) -> Result<Self> {
46+
let path = path.into();
47+
let parquet_exec = ParquetExec::try_from_path(&path, None, None, 0, 1, None)?;
4748
let schema = parquet_exec.schema();
4849
Ok(Self {
49-
path: path.to_string(),
50+
path,
5051
schema,
5152
statistics: parquet_exec.statistics().to_owned(),
5253
max_concurrency,

‎datafusion/src/execution/context.rs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ impl ExecutionContext {
270270
/// Creates a DataFrame for reading a CSV data source.
271271
pub fn read_csv(
272272
&mut self,
273-
filename: &str,
273+
filename: impl Into<String>,
274274
options: CsvReadOptions,
275275
) -> Result<Arc<dyn DataFrame>> {
276276
Ok(Arc::new(DataFrameImpl::new(
@@ -280,7 +280,10 @@ impl ExecutionContext {
280280
}
281281

282282
/// Creates a DataFrame for reading a Parquet data source.
283-
pub fn read_parquet(&mut self, filename: &str) -> Result<Arc<dyn DataFrame>> {
283+
pub fn read_parquet(
284+
&mut self,
285+
filename: impl Into<String>,
286+
) -> Result<Arc<dyn DataFrame>> {
284287
Ok(Arc::new(DataFrameImpl::new(
285288
self.state.clone(),
286289
&LogicalPlanBuilder::scan_parquet(
@@ -474,10 +477,11 @@ impl ExecutionContext {
474477
pub async fn write_csv(
475478
&self,
476479
plan: Arc<dyn ExecutionPlan>,
477-
path: String,
480+
path: impl AsRef<str>,
478481
) -> Result<()> {
482+
let path = path.as_ref();
479483
// create directory to contain the CSV files (one per partition)
480-
let fs_path = Path::new(&path);
484+
let fs_path = Path::new(path);
481485
match fs::create_dir(fs_path) {
482486
Ok(()) => {
483487
let mut tasks = vec![];
@@ -511,11 +515,12 @@ impl ExecutionContext {
511515
pub async fn write_parquet(
512516
&self,
513517
plan: Arc<dyn ExecutionPlan>,
514-
path: String,
518+
path: impl AsRef<str>,
515519
writer_properties: Option<WriterProperties>,
516520
) -> Result<()> {
521+
let path = path.as_ref();
517522
// create directory to contain the Parquet files (one per partition)
518-
let fs_path = Path::new(&path);
523+
let fs_path = Path::new(path);
519524
match fs::create_dir(fs_path) {
520525
Ok(()) => {
521526
let mut tasks = vec![];

‎datafusion/src/logical_plan/builder.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,39 +115,41 @@ impl LogicalPlanBuilder {
115115

116116
/// Scan a CSV data source
117117
pub fn scan_csv(
118-
path: &str,
118+
path: impl Into<String>,
119119
options: CsvReadOptions,
120120
projection: Option<Vec<usize>>,
121121
) -> Result<Self> {
122-
Self::scan_csv_with_name(path, options, projection, path)
122+
let path = path.into();
123+
Self::scan_csv_with_name(path.clone(), options, projection, path)
123124
}
124125

125126
/// Scan a CSV data source and register it with a given table name
126127
pub fn scan_csv_with_name(
127-
path: &str,
128+
path: impl Into<String>,
128129
options: CsvReadOptions,
129130
projection: Option<Vec<usize>>,
130-
table_name: &str,
131+
table_name: impl Into<String>,
131132
) -> Result<Self> {
132133
let provider = Arc::new(CsvFile::try_new(path, options)?);
133134
Self::scan(table_name, provider, projection)
134135
}
135136

136137
/// Scan a Parquet data source
137138
pub fn scan_parquet(
138-
path: &str,
139+
path: impl Into<String>,
139140
projection: Option<Vec<usize>>,
140141
max_concurrency: usize,
141142
) -> Result<Self> {
142-
Self::scan_parquet_with_name(path, projection, max_concurrency, path)
143+
let path = path.into();
144+
Self::scan_parquet_with_name(path.clone(), projection, max_concurrency, path)
143145
}
144146

145147
/// Scan a Parquet data source and register it with a given table name
146148
pub fn scan_parquet_with_name(
147-
path: &str,
149+
path: impl Into<String>,
148150
projection: Option<Vec<usize>>,
149151
max_concurrency: usize,
150-
table_name: &str,
152+
table_name: impl Into<String>,
151153
) -> Result<Self> {
152154
let provider = Arc::new(ParquetTable::try_new(path, max_concurrency)?);
153155
Self::scan(table_name, provider, projection)
@@ -166,10 +168,12 @@ impl LogicalPlanBuilder {
166168

167169
/// Convert a table provider into a builder with a TableScan
168170
pub fn scan(
169-
table_name: &str,
171+
table_name: impl Into<String>,
170172
provider: Arc<dyn TableProvider>,
171173
projection: Option<Vec<usize>>,
172174
) -> Result<Self> {
175+
let table_name = table_name.into();
176+
173177
if table_name.is_empty() {
174178
return Err(DataFusionError::Plan(
175179
"table_name cannot be empty".to_string(),
@@ -184,17 +188,17 @@ impl LogicalPlanBuilder {
184188
DFSchema::new(
185189
p.iter()
186190
.map(|i| {
187-
DFField::from_qualified(table_name, schema.field(*i).clone())
191+
DFField::from_qualified(&table_name, schema.field(*i).clone())
188192
})
189193
.collect(),
190194
)
191195
})
192196
.unwrap_or_else(|| {
193-
DFSchema::try_from_qualified_schema(table_name, &schema)
197+
DFSchema::try_from_qualified_schema(&table_name, &schema)
194198
})?;
195199

196200
let table_scan = LogicalPlan::TableScan {
197-
table_name: table_name.to_string(),
201+
table_name,
198202
source: provider,
199203
projected_schema: Arc::new(projected_schema),
200204
projection,

‎datafusion/src/logical_plan/expr.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ pub struct Column {
4444

4545
impl Column {
4646
/// Create Column from unqualified name.
47-
pub fn from_name(name: String) -> Self {
47+
pub fn from_name(name: impl Into<String>) -> Self {
4848
Self {
4949
relation: None,
50-
name,
50+
name: name.into(),
5151
}
5252
}
5353

@@ -131,7 +131,7 @@ impl fmt::Display for Column {
131131
/// ```
132132
/// # use datafusion::logical_plan::*;
133133
/// let expr = col("c1");
134-
/// assert_eq!(expr, Expr::Column(Column::from_name("c1".to_string())));
134+
/// assert_eq!(expr, Expr::Column(Column::from_name("c1")));
135135
/// ```
136136
///
137137
/// ## Create the expression `c1 + c2` to add columns "c1" and "c2" together

‎datafusion/src/optimizer/filter_push_down.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,8 @@ mod tests {
890890
.join(
891891
&right,
892892
JoinType::Inner,
893-
vec![Column::from_name("a".to_string())],
894-
vec![Column::from_name("a".to_string())],
893+
vec![Column::from_name("a")],
894+
vec![Column::from_name("a")],
895895
)?
896896
.filter(col("a").lt_eq(lit(1i64)))?
897897
.build()?;
@@ -933,8 +933,8 @@ mod tests {
933933
.join(
934934
&right,
935935
JoinType::Inner,
936-
vec![Column::from_name("a".to_string())],
937-
vec![Column::from_name("a".to_string())],
936+
vec![Column::from_name("a")],
937+
vec![Column::from_name("a")],
938938
)?
939939
// "b" and "c" are not shared by either side: they are only available together after the join
940940
.filter(col("c").lt_eq(col("b")))?
@@ -972,8 +972,8 @@ mod tests {
972972
.join(
973973
&right,
974974
JoinType::Inner,
975-
vec![Column::from_name("a".to_string())],
976-
vec![Column::from_name("a".to_string())],
975+
vec![Column::from_name("a")],
976+
vec![Column::from_name("a")],
977977
)?
978978
.filter(col("b").lt_eq(lit(1i64)))?
979979
.build()?;

‎datafusion/src/optimizer/projection_push_down.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ fn optimize_plan(
241241
{
242242
window_expr.iter().try_for_each(|expr| {
243243
let name = &expr.name(schema)?;
244-
let column = Column::from_name(name.to_string());
244+
let column = Column::from_name(name);
245245
if required_columns.contains(&column) {
246246
new_window_expr.push(expr.clone());
247247
new_required_columns.insert(column);
@@ -286,7 +286,7 @@ fn optimize_plan(
286286
let mut new_aggr_expr = Vec::new();
287287
aggr_expr.iter().try_for_each(|expr| {
288288
let name = &expr.name(schema)?;
289-
let column = Column::from_name(name.to_string());
289+
let column = Column::from_name(name);
290290

291291
if required_columns.contains(&column) {
292292
new_aggr_expr.push(expr.clone());

‎datafusion/src/optimizer/utils.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ mod tests {
516516
&mut accum,
517517
)?;
518518
assert_eq!(1, accum.len());
519-
assert!(accum.contains(&Column::from_name("a".to_string())));
519+
assert!(accum.contains(&Column::from_name("a")));
520520
Ok(())
521521
}
522522

‎datafusion/src/physical_plan/aggregates.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ pub fn create_aggregate_expr(
110110
distinct: bool,
111111
args: &[Arc<dyn PhysicalExpr>],
112112
input_schema: &Schema,
113-
name: String,
113+
name: impl Into<String>,
114114
) -> Result<Arc<dyn AggregateExpr>> {
115-
// coerce
115+
let name = name.into();
116116
let arg = coerce(args, input_schema, &signature(fun))?;
117117
if arg.is_empty() {
118118
return Err(DataFusionError::Plan(format!(

‎datafusion/src/physical_plan/expressions/average.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,13 @@ pub fn avg_return_type(arg_type: &DataType) -> Result<DataType> {
6464

6565
impl Avg {
6666
/// Create a new AVG aggregate function
67-
pub fn new(expr: Arc<dyn PhysicalExpr>, name: String, data_type: DataType) -> Self {
67+
pub fn new(
68+
expr: Arc<dyn PhysicalExpr>,
69+
name: impl Into<String>,
70+
data_type: DataType,
71+
) -> Self {
6872
Self {
69-
name,
73+
name: name.into(),
7074
expr,
7175
data_type,
7276
nullable: true,

0 commit comments

Comments
 (0)