Deduplicate Spark function code with native/default datafusion function code

@Jefffrey

Is your feature request related to a problem or challenge?

Some Spark functions have equivalent DataFusion functions, differing slightly in behaviour. Work towards reducing duplicated code, calling out any differences between the functions when we can't merge them together.

Functions with Spark on left, DF (closest) equivalent on right:

Not all of the above are exact equivalents, they're just listed to make it easier to check if we can reduce duplication to some degree. I've listed the Spark functions I noticed that seem to have something similar in DataFusion already.

I would like to work on this.

I would like to work on this.

Cheers, feel free to pick a specific function to work on (I think it would be good to have a PR per function instead of one big PR for all)

As the issue has multiple, I would like to first pick 5 tasks out of them and start working. Will raise a PR per task. If no one else picked others by the time I finish the 5, i'll pick the rest of them.

@Jefffrey

Hi @Jefffrey I was looking at some of the pairs and and have some questions:

For rint -> round, from what I understand, rint uses float round_ties_even while round uses float round, which would produce difference results for floats half-way between integers. Is that intended, or what was the reason?
For array -> make_array,
- I did not seem to find unit tests in datafusion/functions-nested/src/make_array.rs or datafusion/spark/src/functions/spark_array.rs. Are tests kept in other files?
- For coalesce_types, make_array uses type_union_resolution while spark_array uses comparison_coersion (with folding), both of which are implemented in datafusion_expr_common::type_coercion::binary. It seems to me that they are very similar for the purpose of coalescing types and make_array can also use comparison_coersion, but wanted to check in case I misunderstood anything. Also are there unit tests for type_union_resolution? did not seem to find under type_coercion::binary module.
- make_array uses field name LIST_FIELD_DEFAULT_NAME (which is "item") while spark_array uses field name ARRAY_FIELD_DEFAULT_NAME (which is "element"). Are they intended to be different?
- make_array does not handle case for DataType::LargeList while spark_array does. Is there a reason for that?

Thanks a lot!

@jizezhang

For rint -> round, from what I understand, rint uses float round_ties_even while round uses float round, which would produce difference results for floats half-way between integers. Is that intended, or what was the reason?

This is indeed the intention of the ticket; to find out these differences and ensure they are either eliminated if there is no reason for them to be different, or to document these differences so in future we know that they are meant to be different. In this case it's worth going through the commit/PR history to see if there are reasons given, otherwise falling back to testing against Spark to confirm if these are meant to be different.

I did not seem to find unit tests in datafusion/functions-nested/src/make_array.rs or datafusion/spark/src/functions/spark_array.rs. Are tests kept in other files?

We prefer keeping our tests as sqllogictests, see: https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest

For coalesce_types, make_array uses type_union_resolution while spark_array uses comparison_coersion (with folding), both of which are implemented in datafusion_expr_common::type_coercion::binary. It seems to me that they are very similar for the purpose of coalescing types and make_array can also use comparison_coersion, but wanted to check in case I misunderstood anything. Also are there unit tests for type_union_resolution? did not seem to find under type_coercion::binary module.

Unfortunately I'm not sure of this myself.

make_array uses field name LIST_FIELD_DEFAULT_NAME (which is "item") while spark_array uses field name ARRAY_FIELD_DEFAULT_NAME (which is "element"). Are they intended to be different?

I'm inclined to believe they are meant to be different, but some investigation & accompanying documentation would be very helpful for us in the future to confirm the fact.

make_array does not handle case for DataType::LargeList while spark_array does. Is there a reason for that?

I actually think you may have spotted a bug here 🤔

The spark_array implementation does seem to suggest it may return a LargeList for certain inputs, but this conflicts with the stated return type which should always return a List (not a LargeList):

datafusion/datafusion/spark/src/function/array/spark_array.rs

Lines 75 to 108 in 9238779

    
           fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> { 
        
               let mut expr_type = DataType::Null; 
        
               for arg_type in arg_types { 
        
                   if !arg_type.equals_datatype(&DataType::Null) { 
        
                       expr_type = arg_type.clone(); 
        
                       break; 
        
                   } 
        
               } 
        
               if expr_type.is_null() { 
        
                   expr_type = DataType::Int32; 
        
               } 
        
               Ok(DataType::List(Arc::new(Field::new( 
        
                   ARRAY_FIELD_DEFAULT_NAME, 
        
                   expr_type, 
        
                   true, 
        
               )))) 
        
           } 
        
           fn return_field_from_args(&self, args: ReturnFieldArgs) -> Result<FieldRef> { 
        
               let data_types = args 
        
                   .arg_fields 
        
                   .iter() 
        
                   .map(|f| f.data_type()) 
        
                   .cloned() 
        
                   .collect::<Vec<_>>(); 
        
               let return_type = self.return_type(&data_types)?; 
        
               Ok(Arc::new(Field::new( 
        
                   "this_field_name_is_irrelevant", 
        
                   return_type, 
        
                   false, 
        
               ))) 
        
           }

Side note: it's also confusing how both return_type and return_field_from_args are implemented since usually it's one or the other, so could do with some refactoring here

Might be worth mocking an example test which could exhibit this bug (where it returns a LargeList in spite of it stating its return type as regular List)

See the tests for the spark version of array function here: https://github.com/apache/datafusion/blob/9238779f45418e07108079aeef3b51de85e9fb8f/datafusion/sqllogictest/test_files/spark/array/array.slt

And use this as reference for how we can construct a LargeList via SLT tests:

datafusion/datafusion/sqllogictest/test_files/array.slt

Lines 7946 to 7949 in 9238779

    
           query B 
        
           select list_empty(arrow_cast(make_array(1), 'LargeList(Int64)')); 
        
           ---- 
        
           false

@jizezhang

@Jefffrey

@Jefffrey Thanks a lot for the detailed response!

For array I'll start with what you suggested:

Might be worth mocking an example test which could exhibit this bug (where it returns a LargeList in spite of it stating its return type as regular List)

For rint/round, I think this is the PR for rint and this is the initial PR for round. Neither seems to state the reason for choice of rounding function, though the commit for rint did include tests for floats half-way between integers while the commit forround did not.

@Jefffrey Thanks a lot for the detailed response!

For array I'll start with what you suggested:

Might be worth mocking an example test which could exhibit this bug (where it returns a LargeList in spite of it stating its return type as regular List)

For rint/round, I think this is the PR for rint and this is the initial PR for round. Neither seems to state the reason for choice of rounding function, though the commit for rint did include tests for floats half-way between integers while the commit forround did not.

For rint it sounds like best approach is just to leave a comment explaining this is the intended functionality, calling out the difference in using round & round_ties_even so we have concrete reference

query error
SELECT array(arrow_cast(array(1), 'LargeList(Int64)'))
----
DataFusion error: Internal error: Function 'array' returned value of type 'LargeList(Field { name: "element", data_type: LargeList(Field { data_type: Int64, nullable: true }), nullable: true })' while the following type was promised at planning time and expected: 'List(Field { name: "element", data_type: LargeList(Field { data_type: Int64, nullable: true }), nullable: true })'.
This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues

This would confirm the bug?

Looking over the PR for spark array implementation, I did not seem to find comments related to the handling of LargeList. In this case, to fix the bug and align with the behavior of make_array, should the return type be List regardless of whether the inner data type is LargeList or not?

Thanks!

Yep, that reproduction looks good and the fix sounds right.

@Jefffrey

@Jefffrey could you help triggering CI workflows for this PR #18485 ? Last one failed due to formatting (fixed now), but other workflows have all passed. Thanks!

Regarding function names/aliases:

The spark_make_array alias seems pretty odd given I don't believe spark names the array like that:

datafusion/datafusion/spark/src/function/array/spark_array.rs

Line 57 in 6685bbe

aliases: vec![String::from("spark_make_array")],

So perhaps we can just remove it. We could then look into adding array as an alias for make_array (Datafusion version).

I'm not familiar with how critical it is for the Spark version to have the field name, there was some discussion about that in the original PR it seems: #16936 (comment)

Maybe for now we still leave them as separate functions (though I'm still in favour of removing the spark_make_array alias entirely) and just ensure they use the same internal code for doing the actual make array.

Ah when working on the change I realized one more difference between the two: when the array is empty, make_array sets data type to be DataType::Null while spark_array sets it to be DataType::Int32. This logic is used both in determining return types and in make_array_inner method.

Specifically, for make_array, see

datafusion/datafusion/functions-nested/src/make_array.rs

Lines 106 to 107 in 899653f

    
           let element_type = if arg_types.is_empty() { 
        
               Null

and

datafusion/datafusion/functions-nested/src/make_array.rs

Line 161 in 899653f

let array = new_null_array(&Null, length);

And for spark_array, see

datafusion/datafusion/spark/src/function/array/spark_array.rs

Lines 95 to 97 in 899653f

    
           if expr_type.is_null() { 
        
               expr_type = DataType::Int32; 
        
           }

and

datafusion/datafusion/spark/src/function/array/spark_array.rs

Line 163 in 899653f

let array = new_null_array(&DataType::Int32, length);

I think the difference in return types may also explains why make_array uses default return_field_from_args while spark_array overrrides so that it can set nullability to be false?

datafusion/datafusion/spark/src/function/array/spark_array.rs

Lines 105 to 109 in 899653f

    
           Ok(Arc::new(Field::new( 
        
               "this_field_name_is_irrelevant", 
        
               return_type, 
        
               false, 
        
           )))

Also as mentioned previously, make_array_inner method of the two has the difference in using Field::new_list_field with default element name "item" and using Field::new with specified element name "element", which further makes it a bit tricky to share make_array_inner method.

Please let me know your thoughts. Thank you.

This is interesting; I checked on PySpark (4.0.1) and it seems it should be a void (aka null) type:

>>> spark.sql("select array()").printSchema()
root
 |-- array(): array (nullable = false)
 |    |-- element: void (containsNull = false)

So the behaviour of setting it to Int32 when no columns are provided isn't consistent with Spark 🤔

Looks like the code was copied from sail but sail recently fixed this issue lakehq/sail#841. I guess we should also change the return type to be null in those cases?

Looks like the code was copied from sail but sail recently fixed this issue lakehq/sail#841. I guess we should also change the return type to be null in those cases?

Yep, that would be a nice fix to have

@Jefffrey

Hi @Jefffrey , I put out a PR #19006 to merge the two array functions. Please take a look when you get a chance. Thanks.

chore: merge make_array and spark array (#19006)

@Jefffrey

@Jefffrey I took a look at avg, and had some questions;
This is the current behaviour:

Spark avg only handles numeric→Float64, non-distinct, and uses i64 count with Float64 sum. State schema is [sum: input_type, count: Int64].
DF avg supports decimals/durations/ints/floats, distinct, u64 counts, and richer accumulators/state.

My Thoughts:

Extract a configurable/shared avg in datafusion_functions (or a shared helper) that supports a “Spark mode” (i64 counts, state schema), but otherwise reuses the DF avg implementation (type coercion, distinct, groups accumulator).
Replace the Spark avg implementation with a thin wrapper (make_udaf_function! style) over that shared avg, carrying only Spark-specific differences (e.g., count type or any ANSI-mode tweaks).
If count type must stay i64(if this is what we want?), we can make it a small configuration knob in the shared code rather than a forked accumulator; otherwise align to DF’s u64 to remove more divergence.

I would like to know what are your thoughts on this.

@Jefffrey I took a look at avg, and had some questions; This is the current behaviour:

* Spark avg only handles numeric→Float64, non-distinct, and uses i64 count with Float64 sum. State schema is [sum: input_type, count: Int64].

* DF avg supports decimals/durations/ints/floats, distinct, u64 counts, and richer accumulators/state.

My Thoughts:

1. Extract a configurable/shared avg in `datafusion_functions` (or a shared helper) that supports a “Spark mode” (i64 counts, state schema), but otherwise reuses the DF avg implementation (type coercion, distinct, groups accumulator).

2. Replace the Spark avg implementation with a thin wrapper (`make_udaf_function!` style) over that shared avg, carrying only Spark-specific differences (e.g., count type or any ANSI-mode tweaks).

3. If count type must stay `i64`(if this is what we want?), we can make it a small configuration knob in the shared code rather than a forked accumulator; otherwise align to DF’s u64 to remove more divergence.

I would like to know what are your thoughts on this.

I'm hesitant to include the count being i64 as part of the configuration for a shared average, unless we can find concrete reason for this being the case other than "Spark does it this way". For example, if there are execution related issues with using a u64 in comet or if there is an edge case that is triggered when count is i64 instead of u64 (can we do averages for counts that high?).

Similarly for the input types that Spark average supports, we likely need to check against Spark itself if we should be supporting more types natively or if it also just converts input types into float. I can't tell whether the Spark average implementation in DataFusion is completely following Spark semantics, or it so far only implements a subset of the semantics.

@Jefffrey

@Jefffrey There are two differences between DF and Spark version of the Factorial:

Input type: Int32 (Spark) vs Int64 (DataFusion)
Out-of-range handling:
i) Negative values: 1 (DF) vs NULL (Spark)
ii) Values > 20: Error (DF) vs NULL (Spark)

What would be the desired change?

For Char > chr:

The core computation logic differs fundamentally, so only documentation/structural sharing is feasible:

Aspect	DataFusion `chr`	Spark `char`
Value Range	Full Unicode (0 – 0x10FFFF)	Uses `value % 256` (Latin-1 behavior)
Negative Input	Returns error	Returns empty string
Example	`chr(321)` → error (invalid)	`char(321)` → `A` (`321 % 256 = 65`)

Thanks for looking into this; I don't really have the time to try delve into it more so for now I guess it is fine to keep them as is.

I will update my findings for others as I get to them for future reference.

perf: implement convert_to_state for SparkAvg (apache#21548)

Navigation Menu

Repository navigation

Deduplicate Spark function code with native/default datafusion function code #17964

Is your feature request related to a problem or challenge?

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
	let mut expr_type = DataType::Null;
	for arg_type in arg_types {
	if !arg_type.equals_datatype(&DataType::Null) {
	expr_type = arg_type.clone();
	break;
	}
	}

	if expr_type.is_null() {
	expr_type = DataType::Int32;
	}

	Ok(DataType::List(Arc::new(Field::new(
	ARRAY_FIELD_DEFAULT_NAME,
	expr_type,
	true,
	))))
	}

	fn return_field_from_args(&self, args: ReturnFieldArgs) -> Result<FieldRef> {
	let data_types = args
	.arg_fields
	.iter()
	.map(\|f\| f.data_type())
	.cloned()
	.collect::<Vec<_>>();
	let return_type = self.return_type(&data_types)?;
	Ok(Arc::new(Field::new(
	"this_field_name_is_irrelevant",
	return_type,
	false,
	)))
	}

	query B
	select list_empty(arrow_cast(make_array(1), 'LargeList(Int64)'));
	----
	false

Deduplicate Spark function code with native/default datafusion function code #17964

Description

Is your feature request related to a problem or challenge?

Activity

rawwar commented on Oct 31, 2025

Jefffrey commented on Nov 1, 2025

rawwar commented on Nov 2, 2025

jizezhang commented on Nov 3, 2025

Jefffrey commented on Nov 3, 2025

jizezhang commented on Nov 4, 2025

Jefffrey commented on Nov 4, 2025

jizezhang commented on Nov 4, 2025

Jefffrey commented on Nov 4, 2025

jizezhang commented on Nov 4, 2025

7 remaining items

Jefffrey commented on Nov 14, 2025

jizezhang commented on Nov 15, 2025

Jefffrey commented on Nov 17, 2025

jizezhang commented on Nov 18, 2025

Jefffrey commented on Nov 18, 2025

jizezhang commented on Dec 1, 2025

kumarUjjawal commented on Dec 15, 2025

Jefffrey commented on Dec 17, 2025

kumarUjjawal commented on Dec 27, 2025

kumarUjjawal commented on Dec 29, 2025

Jefffrey commented on Dec 29, 2025

kumarUjjawal commented on Dec 29, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions