Skip to content

[minor] Consolidate construction of the list field #13627

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

Merged
alamb merged 1 commit into from
Dec 4, 2024
Merged

[minor] Consolidate construction of the list field #13627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions datafusion/common/src/dfschema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,14 +1382,14 @@ mod tests {

// Succeeds if both have the same element type, disregards names and nullability
assert!(DFSchema::datatype_is_logically_equal(
&DataType::List(Field::new("item", DataType::Int8, true).into()),
&DataType::List(Field::new_list_field(DataType::Int8, true).into()),
&DataType::List(Field::new("element", DataType::Int8, false).into())
));

// Fails if element type is different
assert!(!DFSchema::datatype_is_logically_equal(
&DataType::List(Field::new("item", DataType::Int8, true).into()),
&DataType::List(Field::new("item", DataType::Int16, true).into())
&DataType::List(Field::new_list_field(DataType::Int8, true).into()),
&DataType::List(Field::new_list_field(DataType::Int16, true).into())
));

// Test maps
Expand Down Expand Up @@ -1522,14 +1522,14 @@ mod tests {

// Succeeds if both have the same element type, disregards names and nullability
assert!(DFSchema::datatype_is_semantically_equal(
&DataType::List(Field::new("item", DataType::Int8, true).into()),
&DataType::List(Field::new_list_field(DataType::Int8, true).into()),
&DataType::List(Field::new("element", DataType::Int8, false).into())
));

// Fails if element type is different
assert!(!DFSchema::datatype_is_semantically_equal(
&DataType::List(Field::new("item", DataType::Int8, true).into()),
&DataType::List(Field::new("item", DataType::Int16, true).into())
&DataType::List(Field::new_list_field(DataType::Int8, true).into()),
&DataType::List(Field::new_list_field(DataType::Int16, true).into())
));

// Test maps
Expand Down
44 changes: 20 additions & 24 deletions datafusion/common/src/scalar/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4037,7 +4037,7 @@ mod tests {
#[test]
fn test_to_array_of_size_for_fsl() {
let values = Int32Array::from_iter([Some(1), None, Some(2)]);
let field = Arc::new(Field::new("item", DataType::Int32, true));
let field = Arc::new(Field::new_list_field(DataType::Int32, true));
let arr = FixedSizeListArray::new(Arc::clone(&field), 3, Arc::new(values), None);
let sv = ScalarValue::FixedSizeList(Arc::new(arr));
let actual_arr = sv
Expand Down Expand Up @@ -4093,17 +4093,15 @@ mod tests {
)
} else if O::IS_LARGE {
new_null_array(
&DataType::LargeList(Arc::new(Field::new(
"item",
&DataType::LargeList(Arc::new(Field::new_list_field(
DataType::Int64,
true,
))),
1,
)
} else {
new_null_array(
&DataType::List(Arc::new(Field::new(
"item",
&DataType::List(Arc::new(Field::new_list_field(
DataType::Int64,
true,
))),
Expand All @@ -4122,7 +4120,7 @@ mod tests {

#[test]
fn test_iter_to_array_fixed_size_list() {
let field = Arc::new(Field::new("item", DataType::Int32, true));
let field = Arc::new(Field::new_list_field(DataType::Int32, true));
let f1 = Arc::new(FixedSizeListArray::new(
Arc::clone(&field),
3,
Expand Down Expand Up @@ -4961,15 +4959,15 @@ mod tests {
let null_list_scalar = ScalarValue::try_from_array(&list, 1).unwrap();

let data_type =
DataType::List(Arc::new(Field::new("item", DataType::Int32, true)));
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true)));

assert_eq!(non_null_list_scalar.data_type(), data_type);
assert_eq!(null_list_scalar.data_type(), data_type);
}

#[test]
fn scalar_try_from_list_datatypes() {
let inner_field = Arc::new(Field::new("item", DataType::Int32, true));
let inner_field = Arc::new(Field::new_list_field(DataType::Int32, true));

// Test for List
let data_type = &DataType::List(Arc::clone(&inner_field));
Expand Down Expand Up @@ -5010,19 +5008,20 @@ mod tests {

#[test]
fn scalar_try_from_list_of_list() {
let data_type = DataType::List(Arc::new(Field::new(
"item",
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
let data_type = DataType::List(Arc::new(Field::new_list_field(
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true))),
true,
)));
let data_type = &data_type;
let scalar: ScalarValue = data_type.try_into().unwrap();

let expected = ScalarValue::List(
new_null_array(
&DataType::List(Arc::new(Field::new(
"item",
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
&DataType::List(Arc::new(Field::new_list_field(
DataType::List(Arc::new(Field::new_list_field(
DataType::Int32,
true,
))),
true,
))),
1,
Expand All @@ -5038,13 +5037,12 @@ mod tests {
#[test]
fn scalar_try_from_not_equal_list_nested_list() {
let list_data_type =
DataType::List(Arc::new(Field::new("item", DataType::Int32, true)));
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true)));
let data_type = &list_data_type;
let list_scalar: ScalarValue = data_type.try_into().unwrap();

let nested_list_data_type = DataType::List(Arc::new(Field::new(
"item",
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
let nested_list_data_type = DataType::List(Arc::new(Field::new_list_field(
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true))),
true,
)));
let data_type = &nested_list_data_type;
Expand Down Expand Up @@ -5678,7 +5676,7 @@ mod tests {
let field_a = Arc::new(Field::new("A", DataType::Utf8, false));
let field_primitive_list = Arc::new(Field::new(
"primitive_list",
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true))),
false,
));

Expand Down Expand Up @@ -5879,9 +5877,8 @@ mod tests {
fn build_2d_list(data: Vec<Option<i32>>) -> ListArray {
let a1 = ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(data)]);
ListArray::new(
Arc::new(Field::new(
"item",
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
Arc::new(Field::new_list_field(
DataType::List(Arc::new(Field::new_list_field(DataType::Int32, true))),
true,
)),
OffsetBuffer::<i32>::from_lengths([1]),
Expand Down Expand Up @@ -6876,8 +6873,7 @@ mod tests {
assert_eq!(1, arr.len());
assert_eq!(
arr.data_type(),
&DataType::List(Arc::new(Field::new(
"item",
&DataType::List(Arc::new(Field::new_list_field(
DataType::Timestamp(TimeUnit::Millisecond, Some(s.into())),
true,
)))
Expand Down
2 changes: 1 addition & 1 deletion datafusion/core/tests/sql/aggregates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async fn csv_query_array_agg_distinct() -> Result<()> {
*actual[0].schema(),
Schema::new(vec![Field::new_list(
"array_agg(DISTINCT aggregate_test_100.c2)",
Field::new("item", DataType::UInt32, true),
Field::new_list_field(DataType::UInt32, true),
true
),])
);
Expand Down
5 changes: 2 additions & 3 deletions datafusion/expr-common/src/type_coercion/binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,7 @@ mod tests {
);

// list
let inner_field = Arc::new(Field::new("item", DataType::Int64, true));
let inner_field = Arc::new(Field::new_list_field(DataType::Int64, true));
test_coercion_binary_rule!(
DataType::List(Arc::clone(&inner_field)),
DataType::List(Arc::clone(&inner_field)),
Expand Down Expand Up @@ -2155,8 +2155,7 @@ mod tests {
);

// Negative test: inner_timestamp_field and inner_field are not compatible because their inner types are not compatible
let inner_timestamp_field = Arc::new(Field::new(
"item",
let inner_timestamp_field = Arc::new(Field::new_list_field(
DataType::Timestamp(TimeUnit::Microsecond, None),
true,
));
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/logical_plan/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2432,7 +2432,7 @@ mod tests {
],
false,
);
let string_field = Field::new("item", DataType::Utf8, false);
let string_field = Field::new_list_field(DataType::Utf8, false);
let strings_field = Field::new_list("item", string_field.clone(), false);
let schema = Schema::new(vec![
Field::new("scalar", DataType::UInt32, false),
Expand Down
17 changes: 7 additions & 10 deletions datafusion/expr/src/type_coercion/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ mod tests {

#[test]
fn test_fixed_list_wildcard_coerce() -> Result<()> {
let inner = Arc::new(Field::new("item", DataType::Int32, false));
let inner = Arc::new(Field::new_list_field(DataType::Int32, false));
let current_types = vec![
DataType::FixedSizeList(Arc::clone(&inner), 2), // able to coerce for any size
];
Expand Down Expand Up @@ -996,10 +996,9 @@ mod tests {
#[test]
fn test_nested_wildcard_fixed_size_lists() -> Result<()> {
let type_into = DataType::FixedSizeList(
Arc::new(Field::new(
"item",
Arc::new(Field::new_list_field(
DataType::FixedSizeList(
Arc::new(Field::new("item", DataType::Int32, false)),
Arc::new(Field::new_list_field(DataType::Int32, false)),
FIXED_SIZE_LIST_WILDCARD,
),
false,
Expand All @@ -1008,10 +1007,9 @@ mod tests {
);

let type_from = DataType::FixedSizeList(
Arc::new(Field::new(
"item",
Arc::new(Field::new_list_field(
DataType::FixedSizeList(
Arc::new(Field::new("item", DataType::Int8, false)),
Arc::new(Field::new_list_field(DataType::Int8, false)),
4,
),
false,
Expand All @@ -1022,10 +1020,9 @@ mod tests {
assert_eq!(
coerced_from(&type_into, &type_from),
Some(DataType::FixedSizeList(
Arc::new(Field::new(
"item",
Arc::new(Field::new_list_field(
DataType::FixedSizeList(
Arc::new(Field::new("item", DataType::Int32, false)),
Arc::new(Field::new_list_field(DataType::Int32, false)),
4,
),
false,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/COMMENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ first argument and the definition looks like this:
// `input_type` : data type of the first argument
let mut fields = vec![Field::new_list(
format_state_name(self.name(), "nth_value"),
Field::new("item", args.input_types[0].clone(), true /* nullable of list item */ ),
Field::new_list_field(args.input_types[0].clone(), true /* nullable of list item */ ),
false, // nullable of list itself
)];
```
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/approx_median.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl AggregateUDFImpl for ApproxMedian {
Field::new(format_state_name(args.name, "min"), Float64, false),
Field::new_list(
format_state_name(args.name, "centroids"),
Field::new("item", Float64, true),
Field::new_list_field(Float64, true),
false,
),
])
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/approx_percentile_cont.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl AggregateUDFImpl for ApproxPercentileCont {
),
Field::new_list(
format_state_name(args.name, "centroids"),
Field::new("item", DataType::Float64, true),
Field::new_list_field(DataType::Float64, true),
false,
),
])
Expand Down
9 changes: 4 additions & 5 deletions datafusion/functions-aggregate/src/array_agg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ impl AggregateUDFImpl for ArrayAgg {
}

fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
Ok(DataType::List(Arc::new(Field::new(
"item",
Ok(DataType::List(Arc::new(Field::new_list_field(
arg_types[0].clone(),
true,
))))
Expand All @@ -89,15 +88,15 @@ impl AggregateUDFImpl for ArrayAgg {
return Ok(vec![Field::new_list(
format_state_name(args.name, "distinct_array_agg"),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.input_types[0].clone(), true),
Field::new_list_field(args.input_types[0].clone(), true),
true,
)]);
}

let mut fields = vec![Field::new_list(
format_state_name(args.name, "array_agg"),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.input_types[0].clone(), true),
Field::new_list_field(args.input_types[0].clone(), true),
true,
)];

Expand All @@ -108,7 +107,7 @@ impl AggregateUDFImpl for ArrayAgg {
let orderings = args.ordering_fields.to_vec();
fields.push(Field::new_list(
format_state_name(args.name, "array_agg_orderings"),
Field::new("item", DataType::Struct(Fields::from(orderings)), true),
Field::new_list_field(DataType::Struct(Fields::from(orderings)), true),
false,
));

Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/bit_and_or_xor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl AggregateUDFImpl for BitwiseOperation {
format!("{} distinct", self.name()).as_str(),
),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.return_type.clone(), true),
Field::new_list_field(args.return_type.clone(), true),
false,
)])
} else {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/count.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl AggregateUDFImpl for Count {
Ok(vec![Field::new_list(
format_state_name(args.name, "count distinct"),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.input_types[0].clone(), true),
Field::new_list_field(args.input_types[0].clone(), true),
false,
)])
} else {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/median.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl AggregateUDFImpl for Median {

fn state_fields(&self, args: StateFieldsArgs) -> Result<Vec<Field>> {
//Intermediate state is a list of the elements we have collected so far
let field = Field::new("item", args.input_types[0].clone(), true);
let field = Field::new_list_field(args.input_types[0].clone(), true);
let state_name = if args.is_distinct {
"distinct_median"
} else {
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-aggregate/src/nth_value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ impl AggregateUDFImpl for NthValueAgg {
let mut fields = vec![Field::new_list(
format_state_name(self.name(), "nth_value"),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.input_types[0].clone(), true),
Field::new_list_field(args.input_types[0].clone(), true),
false,
)];
let orderings = args.ordering_fields.to_vec();
if !orderings.is_empty() {
fields.push(Field::new_list(
format_state_name(self.name(), "nth_value_orderings"),
Field::new("item", DataType::Struct(Fields::from(orderings)), true),
Field::new_list_field(DataType::Struct(Fields::from(orderings)), true),
false,
));
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/functions-aggregate/src/sum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl AggregateUDFImpl for Sum {
Ok(vec![Field::new_list(
format_state_name(args.name, "sum distinct"),
// See COMMENTS.md to understand why nullable is set to true
Field::new("item", args.return_type.clone(), true),
Field::new_list_field(args.return_type.clone(), true),
false,
)])
} else {
Expand Down
4 changes: 2 additions & 2 deletions datafusion/functions-nested/benches/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ fn criterion_benchmark(c: &mut Criterion) {

c.bench_function("map_1000", |b| {
let mut rng = rand::thread_rng();
let field = Arc::new(Field::new("item", DataType::Utf8, true));
let field = Arc::new(Field::new_list_field(DataType::Utf8, true));
let offsets = OffsetBuffer::new(ScalarBuffer::from(vec![0, 1000]));
let key_list = ListArray::new(
field,
offsets,
Arc::new(StringArray::from(keys(&mut rng))),
None,
);
let field = Arc::new(Field::new("item", DataType::Int32, true));
let field = Arc::new(Field::new_list_field(DataType::Int32, true));
let offsets = OffsetBuffer::new(ScalarBuffer::from(vec![0, 1000]));
let value_list = ListArray::new(
field,
Expand Down
Loading
Loading