refactor: move acos() to function crate

[SteveLauC]

Which issue does this PR close?

Part of #9285

Rationale for this change

What changes are included in this PR?

Move math function acos() to the function crate

Are these changes tested?

Let's try this in CI:<

Are there any user-facing changes?

I guess no

[alamb]

Thanks @SteveLauC -- it appears there are some CI failuers

[SteveLauC]

it appears there are some CI failuers

Indeed, I am working on the sqllogictest failure

[SteveLauC]

Hi @alamb, I think I might need some help on the sqllogictest CI error, with the acos() function implemented in this PR:

1. select(acos(NULL) would return a column of type ? rather than R
2. select(acos(0), acos(1)) would return columns of type II rather than RR

Honestly, I have no idea about the above behavios...

[SteveLauC]

Hi @alamb, I think I might need some help on the sqllogictest CI error, with the acos() function implemented in this PR:

1. select(acos(NULL) would return a column of type ? rather than R
2. select(acos(0), acos(1)) would return columns of type II rather than RR

Honestly, I have no idea about the above behavios...

Update: I just realized that it is a mistake that I made in <AsocFunc as ScalarUDFImpl>::return_type()

---

It feels that when we do select(acos(NULL)), AsocFunc::return_type() can see the passed DataType::Null, but AsocFunc::invoke() cannot because I didn't see the following panic message:

Unsupported data type DataType::Null for function asoc

It seems that DataFusion will handle those special values for you.

[jonahgao]

select acos() will panic, but I think it should be checked at a higher level, before the implementation of acos.

DataFusion CLI v36.0.0
❯ select acos();
thread 'main' panicked at datafusion/functions/src/math/acos.rs:57:25:
index out of bounds: the len is 0 but the index is 0

In the main branch, it just throws an error.

DataFusion CLI v36.0.0
❯ select acos();
Error during planning: No function matches the given name and argument types 'acos()'. You might need to add explicit type casts.
	Candidate functions:
	acos(Float64/Float32)

[SteveLauC]

select acos() will panic, but I think it should be checked at a higher level, before the implementation of acos.

Huge thanks for catching it!

Just realized isnan() also has this issue, cc @alamb:

$ ./target/debug/datafusion-cli
DataFusion CLI v36.0.0
❯ select isnan();
thread 'main' panicked at /home/steve/Documents/workspace/GitHub/arrow-datafusion/datafusion/functions/src/math/nans.rs:67:39:
index out of bounds: the len is 0 but the index is 0

And cc @yyy1000, abs() also suffers from this.

---

but I think it should be checked at a higher level, before the implementation of acos.

Yeah, we should do it before invoking the function

[yyy1000]

select acos() will panic, but I think it should be checked at a higher level, before the implementation of acos.

Huge thanks for catching it!

Just realized isnan() also has this issue, cc @alamb:

$ ./target/debug/datafusion-cli
DataFusion CLI v36.0.0
❯ select isnan();
thread 'main' panicked at /home/steve/Documents/workspace/GitHub/arrow-datafusion/datafusion/functions/src/math/nans.rs:67:39:
index out of bounds: the len is 0 but the index is 0

And cc @yyy1000, abs() also suffers from this.

but I think it should be checked at a higher level, before the implementation of acos.

Yeah, we should do it before invoking the function

Yeah, thanks for telling me @SteveLauC I added the logic just now. :)

[alamb]

I took the liberty of merging up from main and resolving a conflict on this PR

[alamb]

Thanks again @SteveLauC