-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: implement array functions for JSON arrays #81707
Conversation
25491be
to
709a7dd
Compare
Cool stuff! The danger here is about compatibility. If Postgres ends up with a different definition of what these functions do for JSON one day, we'll be between a rock and a hard place: incompatible with Postgres, but unable to change our behavior for fear of breaking existing applications. That said, I'm not sure the risk here is super high. I'm curious to hear from @rafiss. |
709a7dd
to
8640273
Compare
My answer on https://stackoverflow.com/questions/72347170/delete-element-from-jsonb-array-in-cockaroachdb/72351955#72351955 made me sad, and it's not the first time I've had trouble manipulating a JSONB array. There are a few different, non-mutually-exclusive ways we could make this easier. We could allow aggregate(generator) functions in scalar contexts. We could implement more JSON functions as needed. This PR instead just overloads every function that takes a json[] argument (which is a datum type we don't allow in columns) to also be able to take a json array argument, and alters the return type accordingly. It's not particularly performance-optimized but we can replace individual implementations if needed. That said, this is an unauthorized spike, what do you think? Release note (sql change): Built-in array functions array_append, array_prepend, array_cat, array_remove, array_replace, array_position, and array_positions may now be used with jsonb arrays.
8640273
to
2762b63
Compare
I feel like instead of this, we should implement the postgres14 JSON operator support. See https://aaronbos.dev/posts/update-json-postgresql for an example. @mgartner and I discussed this as an area for an intern project. The last thing I want is for us to defer building out the good postgres syntax because we have a just barely good enough workaround to not having it. |
Whoops wrong link: https://aaronbos.dev/posts/postgres-14-json |
Here is the tracking issue for that: https://cockroachlabs.atlassian.net/browse/CRDB-12464 and #77434 I'd also lean a bit more in favor of doing that. I do have a bit of a concern about adding a PG-incompatible JSON builtin, but perhaps it's unfounded |
Could this be a UDF when we have them? |
Oddly enough I don't think existing postgres JSON operators would work for the problem I linked (removing a non-primitive value from a JSON array). I could be missing a trick though. If you wanted, it wouldn't be too hard to change this from programmatically overloading existing array methods to programmatically creating new methods with their own namespace (e.g. |
I disagree that the postgres syntax does not help: create table t (data jsonb);
insert into t values (' {"a": {"b": [10, 20, 30]}}');
update t set data['a']['b'] = data['a']['b'] - 1; select * from t;
{"a": {"b": [10, 30]}} |
You can do that in CRDB today with update t set data = json_set(data, Array['a','b'], json_extract_path(data, 'a','b') - 1); The json operators are just (a lot) prettier. The missing functionality is being able to specify an element you want to remove, rather than an index. In Postgres you can do |
And I don't particularly think we should support a |
Oh, cool, that looks like a more performant way to smuggle a |
Hopefully in 22.2 we can define that logic in a UDF. A lot of the time the answer for why Postgres doesn't support something is that it's not hard to define the function yourself. |
My answer on
https://stackoverflow.com/questions/72347170/delete-element-from-jsonb-array-in-cockaroachdb/72351955#72351955
made me sad, and it's not the first time I've had trouble manipulating a JSONB array.
There are a few different, non-mutually-exclusive ways we could make this easier. We could allow
aggregate(generator) functions in scalar contexts. We could implement more JSON functions as needed.
This PR instead just overloads every function that takes a json[] argument (which is a datum type
we don't allow in columns) to also be able to take a json array argument, and alters the return type
accordingly. It's not particularly performance-optimized but we can replace individual implementations
if needed.
That said, this is an unauthorized spike, what do you think?
Release note (sql change): Built-in array functions array_append, array_prepend, array_cat, array_remove, array_replace, array_position, and array_positions may now be used with jsonb arrays.