SeaQL / seaography

🧭 GraphQL framework for SeaORM
Apache License 2.0
380 stars 35 forks source link

Add Support For Postgres Vec<T> #159

Closed jsievenpiper closed 9 months ago

jsievenpiper commented 9 months ago

This has been an awesome project to use and I was hoping to give a little bit back to it!

PR Info

New Features

Adds support for arbitrarily-nested Vec types. Makes some assumptions about nullability in cases where the underlying database doesn't have nested nullability concerns. There's also some limitations in how nullability is held at the sea_query level that doesn't directly map to GraphQL's granularity. I'm happy for those assumptions to be changed to whatever the team supporting SeaORM/Seaography thinks makes the most sense.

Based on #128, I'm not certain every possible use case is covered (e.g. filtering on the data in the vec types), but figured they are at least queryable and renderable now, which is a good start!


You can now have an Entity like so:

#[sea_orm(table_name = "cake")]
pub struct Model {
    #[sea_orm(primary_key, auto_increment = false)]
    #[serde(skip_deserializing)]
    pub id: Uuid,
    #[serde(skip_deserializing)]
    pub flavors: Vec<String>,
}

This will become the GraphQL type: [String!]!. Comments on nullability above, but right now it only applies to the Vec itself (e.g. a nullable column would result in [String!] because we don't have insight into whether the inner String can be null or not).

And you can query it:

query {
  cake {
    nodes {
      flavors
    }
  }
}

And get it back:

[
  {
    "flavors": [
      "strawberry",
      "chocolate"
    ]
  }
]

This works for any allowed nested type defined by the sea_query ColumnType that is also supported by Seaography, not just strings!

Bug Fixes

Breaking Changes

Changes

Did some light refactoring to pull duplication down in the files I mucked around in.

jsievenpiper commented 9 months ago

@karatakis would love your review on this whenever you're able -- thank you for all the work you've put into this project!

karatakis commented 9 months ago

Thanks, @jsievenpiper, for taking the time to commit to our project. I will review your code, propose solutions, and determine if it meets our standards. If everything is okay, I will continue to merge it :smile:

karatakis commented 9 months ago

@jsievenpiper, everything looks okay. It's a pass from me if the tests are green. Please take some care to cover the clippy and fmt issues so I can contribute to merging it. Regarding your thoughts about the nested arrays, they are valid but can be implemented in a separate issue.

PS: rebase with main branch, as there are some tests breaking in pgsql that is not your fault.

jsievenpiper commented 9 months ago

@karatakis rebased and fmt/clipped! If there are other test failures after that rebase I'm happy to take a look into them!

jsievenpiper commented 9 months ago

I'll take a look at the test suite and see what needs adjusting.

karatakis commented 9 months ago

@jsievenpiper, it causes a bit of disturbance on the integration tests. I must determine if this behavior is expected or not to modify the tests.

jsievenpiper commented 9 months ago

@karatakis took a look at the integration tests and found a small bug in the PR for mutations -- I ran the postgres suite manually and fixed the PR. I think it should fix the other backends as well.

No test suite changes should be needed!

karatakis commented 9 months ago

@jsievenpiper, thanks for the work. Your contributions are significant and will be shipped with the 1.0.0 version.

jsievenpiper commented 9 months ago

@karatakis my pleasure! Thank you for all for creating such a great project!