Add Support For PostgreSQL Arrays In FromQueryResult Implementation Of JsonValue #1598

anshap1719 · 2023-04-14T03:56:53Z

I wanted to add tests and I tried to as well, but this particular change isn't testable with Mock database because with Mock database, the into_json works as expected even without this change since it directly serializes any rust type into json without matching column type for various backends.

PR Info

Bug Fixes

If a PostgreSQL Model contains array fields, and is serialized as json while querying (using into_json()), the array fields are always skipped. This is because the current implementation of FromQueryResult for JsonValue doesn't handle postgres arrays at all. This PR adds support for the same.

For example, consider the following model (I'm skipping the obvious stuff like derive macros):

pub struct Pizza {
    #[sea_orm(primary_key, auto_increment = false)]
    pub id: i64,
    pub name: String,
    pub toppings: Vec<String>,
}

We also assume that the toppings field is not an empty array in the db for any rows.

If we query this model and expect a json result:

let result = pizza::Entity::find().into_json().all(db).await?;

The final JSON output is always:

{
    "id": 1,
    "name": "Pepperoni"
}

but the expected output is:

{
    "id": 1,
    "name": "Pepperoni",
    "toppings": ["Cheese", "Pepperoni"]
}

…f JsonValue

anshap1719 · 2023-04-30T03:59:31Z

Hello guys, is anything lacking in this PR? This is a fix that we need for our project and have to hack around with it right now.

If you feel like something is missing or incorrectly done, let me know.

Sharpiro · 2023-04-30T16:02:29Z

root json arrays would be very helpful to me as well, thanks @anshap1719

billy1624

Hey @anshap1719, thanks for the great PR!! Sorry for the delay.

I just refactored it: 75ee167
And make sure it only implement Vec<T> if and only if postgres-array feature is enabled: 0163814
Please check :P

anshap1719 · 2023-05-03T13:59:05Z

@billy1624 Thanks for the updates. Didn't realise that I could test it like you added. Thanks!

billy1624

Thanks!! @anshap1719

anshap1719 · 2023-05-25T06:53:19Z

HI @tyt2y3, can you please review this PR? Thanks.

billy1624 · 2023-05-31T09:43:59Z

@tyt2y3 wanna take a look?

anshap1719 · 2023-06-12T07:59:49Z

@billy1624 @tyt2y3 Guys?

billy1624 · 2023-06-12T09:36:26Z

Hey @anshap1719, sorry, we will revisit this tonight :)

anshap1719 · 2023-06-22T11:56:04Z

@billy1624 Any updates?

tyt2y3 · 2023-06-22T22:03:15Z

For some reason it missed my inbox. Sorry for that. However this seems to have touched a deep and quite difficult part of the library.
Need a fresh pair of eyes to review. Even though it looks promising at first glance, concerning the impact I can't make an approval right now.

It might help if you can write a dissection on the implementation.

anshap1719 · 2023-06-23T18:02:48Z

@tyt2y3 Totally understandable. Would it be okay if I just add comments to changes in the "Files changed" view?

tyt2y3 · 2023-06-26T18:36:19Z

@tyt2y3 Totally understandable. Would it be okay if I just add comments to changes in the "Files changed" view?

Sure, that'd be appreciated.

anshap1719 · 2023-06-29T01:11:33Z

src/query/json.rs

                for column in row.columns() {
                    let col = if !column.name().starts_with(pre) {
                        continue;
                    } else {
                        column.name().replacen(pre, "", 1)
                    };
                    let col_type = column.type_info();
+
                    macro_rules! match_postgres_type {


The macro is used for matching primitives against column types, but doesn't consider the fact that postgres columns can be of type array (of a primitive).

anshap1719 · 2023-06-29T01:12:57Z

src/query/json.rs

                    macro_rules! match_postgres_type {
                        ( $type: ty ) => {
-                            if <$type as Type<Postgres>>::type_info().eq(col_type) {
-                                try_get_type!($type, col)
+                            match col_type.kind() {


We try to conditionally match if the column is of type array, or not.

anshap1719 · 2023-06-29T01:13:53Z

src/query/json.rs

+                            match col_type.kind() {
+                                #[cfg(feature = "postgres-array")]
+                                sqlx::postgres::PgTypeKind::Array(_) => {
+                                    if <Vec<$type> as Type<Postgres>>::type_info().eq(col_type) {


If column is indeed of type array, and postgres-array feature is enabled, match the column type against Vec<primitive> instead.

anshap1719 · 2023-06-29T01:14:22Z

src/query/json.rs

+                                        try_get_type!(Vec<$type>, col);
+                                    }
+                                }
+                                _ => {


Otherwise, match column with given primitive type (original logic)

anshap1719 · 2023-06-29T01:14:59Z

src/query/json.rs

@@ -126,9 +139,15 @@ impl FromQueryResult for JsonValue {
                    match_postgres_type!(rust_decimal::Decimal);
                    #[cfg(feature = "with-json")]
                    try_get_type!(serde_json::Value, col);
+                    #[cfg(all(feature = "with-json", feature = "postgres-array"))]
+                    try_get_type!(Vec<serde_json::Value>, col);


Since postgres also supports arrays of json or jsonb, we need to add explicit matching for it in addition to primitive arrays.

anshap1719 · 2023-06-29T01:15:26Z

src/query/json.rs

                    try_get_type!(String, col);
+                    #[cfg(feature = "postgres-array")]
+                    try_get_type!(Vec<String>, col);


Add explicit check for String array, since it's not a primitive

anshap1719 · 2023-06-29T01:15:34Z

src/query/json.rs

                    #[cfg(feature = "with-uuid")]
                    try_get_type!(uuid::Uuid, col);
+                    #[cfg(all(feature = "with-uuid", feature = "postgres-array"))]
+                    try_get_type!(Vec<uuid::Uuid>, col);


Add explicit check for UUID array, since it's not a primitive

anshap1719 · 2023-06-29T01:16:57Z

@tyt2y3 Done. Hope that helps.

anshap1719 · 2023-07-11T04:24:07Z

@tyt2y3 Did you get a chance to review this again yet?

anshap1719 · 2023-07-11T04:24:37Z

@tyt2y3 Sorry, I accidentally hit "Close with comment" for my last comment 😅

anshap1719 · 2023-07-20T08:16:03Z

@tyt2y3 Any updates on this one? Would really appreciate it if we can get this into the next release whenever that happens.

anshap1719 · 2023-07-31T10:58:17Z

@billy1624 @tyt2y3 Guys?

tyt2y3 · 2023-07-31T19:35:29Z

Got you. I'll put this on my pipeline

anshap1719 · 2023-07-31T22:15:06Z

Thanks @tyt2y3

tyt2y3

Nicely done!

tyt2y3 · 2023-08-01T18:42:02Z

Thank you for your patience.

anshap1719 · 2023-08-02T04:45:16Z

Awesome, thanks for your efforts as well @billy1624 that improved the PR.

github-actions · 2023-08-04T21:40:30Z

🎉 Released In 0.12.2 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

anshap1719 added 2 commits April 14, 2023 03:42

Add Support For PostgreSQL Arrays In FromQueryResult Implementation O…

6a1b14b

…f JsonValue

Add support for root arrays in JSON in SeaORM #1517

7304c0f

billy1624 added 3 commits May 3, 2023 16:02

Refactoring

75ee167

Only when postgres-array is enabled

0163814

Add test cases

d426576

billy1624 assigned anshap1719 May 3, 2023

billy1624 reviewed May 3, 2023

View reviewed changes

Merge branch 'master' into pr/1598

93229ce

billy1624 requested a review from tyt2y3 May 3, 2023 08:47

billy1624 approved these changes May 12, 2023

View reviewed changes

Merge branch 'SeaQL:master' into master

391780b

Merge branch 'SeaQL:master' into master

6fe62fd

anshap1719 commented Jun 29, 2023

View reviewed changes

Merge branch 'SeaQL:master' into master

fe69df0

anshap1719 closed this Jul 11, 2023

anshap1719 reopened this Jul 11, 2023

tyt2y3 approved these changes Aug 1, 2023

View reviewed changes

tyt2y3 merged commit 5fa2c1c into SeaQL:master Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support For PostgreSQL Arrays In FromQueryResult Implementation Of JsonValue #1598

Add Support For PostgreSQL Arrays In FromQueryResult Implementation Of JsonValue #1598

anshap1719 commented Apr 14, 2023 •

edited

Loading

anshap1719 commented Apr 30, 2023 •

edited by tyt2y3

Loading

Sharpiro commented Apr 30, 2023

billy1624 left a comment

anshap1719 commented May 3, 2023

billy1624 left a comment

anshap1719 commented May 25, 2023

billy1624 commented May 31, 2023

anshap1719 commented Jun 12, 2023

billy1624 commented Jun 12, 2023

anshap1719 commented Jun 22, 2023

tyt2y3 commented Jun 22, 2023 •

edited

Loading

anshap1719 commented Jun 23, 2023

tyt2y3 commented Jun 26, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 Jun 29, 2023

anshap1719 commented Jun 29, 2023

anshap1719 commented Jul 11, 2023

anshap1719 commented Jul 11, 2023

anshap1719 commented Jul 20, 2023

anshap1719 commented Jul 31, 2023

tyt2y3 commented Jul 31, 2023

anshap1719 commented Jul 31, 2023

tyt2y3 left a comment

tyt2y3 commented Aug 1, 2023

anshap1719 commented Aug 2, 2023

github-actions bot commented Aug 4, 2023

Add Support For PostgreSQL Arrays In FromQueryResult Implementation Of JsonValue #1598

Add Support For PostgreSQL Arrays In FromQueryResult Implementation Of JsonValue #1598

Conversation

anshap1719 commented Apr 14, 2023 • edited Loading

PR Info

Bug Fixes

anshap1719 commented Apr 30, 2023 • edited by tyt2y3 Loading

Sharpiro commented Apr 30, 2023

billy1624 left a comment

Choose a reason for hiding this comment

anshap1719 commented May 3, 2023

billy1624 left a comment

Choose a reason for hiding this comment

anshap1719 commented May 25, 2023

billy1624 commented May 31, 2023

anshap1719 commented Jun 12, 2023

billy1624 commented Jun 12, 2023

anshap1719 commented Jun 22, 2023

tyt2y3 commented Jun 22, 2023 • edited Loading

anshap1719 commented Jun 23, 2023

tyt2y3 commented Jun 26, 2023

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 Jun 29, 2023

Choose a reason for hiding this comment

anshap1719 commented Jun 29, 2023

anshap1719 commented Jul 11, 2023

anshap1719 commented Jul 11, 2023

anshap1719 commented Jul 20, 2023

anshap1719 commented Jul 31, 2023

tyt2y3 commented Jul 31, 2023

anshap1719 commented Jul 31, 2023

tyt2y3 left a comment

Choose a reason for hiding this comment

tyt2y3 commented Aug 1, 2023

anshap1719 commented Aug 2, 2023

github-actions bot commented Aug 4, 2023

🎉 Released In 0.12.2 🎉

anshap1719 commented Apr 14, 2023 •

edited

Loading

anshap1719 commented Apr 30, 2023 •

edited by tyt2y3

Loading

tyt2y3 commented Jun 22, 2023 •

edited

Loading