add warning when map with double or real as key is created

[singcha]

Test plan -
Unit tests & manual tests on cluster

== RELEASE NOTES ==
General Changes
Creating a map with double/real as keys will generate warnings

[souravpal]

love the test cases!

[rschlussel]

Instead of Node: in the warning, I would say Expression: since that's more comprehensible to users

[kaikalur]

Hmm so this is a bit tricky. Ideally we don't want to warn on intermediate/inferred types like histogram. SO I suggest doing it only for cases where the user explicitly creates this type like column definitions or cast expression (whereve syntactically a Type is allowed).

[singcha]

Creating a histogram with double buckets is going to have non-deterministic behavior. Thats a user action trying to create map equivalent indirectly with double keys.
I think it still adds value from end user perspective. thoughts?

[kaikalur]

Creating a histogram with double buckets is going to have non-deterministic behavior. Thats a user action trying to create map equivalent indirectly with double keys. I think it still adds value from end user perspective. thoughts?

Not really as the user can't fix it. The map in this case is just a convenience for presenting histogram results. Complaining about it will be too noisy.

I suggest just doing it in the AstBuilder when you are building a type node.

[kaikalur]

Creating a histogram with double buckets is going to have non-deterministic behavior. Thats a user action trying to create map equivalent indirectly with double keys. I think it still adds value from end user perspective. thoughts?

Not really as the user can't fix it. The map in this case is just a convenience for presenting histogram results. Complaining about it will be too noisy.

I suggest just doing it in the AstBuilder when you are building a type node.

Also maybe warn if they try to "modify" or materialize (write to table) the map - like concat/union etc which can end up with duplicate keys. So if the histogram is not written to a table, but just used for analysis it's fine. But if it is written to a table then there should be a warning.

[rschlussel]

So if the histogram is not written to a table, but just used for analysis it's fine. But if it is written to a table then there should be a warning.

I'm not sure that makes sense. There isn't really an issue with writing to a table. The whole problem is that the user could non-deterministically create duplicate keys. So i would either warn all the time with histogram or not at all.

[kaikalur]

So if the histogram is not written to a table, but just used for analysis it's fine. But if it is written to a table then there should be a warning.

I'm not sure that makes sense. There isn't really an issue with writing to a table. The whole problem is that the user could non-deterministically create duplicate keys. So i would either warn all the time with histogram or not at all.

The main thing is the map if it is created is "correct" - only when try to manipulate it you can get it into trouble. But yeah I see your point - writing to table is ok. But if we see the CREATE TABLE in Presto, we can warn on column type being a bad map. Or always warn on write to such tables to handle Spark created tables.

[rschlussel]

Small comment, otherwise looks good to me. @kaikalur any other comments?

[rschlussel]

nit: missing a space before "Please"

[singcha]

fixed!

[kaikalur]

Nice!