Add the row_to_json Method for diesel postgres #4303
Conversation
PratikFandade
changed the title
| impl RecordOrNullableRecord for Record<Text> {} | ||
| impl RecordOrNullableRecord for Record<Nullable<Text>> {} | ||
| impl RecordOrNullableRecord for Nullable<Record<Text>> {} | ||
| impl RecordOrNullableRecord for Nullable<Record<Nullable<Text>>> {} |
There was a problem hiding this comment.
Record is a somewhat especial type, in so far that it always accepts a tuple as generic argument. So something like Record<(Integer, Text)> or Record<(Text, Date, Integer)>.
The right variant here would be to write something like impl<T> RecordOrNullableRecord for Record<T> {} and impl<T> RecordOrNullableRecord for Nullable<Record<T>> {}
There was a problem hiding this comment.
Thanks @weiznich , I'll make these changes
| /// # } | ||
| /// ``` | ||
| #[sql_name = "jsonb_object"] | ||
| fn row_to_json<R: RecordOrNullableRecord + SingleValue + CombinedNullableValue<Record<Text>, Jsonb>>(record: R) -> R::Out; |
There was a problem hiding this comment.
I fear this function is somewhat problematic to implement with the current diesel types setup. It doesn't accept anonymous record types (as represented by the diesel Record type), but only rows with names. The later is something that's not yet expressible with diesels type setup.
Given that I suppose this won't work in that way :(
(I also do not have any real suggestion how to make it easily work)
There was a problem hiding this comment.
I'll modify the function as pointed by @meeshal to make it correct
diesel_derives/tests/auto_type.rs
Outdated
| @@ -52,7 +52,8 @@ table! { | |||
| multirange -> Multirange<Integer>, | |||
| timestamptz -> Timestamptz, | |||
| name -> Text, | |||
| text_array -> Array<Text> | |||
| text_array -> Array<Text>, | |||
| record -> Record, | |||
There was a problem hiding this comment.
We would need to define the record type here as Record<(Integer, Text)> or something like that to fix the compiler error.
There was a problem hiding this comment.
Sure, I'll make the changes
PratikFandade
force-pushed
the
pg_row_to_json
branch
from
e03076d
to
b86dc5a
Compare
| #[diagnostic::on_unimplemented( | ||
| message = "`{Self}` is neither `Record<Text>`, `Record<Nullable<Text>>`,\ | ||
| `Nullable<Record<Text>>` nor `diesel::sql_types::Nullable<Record<Nullable<Text>>>`", | ||
| note = "try to provide an expression that produces one of the expected sql types" | ||
| )] |
There was a problem hiding this comment.
The reference to Text fragments here is outdated
There was a problem hiding this comment.
Opps! My bad, guess I missed to unstash some changes, I'll fix this.
| /// # let connection = &mut establish_connection(); | ||
| /// | ||
| /// // Example SQL query selecting a custom record type | ||
| /// let query = diesel::select(sql::<Record<(Text, Integer)>>("ROW('John', 30)::record")); |
There was a problem hiding this comment.
Imho, row_to_json should be used here instead of sql so, the returned value could be casted to json right away.
There was a problem hiding this comment.
I'm so sorry, will fix this test
| /// # } | ||
| /// ``` | ||
| #[sql_name = "row_to_json"] | ||
| fn row_to_json<R: RecordOrNullableRecord + SingleValue + MaybeNullableValue<Json>>(record: R) -> R::Out; |
There was a problem hiding this comment.
SingleValue seems to be redundant as it is already implemented by MaybeNullableValue.
There was a problem hiding this comment.
Thanks for the insight will make these changes
weiznich
force-pushed
the
pg_row_to_json
branch
from
8d75262
to
8ce5b1b
Compare
Adding the support as specified in #4216
Not sure if everything correct, still new to rust, let me know if you need any other changes.