Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Minor: Use ScalarValue::from impl for strings #8429

Merged
merged 2 commits into from
Dec 6, 2023
Merged

Minor: Use ScalarValue::from impl for strings #8429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f04cb9e
Minor: Use ScalarValue::from impl for strings
alamb Dec 5, 2023
8b96b94
fix typo
alamb Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion datafusion/common/src/pyarrow.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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")),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks better and more concise to me. Hopefully by updating the code we'll also make the codebase easier to work with over time too

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1603,11 +1603,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,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
View file
Open in desktop
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
9 changes: 3 additions & 6 deletions datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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