andygrove commented on PR #5854:
URL:
https://github.com/apache/datafusion-comet/pull/5854#issuecomment-5876593320
This is a light fully automated review since there are so many PRs open.
The floating point note at
`spark/src/main/scala/org/apache/comet/serde/maps.scala:144`, and the known
limitation at
`docs/source/contributor-guide/expression-audits/map_funcs.md:53`, say Spark
3.4 and 3.5 already match because they do not normalize. I think that only
holds for a top-level `FLOAT` or `DOUBLE` key. For a struct or array key type,
`ArrayBasedMapBuilder` on every version from 3.4.3 through 4.1.1 keys its dedup
`TreeMap` on `TypeUtils.getInterpretedOrdering`, and the double ordering there
is `SQLOrderingUtil.compareDoubles`, which treats `-0.0` and `+0.0` as equal.
The upstream `map_deduplicate_keys` keys a `HashMap` on `ScalarValue`, whose
struct and list equality compares the float bits, so it keeps them apart. With
a column `ks array<struct<a: double>>` holding `array(named_struct('a', -0.0D),
named_struct('a', 0.0D))`, `map_from_arrays(ks, array(1, 2))` raises
`DUPLICATED_MAP_KEY` in Spark and returns two entries in Comet. Under
`LAST_WIN` Spark returns one entry,
and before this change that policy was declined, so this case used to be
correct. Could the note scope the 3.4 and 3.5 claim to top-level keys and call
out nested keys on every version? Or, since `MapKeySupport.keySupport` already
declines a floating point type at any nesting level, would it make sense for
`MapBuilderSupport.keySupport` to decline a complex key type that contains a
float or double?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
