fix: Add Int32 type override for Dialects #12916
Conversation
| } | ||
|
|
||
| fn int32_cast_dtype(&self) -> ast::DataType { | ||
| ast::DataType::Custom(ObjectName(vec![Ident::new("SIGNED")]), vec![]) |
There was a problem hiding this comment.
MySQL integer is signed by default
Also where do we specify it's integer and not eg bigint or tinyint?
https://dev.mysql.com/doc/refman/8.4/en/integer-types.html
There was a problem hiding this comment.
perhaps SIGNED INTEGER would work (it does in my MySQL)
There was a problem hiding this comment.
MySQL integer is signed by default
I'm not sure what you mean by this. There's already a rule to rewrite BIGINT as SIGNED, but not for INTEGER. On the current version, it generates CAST(.. INTEGER) which isn't supported.
Also where do we specify it's integer and not eg bigint or tinyint?
you cannot specify the size of an integer in cast with MySQL - it always casts to BIGINT. SIGNED INTEGER does work, but it's syntactic sugar and is identical to SIGNED.
https://dev.mysql.com/doc/refman/8.4/en/cast-functions.html#function_cast
There was a problem hiding this comment.
Maybe @phillipleblanc or @sgrebnov can offer some help here
There was a problem hiding this comment.
The change LGTM
- MySQL
castdoes not supportIntegerorBigIntwhich are currently used by default to unparse Int32 and Int64 unparsing. List of supported types for MySQL cast:
https://dev.mysql.com/doc/refman/8.4/en/cast-functions.html#function_cast - So this PR fallbacks to using
SIGNED [INTEGER]similar toint64_cast_dtypewhich will produce a signed BIGINT. I prefer justSIGNEDto be consistent withint64_cast_dtype(or update both)
Possible alternative is to see if we can get rid of casting at all when it is unnecessary as Int32 cast intention and BigInt are different
4 participants
Which issue does this PR close?
Closes #12915.
Rationale for this change
What changes are included in this PR?
Dialectto specify the cast type for Int32 integers, replicating the existing behavior for Int64 integers.Are these changes tested?
Yes.
Are there any user-facing changes?
No.