Skip to content

Commit

Permalink
Minor: Use ScalarValue::from impl for strings (apache#8429)
Browse files Browse the repository at this point in the history
* Minor: Use ScalarValue::from impl for strings

* fix typo
  • Loading branch information
alamb authored and appletreeisyellow committed Dec 15, 2023
1 parent d995ae9 commit da6efaf
Show file tree
Hide file tree
Showing 28 changed files with 83 additions and 167 deletions.
2 changes: 1 addition & 1 deletion datafusion/common/src/pyarrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ mod tests {
ScalarValue::Boolean(Some(true)),
ScalarValue::Int32(Some(23)),
ScalarValue::Float64(Some(12.34)),
ScalarValue::Utf8(Some("Hello!".to_string())),
ScalarValue::from("Hello!"),
ScalarValue::Date32(Some(1234)),
];

Expand Down
25 changes: 11 additions & 14 deletions datafusion/common/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ impl ScalarValue {

/// Returns a [`ScalarValue::Utf8`] representing `val`
pub fn new_utf8(val: impl Into<String>) -> Self {
ScalarValue::Utf8(Some(val.into()))
ScalarValue::from(val.into())
}

/// Returns a [`ScalarValue::IntervalYearMonth`] representing
Expand Down Expand Up @@ -2699,7 +2699,7 @@ impl ScalarValue {

/// Try to parse `value` into a ScalarValue of type `target_type`
pub fn try_from_string(value: String, target_type: &DataType) -> Result<Self> {
let value = ScalarValue::Utf8(Some(value));
let value = ScalarValue::from(value);
let cast_options = CastOptions {
safe: false,
format_options: Default::default(),
Expand Down Expand Up @@ -3581,9 +3581,9 @@ mod tests {
#[test]
fn test_list_to_array_string() {
let scalars = vec![
ScalarValue::Utf8(Some(String::from("rust"))),
ScalarValue::Utf8(Some(String::from("arrow"))),
ScalarValue::Utf8(Some(String::from("data-fusion"))),
ScalarValue::from("rust"),
ScalarValue::from("arrow"),
ScalarValue::from("data-fusion"),
];

let array = ScalarValue::new_list(scalars.as_slice(), &DataType::Utf8);
Expand Down Expand Up @@ -4722,7 +4722,7 @@ mod tests {
Some(vec![
ScalarValue::Int32(Some(23)),
ScalarValue::Boolean(Some(false)),
ScalarValue::Utf8(Some("Hello".to_string())),
ScalarValue::from("Hello"),
ScalarValue::from(vec![
("e", ScalarValue::from(2i16)),
("f", ScalarValue::from(3i64)),
Expand Down Expand Up @@ -4915,17 +4915,17 @@ mod tests {

// Define struct scalars
let s0 = ScalarValue::from(vec![
("A", ScalarValue::Utf8(Some(String::from("First")))),
("A", ScalarValue::from("First")),
("primitive_list", l0),
]);

let s1 = ScalarValue::from(vec![
("A", ScalarValue::Utf8(Some(String::from("Second")))),
("A", ScalarValue::from("Second")),
("primitive_list", l1),
]);

let s2 = ScalarValue::from(vec![
("A", ScalarValue::Utf8(Some(String::from("Third")))),
("A", ScalarValue::from("Third")),
("primitive_list", l2),
]);

Expand Down Expand Up @@ -5212,7 +5212,7 @@ mod tests {
check_scalar_cast(ScalarValue::Float64(None), DataType::Int16);

check_scalar_cast(
ScalarValue::Utf8(Some("foo".to_string())),
ScalarValue::from("foo"),
DataType::Dictionary(Box::new(DataType::Int32), Box::new(DataType::Utf8)),
);

Expand Down Expand Up @@ -5493,10 +5493,7 @@ mod tests {
(ScalarValue::Int8(None), ScalarValue::Int16(Some(1))),
(ScalarValue::Int8(Some(1)), ScalarValue::Int16(None)),
// Unsupported types
(
ScalarValue::Utf8(Some("foo".to_string())),
ScalarValue::Utf8(Some("bar".to_string())),
),
(ScalarValue::from("foo"), ScalarValue::from("bar")),
(
ScalarValue::Boolean(Some(true)),
ScalarValue::Boolean(Some(false)),
Expand Down
20 changes: 4 additions & 16 deletions datafusion/core/src/datasource/listing/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,19 +526,13 @@ mod tests {
f1.object_meta.location.as_ref(),
"tablepath/mypartition=val1/file.parquet"
);
assert_eq!(
&f1.partition_values,
&[ScalarValue::Utf8(Some(String::from("val1"))),]
);
assert_eq!(&f1.partition_values, &[ScalarValue::from("val1")]);
let f2 = &pruned[1];
assert_eq!(
f2.object_meta.location.as_ref(),
"tablepath/mypartition=val1/other=val3/file.parquet"
);
assert_eq!(
f2.partition_values,
&[ScalarValue::Utf8(Some(String::from("val1"))),]
);
assert_eq!(f2.partition_values, &[ScalarValue::from("val1"),]);
}

#[tokio::test]
Expand Down Expand Up @@ -579,10 +573,7 @@ mod tests {
);
assert_eq!(
&f1.partition_values,
&[
ScalarValue::Utf8(Some(String::from("p1v2"))),
ScalarValue::Utf8(Some(String::from("p2v1")))
]
&[ScalarValue::from("p1v2"), ScalarValue::from("p2v1"),]
);
let f2 = &pruned[1];
assert_eq!(
Expand All @@ -591,10 +582,7 @@ mod tests {
);
assert_eq!(
&f2.partition_values,
&[
ScalarValue::Utf8(Some(String::from("p1v2"))),
ScalarValue::Utf8(Some(String::from("p2v1")))
]
&[ScalarValue::from("p1v2"), ScalarValue::from("p2v1")]
);
}

Expand Down
3 changes: 1 addition & 2 deletions datafusion/core/src/datasource/physical_plan/avro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,7 @@ mod tests {
.await?;

let mut partitioned_file = PartitionedFile::from(meta);
partitioned_file.partition_values =
vec![ScalarValue::Utf8(Some("2021-10-26".to_owned()))];
partitioned_file.partition_values = vec![ScalarValue::from("2021-10-26")];

let avro_exec = AvroExec::new(FileScanConfig {
// select specific columns of the files as well as the partitioning
Expand Down
3 changes: 1 addition & 2 deletions datafusion/core/src/datasource/physical_plan/csv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,7 @@ mod tests {

// Add partition columns
config.table_partition_cols = vec![Field::new("date", DataType::Utf8, false)];
config.file_groups[0][0].partition_values =
vec![ScalarValue::Utf8(Some("2021-10-26".to_owned()))];
config.file_groups[0][0].partition_values = vec![ScalarValue::from("2021-10-26")];

// We should be able to project on the partition column
// Which is supposed to be after the file fields
Expand Down
42 changes: 12 additions & 30 deletions datafusion/core/src/datasource/physical_plan/file_scan_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,15 +654,9 @@ mod tests {
// file_batch is ok here because we kept all the file cols in the projection
file_batch,
&[
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"2021".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"10".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"26".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::from("2021")),
wrap_partition_value_in_dict(ScalarValue::from("10")),
wrap_partition_value_in_dict(ScalarValue::from("26")),
],
)
.expect("Projection of partition columns into record batch failed");
Expand All @@ -688,15 +682,9 @@ mod tests {
// file_batch is ok here because we kept all the file cols in the projection
file_batch,
&[
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"2021".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"10".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"27".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::from("2021")),
wrap_partition_value_in_dict(ScalarValue::from("10")),
wrap_partition_value_in_dict(ScalarValue::from("27")),
],
)
.expect("Projection of partition columns into record batch failed");
Expand Down Expand Up @@ -724,15 +712,9 @@ mod tests {
// file_batch is ok here because we kept all the file cols in the projection
file_batch,
&[
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"2021".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"10".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::Utf8(Some(
"28".to_owned(),
))),
wrap_partition_value_in_dict(ScalarValue::from("2021")),
wrap_partition_value_in_dict(ScalarValue::from("10")),
wrap_partition_value_in_dict(ScalarValue::from("28")),
],
)
.expect("Projection of partition columns into record batch failed");
Expand All @@ -758,9 +740,9 @@ mod tests {
// file_batch is ok here because we kept all the file cols in the projection
file_batch,
&[
ScalarValue::Utf8(Some("2021".to_owned())),
ScalarValue::Utf8(Some("10".to_owned())),
ScalarValue::Utf8(Some("26".to_owned())),
ScalarValue::from("2021"),
ScalarValue::from("10"),
ScalarValue::from("26"),
],
)
.expect("Projection of partition columns into record batch failed");
Expand Down
4 changes: 2 additions & 2 deletions datafusion/core/src/datasource/physical_plan/parquet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1606,11 +1606,11 @@ mod tests {
let partitioned_file = PartitionedFile {
object_meta: meta,
partition_values: vec![
ScalarValue::Utf8(Some("2021".to_owned())),
ScalarValue::from("2021"),
ScalarValue::UInt8(Some(10)),
ScalarValue::Dictionary(
Box::new(DataType::UInt16),
Box::new(ScalarValue::Utf8(Some("26".to_owned()))),
Box::new(ScalarValue::from("26")),
),
],
range: None,
Expand Down
4 changes: 2 additions & 2 deletions datafusion/core/src/test/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl VarProvider for SystemVar {
/// get system variable value
fn get_value(&self, var_names: Vec<String>) -> Result<ScalarValue> {
let s = format!("{}-{}", "system-var", var_names.concat());
Ok(ScalarValue::Utf8(Some(s)))
Ok(ScalarValue::from(s))
}

fn get_type(&self, _: &[String]) -> Option<DataType> {
Expand All @@ -61,7 +61,7 @@ impl VarProvider for UserDefinedVar {
fn get_value(&self, var_names: Vec<String>) -> Result<ScalarValue> {
if var_names[0] != "@integer" {
let s = format!("{}-{}", "user-defined-var", var_names.concat());
Ok(ScalarValue::Utf8(Some(s)))
Ok(ScalarValue::from(s))
} else {
Ok(ScalarValue::Int32(Some(41)))
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/execution/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl SessionConfig {

/// Set a generic `str` configuration option
pub fn set_str(self, key: &str, value: &str) -> Self {
self.set(key, ScalarValue::Utf8(Some(value.to_string())))
self.set(key, ScalarValue::from(value))
}

/// Customize batch size
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ impl Expr {
Expr::GetIndexedField(GetIndexedField {
expr: Box::new(self),
field: GetFieldAccess::NamedStructField {
name: ScalarValue::Utf8(Some(name.into())),
name: ScalarValue::from(name.into()),
},
})
}
Expand Down
6 changes: 3 additions & 3 deletions datafusion/expr/src/literal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ pub trait TimestampLiteral {

impl Literal for &str {
fn lit(&self) -> Expr {
Expr::Literal(ScalarValue::Utf8(Some((*self).to_owned())))
Expr::Literal(ScalarValue::from(*self))
}
}

impl Literal for String {
fn lit(&self) -> Expr {
Expr::Literal(ScalarValue::Utf8(Some((*self).to_owned())))
Expr::Literal(ScalarValue::from(self.as_ref()))
}
}

impl Literal for &String {
fn lit(&self) -> Expr {
Expr::Literal(ScalarValue::Utf8(Some((*self).to_owned())))
Expr::Literal(ScalarValue::from(self.as_ref()))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3297,10 +3297,7 @@ mod tests {
col("c4"),
NullableInterval::from(ScalarValue::UInt32(Some(9))),
),
(
col("c1"),
NullableInterval::from(ScalarValue::Utf8(Some("a".to_string()))),
),
(col("c1"), NullableInterval::from(ScalarValue::from("a"))),
];
let output = simplify_with_guarantee(expr.clone(), guarantees);
assert_eq!(output, lit(false));
Expand All @@ -3323,8 +3320,8 @@ mod tests {
col("c1"),
NullableInterval::NotNull {
values: Interval::try_new(
ScalarValue::Utf8(Some("d".to_string())),
ScalarValue::Utf8(Some("f".to_string())),
ScalarValue::from("d"),
ScalarValue::from("f"),
)
.unwrap(),
},
Expand Down
6 changes: 3 additions & 3 deletions datafusion/optimizer/src/simplify_expressions/guarantees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,8 @@ mod tests {
col("x"),
NullableInterval::MaybeNull {
values: Interval::try_new(
ScalarValue::Utf8(Some("abc".to_string())),
ScalarValue::Utf8(Some("def".to_string())),
ScalarValue::from("abc"),
ScalarValue::from("def"),
)
.unwrap(),
},
Expand Down Expand Up @@ -463,7 +463,7 @@ mod tests {
ScalarValue::Int32(Some(1)),
ScalarValue::Boolean(Some(true)),
ScalarValue::Boolean(None),
ScalarValue::Utf8(Some("abc".to_string())),
ScalarValue::from("abc"),
ScalarValue::LargeUtf8(Some("def".to_string())),
ScalarValue::Date32(Some(18628)),
ScalarValue::Date32(None),
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/simplify_expressions/regex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl OperatorMode {
let like = Like {
negated: self.not,
expr,
pattern: Box::new(Expr::Literal(ScalarValue::Utf8(Some(pattern)))),
pattern: Box::new(Expr::Literal(ScalarValue::from(pattern))),
escape_char: None,
case_insensitive: self.i,
};
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-expr/benches/in_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn do_benches(
.collect();

let in_list: Vec<_> = (0..in_list_length)
.map(|_| ScalarValue::Utf8(Some(random_string(&mut rng, string_length))))
.map(|_| ScalarValue::from(random_string(&mut rng, string_length)))
.collect();

do_bench(
Expand Down
14 changes: 2 additions & 12 deletions datafusion/physical-expr/src/aggregate/min_max.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1297,12 +1297,7 @@ mod tests {
#[test]
fn max_utf8() -> Result<()> {
let a: ArrayRef = Arc::new(StringArray::from(vec!["d", "a", "c", "b"]));
generic_test_op!(
a,
DataType::Utf8,
Max,
ScalarValue::Utf8(Some("d".to_string()))
)
generic_test_op!(a, DataType::Utf8, Max, ScalarValue::from("d"))
}

#[test]
Expand All @@ -1319,12 +1314,7 @@ mod tests {
#[test]
fn min_utf8() -> Result<()> {
let a: ArrayRef = Arc::new(StringArray::from(vec!["d", "a", "c", "b"]));
generic_test_op!(
a,
DataType::Utf8,
Min,
ScalarValue::Utf8(Some("a".to_string()))
)
generic_test_op!(a, DataType::Utf8, Min, ScalarValue::from("a"))
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-expr/src/aggregate/string_agg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ mod tests {
)
.unwrap();

let delimiter = Arc::new(Literal::new(ScalarValue::Utf8(Some(delimiter))));
let delimiter = Arc::new(Literal::new(ScalarValue::from(delimiter)));
let schema = Schema::new(vec![Field::new("a", coerced[0].clone(), true)]);
let agg = create_aggregate_expr(
&function,
Expand Down
Loading

0 comments on commit da6efaf

Please sign in to comment.