fix: fixed nondeterminism in UdfIndex

[Sullivan-Patrick]

Description

When working on LEAST and GREATEST functions, I noticed there was some nondeterminism inside the UdfIndex class, specifically with the order field, which was intended to give comparably equal functions a final ordering based in the way they were added inside the Udf's class. This wasn't working in practice, and was leading to equally comparable methods that matched a query being selected arbitrarily. For example, when calling LEAST(INT, LONG), several UDFs could be selected: LEAST(LONG, LONG...), LEAST(DOUBLE, DOUBLE...), LEAST(DECIMAL, DECIMAL).

To fix this, UdfIndex.Node.compare() was changed to no longer consider order. As this change implies we are no longer able to select a matching function if multiple exist, getFunction now throws a KsqlException if there is more than one equally comparable method that would match the query.

A minor change that was made on top of this is adding to UdfIndex.Node.compare() the preference of picking methods with fewer generics as a final comparison.

BREAKING CHANGE: Existing queries that relied on vague implicit casting will not be started after an upgrade, and new queries that rely on vague implicit casting will be rejected. For example, foo(INT, INT) will not be able to resolve against two underlying function signatures of foo(BIGINT, BIGINT) and foo(DOUBLE, DOUBLE). Calling a function whose only parameter is variadic with an explicit null will also result in the call being rejected as vague.

Testing done

Added to UDFIndexTest negative tests which ensure that queries which would rely on vague implicit casting fail properly.

Some tests that relied on vague implicit casting may need to be removed, this is worth discussion.

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[ghost]

@confluentinc It looks like @Sullivan-Patrick just signed our Contributor License Agreement. 👍

Always at your service,

clabot

[vcrfxia]

Thanks @Sullivan-Patrick ! LGTM besides the failing unit test and updating the "breaking change" description as we discussed offline. Great stuff.

[JimGalasyn]

LGTM!

[vcrfxia]

Thanks @Sullivan-Patrick . This is great! One minor comment inline.

Also a suggestion to improve the "breaking change" message in your PR description (which is also great, btw): let's clarify what it means for "Functions that relied on vague implicit casting" to "no longer work." To "no longer work" means that any new queries will be rejected, and any existing queries will not be started after an upgrade.

LGTM otherwise!

[vcrfxia]

Thanks @Sullivan-Patrick ! 🎉