Feature parametric types #4005
Feature parametric types #4005
Conversation
|
I am not entirely sure if that's right direction. It seems like having two separate lists for literal parameters and type parameters was simpler from code's perspective. Do we really want to unify both of those things? Without unifying we are losing the way to distinguish between two types |
|
some comments |
pnowojski
force-pushed
the
feature_parametric-types
branch
from
e693590
to
6036a80
Compare
pnowojski
force-pushed
the
feature_parametric-types
branch
from
6036a80
to
ae6cf2c
Compare
|
@martint I have changed implementation of It would be best to minimize changes to commits from this pull request, since there is a lot of more code that depends on those three commits (rebased Dain's VARCHAR changes + decimal changes). We still need to decide at least what to do with backward compatibility with previous clients. As a future refactor somewhere down the road, we should indeed do something about redundancy between TypeSignatureParameter, TypeParameter and ClientTypeSignatureParameter classes. Do we need ClientTypeSignature at all? It predates dependency of presto-client to presto-spi, so it might HAD some sense long time ago, but at this moment, I don't see a point of having TypeSignature and ClientTypeSignature? |
- generalized parametric types - deprecated literalParameters - added parser support for BIGINT typeParameter - renamed getParameters to getTypeParametersAsTypeSignatures - bindParameters for generalized type signature
|
Great, I'll take a look. I don't think we need ClientTypeSignature any more, by the way. |
pnowojski
force-pushed
the
feature_parametric-types
branch
from
ae6cf2c
to
9c9a102
Compare
| @@ -238,7 +238,7 @@ valueExpression | |||
| primaryExpression | |||
| : NULL #nullLiteral | |||
| | interval #intervalLiteral | |||
| | identifier STRING #typeConstructor | |||
| | type STRING #typeConstructor | |||
There was a problem hiding this comment.
I'm not sure we want to allow arbitrary types for the "type constructor" idiom. I actually think we made a mistake in allowing anything beyond the special cases described in the SQL spec (INTERVAL, DATE, etc).
There was a problem hiding this comment.
Isn't this out of scope of this pull request? Can we create an issue for "disallow arbitrary types in type constructor"? Especially that @losipiuk said that such solution was discussed with you about constructing decimal literals.
What I was suggesting is not that we change what we support today (at least not yet), but to not extend the syntax beyond that. These look weird to me: Is there a use case for specifying the type parameters in these expressions given that it can be inferred from the value? |
|
Superseded by #4264 |
This is part of the work for the following issue: #3938
TODO/Questions: