Skip to content
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

Implemented 'Array' type for Postgres. #205

Merged
merged 3 commits into from
Jan 2, 2022
Merged

Conversation

kev0960
Copy link
Contributor

@kev0960 kev0960 commented Dec 13, 2021

This introduces "Array" as a new Value variant.

It can be derived from all Vec types, where T is a valid Value, except for u8, which is already handled by Bytes.

Note that sqlx and postgres-types already supports an array, so this can be implemented without too much hassle.

This introduces "Array" as a new Value variant.
It can be derived from all Vec<T> types, where T is a valid Value,
   except for u8, which is already handled by Bytes.
@kev0960
Copy link
Contributor Author

kev0960 commented Dec 13, 2021

Hmm, actually, this will not work because we cannot provide bindings to Sqlx.

Sqlx expects Vec<T> for an array, and we have to somehow convert Vec<Value> to Vec<T>. :(

@billy1624
Copy link
Member

billy1624 commented Dec 13, 2021

Hey @kev0960, thanks for the contributions! I assumed you want to support Vec<T> in sea-orm for Postgres?

@kev0960
Copy link
Contributor Author

kev0960 commented Dec 13, 2021

Yes I do :)

@kev0960
Copy link
Contributor Author

kev0960 commented Dec 13, 2021

I don't like the idea but we can probably workaround by defining bunch of definitions like

ArrayInt32(Option<Vec<i32>>),
ArrayInt64(Option<Vec<i64>>),
ArrayString(Option<Vec<String>>),
...

and treat them individually. I really cant think of the "clean" way to convert the generic Array(Option<Vec<Value>>) into Vec<ConcreteType> since "Value" does not really carry the type we have to convert to.

Any ideas are welcomed :)

@billy1624
Copy link
Member

Let me think loll :P


#[cfg(feature = "with-array")]
#[cfg_attr(docsrs, doc(cfg(feature = "with-array")))]
Array(Option<Box<Vec<Value>>>),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think a solution like this will work out:

pub enum Value<A> {

and then this becomes:

    Array(Option<Box<Vec<A>>>),

We would have to convert the other code to account for the generic argument, but I'm thinking if such a approach will work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub enum Value<A> {

So, A is unbounded? Why is this necessary?

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply. Actually, this looks very good!

@tyt2y3 tyt2y3 merged commit 04c8712 into SeaQL:master Jan 2, 2022
@billy1624
Copy link
Member

Hey @tyt2y3, I think something is missing in this PR and it shouldn't be merged into master yet.

Binding array values to SQLx PostgreSQL is not ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants