Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

functions params should be implicitly cast to wider type #3791

Closed
big-andy-coates opened this issue Nov 7, 2019 · 0 comments · Fixed by #4406
Closed

functions params should be implicitly cast to wider type #3791

big-andy-coates opened this issue Nov 7, 2019 · 0 comments · Fixed by #4406

Comments

@big-andy-coates
Copy link
Contributor

KSQL has rules for what types can be implicitly converted to a wider type, e.g INT can be implicitly converted to BIGINT. These rules are encapsulated in SqlBaseType.canImplicitlyCast and used by DefaultSqlValueCoercer.

Rules are that the following implicit casts are possible:

INT -> BIGINT -> DECIMAL -> DOUBLE.

(Yes, the DECIMAL -> DOUBLE is potentially lossy, but that's outside the scope of this issue).

The function framework does not support implicitly casting a parameter to a wider type. For example, the exp function can take a DOUBLE but there is not a variant that takes a DECIMAL. At the moment is you try and call exp with a DECIMAL it fails with an error. What is should do is use the implicit casting rules to widen the DECIMAL to a DOUBLE and call the double version.

Note: it looks like at some point that GenericRowValueTypeEnforcer might have done this conversion, though it doesn't enforce the same rules on what can be implicitly cast. Also, I'm about to remove it.

big-andy-coates added a commit to big-andy-coates/ksql that referenced this issue Nov 7, 2019
This change sees `GenericRowValueTypeEnforcer` removed from the code base.

It's being removed as the functionality it provides is:

1. Not needed in the production code path as values are always of the right type.
1. Only needed for a few tests to pass, (which have been fixed up).
1. It supports coercing between types which are outside of KSQLs implicit casting rules, e.g. coerce String -> BIGINT.

We should support implicitly casting parameters to functions to wider types,
as defined by KSQLs implicit casting rules. However, this is not currently the
case. See confluentinc#3791.
big-andy-coates added a commit that referenced this issue Nov 7, 2019
This change sees `GenericRowValueTypeEnforcer` removed from the code base.

It's being removed as the functionality it provides is:

1. Not needed in the production code path as values are always of the right type.
1. Only needed for a few tests to pass, (which have been fixed up).
1. It supports coercing between types which are outside of KSQLs implicit casting rules, e.g. coerce String -> BIGINT.

We should support implicitly casting parameters to functions to wider types,
as defined by KSQLs implicit casting rules. However, this is not currently the
case. See #3791.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants