-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
sql: allow implicit casting of string to more array types #54709
Conversation
e9fdecb
to
34a84d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some logic tests as part of this change? I see there's an array
logic test file, which may be a good place?
Change looks good to me, though it would be nice to get a quick look from @otan as well as he's way more familiar with this stuff.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @aadityasondhi, @arulajmani, and @solongordon)
34a84d7
to
41b8bbe
Compare
added the logic test. @otan would you like to look at this? no need to if you're busy, i feel comfortable going ahead as is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fine to me, curious why we can't do the others
ah you mean arrays of other types? we could -- i was just being lazy and sort of deferring to the types that were already tested in constant_test.go. well technically, we don't support |
happy for that to be a separate PR. just wondering where the line stops :P |
41b8bbe
to
0508beb
Compare
It was easy enough to add FloatArray, INetArray, and VarbitArray to this PR. unfortunately I had trouble with Box2d, Geometry, and Geography arrays, so not including them here. |
TFTRs! i'm merging, and will backport to 20.2 after the initial 20.2.0 release goes out bors r=otan,arulajmani |
bors r- fixing formatting in the logic test |
Canceled. |
Release note (sql change): A string literal like '{X, Y, Z}' is now automatically casted to an array when appropriate. Support is added for UUID, Date, Bool, Time, TimeTZ, Timestamp, TimestampTZ, Float, INet, Varbit and Interval arrays. (Int and Decimal were already supported.)
0508beb
to
802b8a2
Compare
bors r=otan,arulajmani |
Build failed (retrying...): |
Build succeeded: |
Release note (sql change): A string literal like '{X, Y, Z}'
is now automatically casted to an array when appropriate. Support is
added for UUID, Date, Bool, Time, TimeTZ, Timestamp, TimestampTZ, and
Interval arrays. (Int and Decimal were already supported.)
fixes #54672