issues-336 Third try impl postgres-array #467

ikrivosheev · 2022-10-09T16:40:32Z

PR Info

Closes Query binders: Add support for postgres arrays #336

Other PR:

This is the third try to impl postgres-array in SeaQuery. Here I store type information into Value and then try to convert Vec<Value> into Vec<T>. This look safe.

Adds

Support one dimension Postgres Array into sea-query-binder
enum ArrayType with all possible types
Add Value::except method

Breaking Changes

ColumnType::Array store Box<ColumnType> instead string
Value::Array store addition field: ArrayType, where store type of array elements

Changes

Small improvement Iden::to_string

ikrivosheev · 2022-10-09T16:44:23Z

@tyt2y3 @billy1624 can you look next my idea?)

Sytten · 2022-10-09T17:20:39Z

For me any solution that cant be extended to support custom types is a no go. That is why the trait based solution is used by all other libraries.

ikrivosheev · 2022-10-09T17:26:48Z

For me any solution that cant be extended to support custom types is a no go. That is why the trait based solution is used by all other libraries.

I try to implement trait solution, but got a lot of problems. I still don't understand how to solve them all...

sea-query-postgres/Cargo.toml

tyt2y3 · 2022-10-10T01:22:31Z

sea-query-binder/Cargo.toml

@@ -17,7 +17,7 @@ rust-version = "1.60"
 [lib]

 [dependencies]
-sea-query = { version = "^0", path = ".." }
+sea-query = { version = "^0", path = "..", features = ["thread-safe"] }


This makes sense

tyt2y3 · 2022-10-10T01:36:48Z

This looks good, especially as an intermediate step. The crate architecture has been revamped in 0.27 that is waiting for a release. What's important now is to roll out the next iteration of SeaORM. And maybe I can give a try on ValueTrait sometime after that.

ikrivosheev · 2022-10-10T17:38:21Z

@tyt2y3 @billy1624 @Sytten hello! I have done with some part of this PR.

Create new enum: ArrayType
Add support for Value::Array for sqlx/postgres
Extend trait ValueType: fn array_type() -> ArrayType and fn except(v: Value, msg: &str) -> Self

TODO:

Add support for Vec<Option<T>> - don't know how to do it yet

tyt2y3 · 2022-10-11T06:19:50Z

Great work! I think now I have an idea as to how ValueTrait would work with SQLx, I am willing to give a try after this settled and we released 0.27

tyt2y3 · 2022-10-12T16:57:24Z

This seems to be very nearly done?

tyt2y3 · 2022-10-12T16:59:27Z

@ikrivosheev check this 79625e0 , not sure if it really works at downstream, but it compiles

ikrivosheev · 2022-10-12T17:24:21Z

This seems to be very nearly done?

I want to add support for Vec<Option<T>>

…_v3' into feature/issues-336_array_support_v3

ikrivosheev · 2022-10-12T17:46:45Z

This seems to be very nearly done?

I want to add support for Vec<Option<T>>

Or later...

@billy1624 @tyt2y3 I updated the description. Can you review the PR?

tyt2y3 · 2022-10-13T16:43:15Z

sea-query-binder/Cargo.toml

 sqlx = { version = "^0.6.1", optional = true }
+serde_json = { version = "^1", optional = true }
+chrono = { version = "^0.4", default-features = false, features = ["clock"], optional = true }


can we pub all these from a special export module in sea-query, then we don't have to specify the dependency twice?

I can imagine the two Cargo.toml might go out of sync in the future.

We want to make our third-party libraries as part of public interface?

tyt2y3 · 2022-10-14T06:08:04Z

Cool enough. @billy1624 thoughts?

billy1624

Wow!! That's smart!

billy1624 · 2022-10-14T09:33:32Z

src/value.rs

+ fn except(v: Value, msg: &str) -> Self {
+ Self::try_from(v).expect(msg)
+ }


Would it be better to rename it as expect? Since it's a wrapper method of try_from then expect.

This also align with the API of... few lines above it

fn unwrap(v: Value) -> Self { Self::try_from(v).unwrap() }

billy1624 · 2022-10-14T09:34:02Z

src/value.rs

+ pub fn except<T>(self, msg: &str) -> T
+ where
+ T: ValueType,
+ {
+ T::except(self, msg)
+ }


Same here :)

billy1624 · 2022-10-14T09:58:02Z

sea-query-binder/src/sqlx_postgres.rs

+ ArrayType::Bool => {
+ let value: Option<Vec<bool>> =
+ Value::Array(ty, v).except("Invalid type for array value");
+ args.add(value)
+ }


I think ordinary user won't see this message (unless someone intentionally construct a vector of boxed value with different value variants in it) but the error message itself can be more specific.

Value::Array(ty, v).except("This Value::Array should consist of Value::Bool");

billy1624 · 2022-10-14T10:09:44Z

src/value.rs

@@ -222,7 +323,7 @@ macro_rules! type_to_value {
 }

 macro_rules! type_to_box_value {
- ( $type: ty, $name: ident, $col_type: expr ) => {
+ ( $type: ty, $name: ident, $col_type: expr, $array_type: expr ) => {


Nitpick: a trivial refactoring

Reuse $name ident in type_to_box_value macro for ArrayType variant ikrivosheev/sea-query#1

billy1624 · 2022-10-14T10:13:08Z

Cool enough. @billy1624 thoughts?

Looks good to me except comments above :D

Reuse $name ident in type_to_box_value macro for ArrayType variant

ikrivosheev · 2022-10-14T11:16:18Z

@billy1624 thank you for review! I corrected all comments

billy1624 · 2022-10-15T14:50:19Z

src/value.rs

@@ -185,6 +274,13 @@ impl Value {
 {
 T::unwrap(self)
 }
+
+ pub fn except<T>(self, msg: &str) -> T


Are we keeping this as except?

Thank you! Fix

tyt2y3 · 2022-10-16T07:58:26Z

One last thing, can we also update the sqlx-postgres example (with several Array types) and make it part of our CI?

ikrivosheev mentioned this pull request Oct 9, 2022

issues-336 Init array support #390

Closed

10 tasks

tyt2y3 reviewed Oct 10, 2022

View reviewed changes

sea-query-postgres/Cargo.toml Outdated Show resolved Hide resolved

tyt2y3 reviewed Oct 10, 2022

View reviewed changes

issues-336 Third try impl postgres-array

ac92bd1

ikrivosheev force-pushed the feature/issues-336_array_support_v3 branch from 835707e to ac92bd1 Compare October 10, 2022 17:31

issues-336 Fix compile errors

a2931d8

ikrivosheev requested review from tyt2y3 and billy1624 October 10, 2022 17:38

ikrivosheev added 3 commits October 10, 2022 20:40

issues-336 Fix sqlx_postgres.rs

f554714

issues-336 Fix compile error

f32f6c8

issues-336 Fix example

8154a5a

Merge branch 'master' into feature/issues-336_array_support_v3

010af8a

ikrivosheev added 2 commits October 12, 2022 20:45

issues-336 Add small documentation

4cd974b

Merge remote-tracking branch 'origin/feature/issues-336_array_support…

fa26dad

…_v3' into feature/issues-336_array_support_v3

ikrivosheev marked this pull request as ready for review October 12, 2022 17:46

tyt2y3 reviewed Oct 13, 2022

View reviewed changes

billy1624 reviewed Oct 14, 2022

View reviewed changes

Reuse $name ident in type_to_box_value macro for ArrayType variant

9335b75

billy1624 reviewed Oct 14, 2022

View reviewed changes

ikrivosheev added 2 commits October 14, 2022 14:07

Merge pull request #1 from billy1624/pr/467

21f115e

Reuse $name ident in type_to_box_value macro for ArrayType variant

issues-336 Fix after review

d5424aa

issues-336 Fix macros

b882d86

ikrivosheev requested review from tyt2y3 and billy1624 October 14, 2022 11:29

ikrivosheev self-assigned this Oct 14, 2022

billy1624 reviewed Oct 15, 2022

View reviewed changes

issues-336 Rename function

7bba3e2

tyt2y3 merged commit 9c2a6b2 into SeaQL:master Oct 16, 2022

ikrivosheev deleted the feature/issues-336_array_support_v3 branch October 16, 2022 08:33

kudlatyamroth mentioned this pull request Oct 16, 2022

Be able to support Array in database SeaQL/sea-orm#576

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issues-336 Third try impl postgres-array #467

issues-336 Third try impl postgres-array #467

ikrivosheev commented Oct 9, 2022 •

edited

Loading

ikrivosheev commented Oct 9, 2022

Sytten commented Oct 9, 2022

ikrivosheev commented Oct 9, 2022

tyt2y3 Oct 10, 2022

tyt2y3 commented Oct 10, 2022 •

edited

Loading

ikrivosheev commented Oct 10, 2022

tyt2y3 commented Oct 11, 2022

tyt2y3 commented Oct 12, 2022

tyt2y3 commented Oct 12, 2022 •

edited

Loading

ikrivosheev commented Oct 12, 2022 •

edited

Loading

ikrivosheev commented Oct 12, 2022 •

edited

Loading

tyt2y3 Oct 13, 2022 •

edited

Loading

ikrivosheev Oct 14, 2022

tyt2y3 Oct 16, 2022

tyt2y3 commented Oct 14, 2022

billy1624 left a comment

billy1624 Oct 14, 2022

ikrivosheev Oct 14, 2022

billy1624 Oct 14, 2022

ikrivosheev Oct 14, 2022

billy1624 Oct 14, 2022

ikrivosheev Oct 14, 2022

billy1624 Oct 14, 2022 •

edited

Loading

ikrivosheev Oct 14, 2022

billy1624 commented Oct 14, 2022 •

edited

Loading

ikrivosheev commented Oct 14, 2022

billy1624 Oct 15, 2022

ikrivosheev Oct 15, 2022

tyt2y3 commented Oct 16, 2022

issues-336 Third try impl postgres-array #467

issues-336 Third try impl postgres-array #467

Conversation

ikrivosheev commented Oct 9, 2022 • edited Loading

PR Info

Adds

Breaking Changes

Changes

ikrivosheev commented Oct 9, 2022

Sytten commented Oct 9, 2022

ikrivosheev commented Oct 9, 2022

Choose a reason for hiding this comment

tyt2y3 commented Oct 10, 2022 • edited Loading

ikrivosheev commented Oct 10, 2022

tyt2y3 commented Oct 11, 2022

tyt2y3 commented Oct 12, 2022

tyt2y3 commented Oct 12, 2022 • edited Loading

ikrivosheev commented Oct 12, 2022 • edited Loading

ikrivosheev commented Oct 12, 2022 • edited Loading

tyt2y3 Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyt2y3 commented Oct 14, 2022

billy1624 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

billy1624 Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

billy1624 commented Oct 14, 2022 • edited Loading

ikrivosheev commented Oct 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyt2y3 commented Oct 16, 2022

ikrivosheev commented Oct 9, 2022 •

edited

Loading

tyt2y3 commented Oct 10, 2022 •

edited

Loading

tyt2y3 commented Oct 12, 2022 •

edited

Loading

ikrivosheev commented Oct 12, 2022 •

edited

Loading

ikrivosheev commented Oct 12, 2022 •

edited

Loading

tyt2y3 Oct 13, 2022 •

edited

Loading

billy1624 Oct 14, 2022 •

edited

Loading

billy1624 commented Oct 14, 2022 •

edited

Loading