-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
implement json_object with separate keys and values arguments #4253
implement json_object with separate keys and values arguments #4253
Conversation
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.
Thanks for open the PR!
The changes look good to me.
@weiznich i came up with this type signature
to fix the auto_type error but i dont know if it's correct or not? |
/// # } | ||
/// ``` | ||
#[sql_name = "json_object"] | ||
fn json_object_with_keys_and_values<K: TextArrayOrNullableTextArray + MaybeNullableValue<Json>, V: TextArrayOrNullableTextArray + MaybeNullableValue<Json>>( |
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.
Unfortunately that's still not 100% correct as it only couples the nullability of the return value onto the first argument, but this function seems to return also Null
if you pass in Null
as second argument. (I'm sorry that null value handling is such a mess. I also feel that postgres doesn't document this in a good fashion at all.)
I cannot say without further experimentation how to solve this. I will update the comment as soon as I know more.
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.
I did write a doc test where the 2nd argument is null
and it returns null
as expected, but you're right that the output is coupled with the first argument only. I couldn't think of a way to couple it with both arguments
c645366
to
ad2fb0e
Compare
@weiznich i made a small change to the signature, take a look now |
ad2fb0e
to
b488df4
Compare
I've pushed b488df4 which introduces a new helper trait that essentially evaluates to |
added support for
json_object(keys[],values[])
under #4216