sql: support binary format for all arrays#23126
Merged
justinj merged 1 commit intocockroachdb:masterfrom Feb 28, 2018
Merged
Conversation
Member
|
This change is |
Contributor
|
Would this work for creating an array type for jsonb columns by any chance? |
nvb
approved these changes
Feb 27, 2018
| "192.168./10", | ||
| } | ||
|
|
||
| var arrayUUIDInputs = []string{ |
Contributor
There was a problem hiding this comment.
Consider adding a test for decimalArrayInputs as well.
Contributor
Author
There was a problem hiding this comment.
Good idea, I just took the tests for decimal and arbitrarily grouped them into arrays.
| subWriter.putInt32(int32(v.Len())) | ||
| subWriter.putInt32(int32(v.Len())) | ||
| // Lower bound, we only support a lower bound of 1. | ||
| subWriter.putInt32(1) |
Contributor
There was a problem hiding this comment.
I'm curious how you caught this. What was failing?
Contributor
Author
There was a problem hiding this comment.
The tests I just added were, haha. But I guess this is a good indicator that nobody really makes use of the lower bound.
d75bc95 to
82c0b42
Compare
Contributor
Author
|
@hjames9 We punted arrays of |
Contributor
|
I have an existing Postgresql schema that stores json obtained from REST
calls in unrelated systems. For instance, like a payment processing system
that calls one service to process a credit card and one to process
inventory moving. The responses back from each system I would like to keep
separate and not try to comingle them into one giant message.
…On Feb 27, 2018 3:32 PM, "Justin Jaffray" ***@***.***> wrote:
@hjames9 <https://github.com/hjames9> We punted arrays of JSONB values
for the meantime because it causes some annoyances with respect to
formatting. Out of curiosity - what do you want them for that you couldn't
just use a JSONB-level array for?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#23126 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEAIVW8NURZMUp5MOweAtGtrF3tVy9Sfks5tZGZRgaJpZM4SUFjr>
.
|
Contributor
|
What about storing the responses in multiple SQL rows instead of an array?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Contributor
|
That's usually the suggestion for avoiding using array types, however I'd
prefer to use the same schema and not make too many code changes if
possible, especially considering that the intention is to support arrays in
general.
…On Feb 27, 2018 4:29 PM, "kena" ***@***.***> wrote:
What about storing the responses in multiple SQL rows instead of an array?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#23126 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEAIVapuETQCkhaZG1FqPNHxbI5zGno8ks5tZHPXgaJpZM4SUFjr>
.
|
Fixes cockroachdb#23063. Fixes cockroachdb#21433. Release note (bug fix): ARRAYs can now be used with the postgres binary format.
82c0b42 to
059ebba
Compare
Contributor
Author
|
TFTRs! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #23063.
Fixes #21433.
More testing probably needed but wanted to get this guy out there, and
see if anyone has any ideas for a nice way to do more large scale
testing than generating test data for every type of array we support?
Release note (bug fix): ARRAYs can now be used with the postgres binary
format.