ksql
ksql copied to clipboard
Handling of parameterized complex types in arguments to UDFs
Fixes #2085.
Description
Formerly, it was not possible to load variants of the same UDF that only differ in the type parameter of the complex argument type, e.g., f(List<String> list)
and f(List<Long> list)
. This was due to that the class FunctionParameter
, which encapsulates an argument of a UDF, only stored the type attribute of the argument schema and not the whole schema, and thus ignoring the type parameters of Maps and Arrays.
The bugfix changes FunctionParameter
to store the whole schema of the argument type and modifies its methods to take into account type parameters of Maps and Arrays.
Moreover, the toString method of KsqlFunction
now uses SchemaUtil::getSqlTypeName
to stringify argument types so that type parameters of Maps and Arrays are no longer omitted.
Testing done
All unit and integration tests of KSQL succeed. I tested the UdfFactory being able to distinguish between type parameters with a UDF in the manner described in #2085.
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 #
")
It looks like @anekdoti hasn't signed our Contributor License Agreement, yet.
The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia
You can read and sign our full Contributor License Agreement here.
Once you've signed reply with [clabot:check]
to prove it.
Appreciation of efforts,
clabot
It looks like @anekdoti hasn't signed our Contributor License Agreement, yet.
The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia
You can read and sign our full Contributor License Agreement here.
Once you've signed reply with
[clabot:check]
to prove it.Appreciation of efforts,
clabot
[clabot:check]
@confluentinc It looks like @anekdoti just signed our Contributor License Agreement. :+1:
Always at your service,
clabot
Hey @anekdoti, thanks for raising the PR!
Are you able to add some test cases to this please?
Hello big-andy-coates,
thanks for taking a look at my PR! I just added two test cases which show that functions that differ in the type parameter of their complex argument type do not raise an exception anymore.
Hello @dguy and @big-andy-coates ,
is there something I can do to bring my pull request forward?
All the best, anekdoti
Hi @anekdoti, sorry about the delay. I think my previous comment still stands, i.e., I think we need some tests to prove that the correct functions are invoked when we have a UDF with parameterized functions.
Hi @dguy,
thanks for your reply. Your comment makes it clearer which tests are still missing - I will do provide them in the following days.
I believe this is no longer a problem after #2595, which introduced the new UdfIndex that takes into account the argument schema.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.
:x: dguy
:x: anekdoti
You have signed the CLA already but the status is still pending? Let us recheck it.