Write unit test(s) for Spark functions

[sjrusso8]

Description

There are many functions for Spark, and most of them are created via a macro. However, not all of them have unit test coverage. Create additional unit tests based on similar test conditions from the exiting Spark API test cases.

I have been mirror the docstring tests from the PySpark API for reference.

[lexara-prime-ai]

Hi, would the scope for this be the following path - core/src/functions/mod.rs
I can see quite a number of tests from other modules like column.rs.

[sjrusso8]

Hi @lexara-prime-ai Welcome! Great question!

I have been trying to mirror the doctests from pyspark 3.5. And all the column unit tests in the docs are technically using the function col. So when I was writing some of the tests I just lumped them into functions. Was this correct? Hindsight probably not, but I think I was just in the zone of writing tests and kept moving forward.

Might be worth while to move those tests into column.rs

[lexara-prime-ai]

Thanks @sjrusso8! The explanation makes a lot of sense. Appreciate the groundwork you've laid with the existing test cases, they've been incredibly helpful as I piece everything together. I'll do my best to contribute and assist as much as possible.

[lexara-prime-ai]

Hi @sjrusso8 hope you're doing well.

Quick question on how the isnotnull function has been implemented.
I assumed it would be the exact opposite of the isnull function implementation.
However, the function that gets generated by the generate_functions! macro requires an extra argument.

Moving the "definition" from here

// functions that require exactly two col arguments
generate_functions!(
    two_cols: nvl,
    nullif,
    isnotnull,
    ifnull,

to here

// functions that require a single col argument
generate_functions!(
    one_col: isnan,
    isnull,
    isnotnull, // <---
    sqrt,

allows compilation and the test passes...

res -> RecordBatch { schema: Schema { fields: [Field { name: "a", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "r1", data_type: Boolean, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }], metadata: {} }, columns: [PrimitiveArray<Int64>
[
  null,
  1,
], BooleanArray
[
  false,
  true,
]], row_count: 2 }
res -> RecordBatch { schema: Schema { fields: [Field { name: "a", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} }, Field { name: "r1", data_type: Boolean, nullable: false, dict_id: 0, dict_is_ordered: false, metadata: {} }], metadata: {} }, columns: [PrimitiveArray<Int64>
[
  null,
  1,
], BooleanArray
[
  true,
  false,
]], row_count: 2 }
test functions::tests::test_func_is_not_null ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 117 filtered out; finished in 0.11s

INITIAL COMPILER DIAGNOSTIC

  --> core/src/functions/mod.rs:1180:32
     |
1180 |             .select([col("a"), isnotnull("a").alias("r1")])
     |                                ^^^^^^^^^-----
     |                                         ||
     |                                         |expected all arguments to be this `&str` type because they need to match the type of this parameter
     |                                         an argument of type `&str` is missing
     |
note: function defined here
    --> core/src/functions/mod.rs:468:5
     |
294  |             pub fn $func_name<T: expressions::ToExpr>(col1: T, col2: T) -> Column
     |                               -                       -------  ------- this parameter needs to match the `&str` type of `col1`
     |                               |                       |
     |                               |                       `col2` needs to match the `&str` type of this parameter
     |                               `col1` and `col2` all reference this parameter T
...
468  |     isnotnull,
     |     ^^^^^^^^^
help: provide the argument
     |
1180 |             .select([col("a"), isnotnull("a", /* &str */).alias("r1")])

[sjrusso8]

@lexara-prime-ai I see what you mean. It should be only one input argument for the function, so your change is correct. Probably was not caught because of the lack of unit tests :)

I'm getting the list of arguments from the docs here -> https://spark.apache.org/docs/latest/api/python/reference/pyspark.sql/functions.html

[lexara-prime-ai]

Awesome!
Thanks for the confirmation! I'll make a note of the subtle change as part of the commit message for keeping track.

[sjrusso8]

Column code coverage was added, keeping this open for now to track other function coverage

[lexara-prime-ai]

@sjrusso8 was looking through other modules as well. Are there any areas that you'd consider high priority so I can start from there?

[sjrusso8]

@lexara-prime-ai there are a bunch of functions that have not been implemented. I attempted to make an inventory on the issue #60.

Any of the functions that do something with UDFs probably can't be implemented. Spark UDFs only really work with python and scala right now

[lexara-prime-ai]

Ok just went through the list, I can take a look at the other issues in the meantime.