fix: Avoid precision loss for atan2 with integer args

[neilconway]

Which issue does this PR close?

• Closes atan2 precision loss with integer arguments #22514.

Rationale for this change

atan2 defined two input signatures: (Float32, Float32) and (Float64, Float64) (in that order). That meant that integer inputs were coerced into Float32 values, which lead to incorrect results: atan2(1, 1000000) resulted in less precision than atan2(1.0, 1000000.0); the results for the former were also inconsistent with the behavior of atan2 in Postgres and DuckDB.

Fix this by only using the Float32 path when given two Float32 inputs; for other inputs, we should use Float64. This avoids rounding for large integer inputs (Float32 has only 24 mantissa bits, so larger integers would get rounded).

What changes are included in this PR?

• Fix atan2 signature to only take the Float32 code path for two Float32 inputs
• Update SLT, add new SLT test

Are these changes tested?

Yes, new test added.

Are there any user-facing changes?

Yes: the return type and semantics of atan2 in some circumstances has changed. atan2 will now only be computed in Float32 when passed two Float32 values. In all other cases, the computation will be done in Float64 and a Float64 value will be returned.

[Jefffrey]

Would we see similar issues for other math functions, meaning if we follow this approach we'd have to change math functions to operate predominantly on f64?

[neilconway]

Would we see similar issues for other math functions, meaning if we follow this approach we'd have to change math functions to operate predominantly on f64?

We wouldn't. The main thing we need to avoid is using f32 for integer inputs. We can either do that by just removing f32 support, or ensuring that we use the f64 path for integer inputs. This PR initially did the former, but on closer examination, I see that quite a few DataFusion numerical functions have a float32 code path. So it is probably more consistent to switch to the second approach, which I've now done.

[alamb]

Makes sense to me -- thank you @neilconway

[Jefffrey]

we could simplify as so too

fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
    Ok(arg_types[0].clone())
}

since args are already coerced by this point

[neilconway]

Personally I'd prefer to be explicit, rather than depending on the coupling / timing dependency between type coercion and return_type. There's enough subtle behavior in this code as it is...