add partly Mysql support #48

p-alik · 2020-05-15T20:59:23Z

This PR doesn't solve the issue #3 entirely. It doesn't contain implementation of insert mutation for Mysql. Consequently wundergraph_example doesn't support Mysql also

Add corresponding feature flags to the wundergraph_derive, wundergraph and wundergraph_cli crate

diesel limits bei u64::MAX https://github.com/diesel-rs/diesel/blob/968e7f79cf6953eb66ddb4256c1e8d5991dc808d/diesel/src/mysql/query_builder/limit_offset.rs#L57 Since GraphQL doesn't know unsigned types, limit is restricted i64::MAX

…iesel::mysql::Mysql

This requires some manual `HandleInsert` + `HandleBatchInsert` impls because the default provided one only works with integer primary keys, because there is no way to generally receive the last inserted primary key value using mysql. Interestingly we cannot derive `Insertable` then anymore because of an error about conflicting impls with the wild card impl in wundergraph itself. I think that's because rustc will not use associated types to see if an impl actual is conflicting or not.

weiznich

Thanks for working on this 👍

The changes itself are looking fine, but we should find some solution for HandleInsert/HandleBatchInsert. I'm fine with supporting only integer primary keys there by default and requiring manual implementations for everyone else, as long as it is documented properly.
Additionally I think that wundergraph_cli does require some additions to support mysql.

To fix the CI you need to run rustfmt and rebase the change to the latest master (as it contains another CI fix).

wundergraph/src/query_builder/selection/offset.rs

weiznich · 2020-05-16T18:55:51Z

wundergraph/src/query_builder/selection/offset.rs

+            if select.argument("limit").is_some() {
+                Ok(q)
+            } else {
+                Ok(<_ as LimitDsl>::limit(q, std::i64::MAX))


A small comment why this is necessary would be nice, maybe just link the relevant mysql documentation?

I put diesel's comment there

weiznich · 2020-05-16T18:56:33Z

wundergraph_cli/Cargo.toml

@@ -24,5 +24,6 @@ serde_json = "1"

 [features]
 default = ["postgres", "sqlite"]
-sqlite = ["diesel/sqlite"]
+mysql = ["diesel/mysql"]


There are no other changes required to get wundergraph_cli working?

I've to prove it

cargo run --features mysql --no-default-features -- print-schema mysql://**@localhost/wundergraph runs withot any warnings (after e3c50f4)

print_schema::tests::round_trip test requires Mysql insert migration.

print_schema::tests::infer_schema fails with

Snapshot file: wundergraph_cli/src/print_schema/snapshots/tests__infer_schema.snap Snapshot: infer_schema Source: wundergraph_cli/src/print_schema/mod.rs:178 -old snapshot +new results ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── &s ────────────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 2 2 │ use wundergraph::scalar::WundergraphScalarValue; 3 3 │ use wundergraph::WundergraphEntity; 4 4 │ 5 5 │ table! { 6 │- infer_test.comments (id) { 7 │- id -> Int4, 8 │- post -> Nullable<Int4>, 9 │- commenter -> Nullable<Int4>, 6 │+ comments (id) { 7 │+ id -> Integer, 8 │+ post -> Nullable<Integer>, 9 │+ commenter -> Nullable<Integer>, 10 10 │ content -> Text, 11 11 │ } 12 12 │ } 13 13 │ 14 14 │ table! { 15 │- infer_test.posts (id) { 16 │- id -> Int4, 17 │- author -> Nullable<Int4>, 15 │+ posts (id) { 16 │+ id -> Integer, 17 │+ author -> Nullable<Integer>, 18 18 │ title -> Text, 19 │- datetime -> Nullable<Timestamp>, 19 │+ datetime -> Timestamp, 20 20 │ content -> Nullable<Text>, 21 21 │ } 22 22 │ } 23 23 │ 24 24 │ table! { 25 │- infer_test.users (id) { 26 │- id -> Int4, 25 │+ users (id) { 26 │+ id -> Integer, 27 27 │ name -> Text, 28 28 │ } 29 29 │ } 30 30 │ 31 31 │ allow_tables_to_appear_in_same_query!( 39 39 │ #[table_name = "comments"] 40 40 │ #[primary_key(id)] 41 41 │ pub struct Comment { 42 42 │ id: i32, 43 │- post: Option<HasOne<i32, Post>>, 44 │- commenter: Option<HasOne<i32, User>>, 43 │+ post: Option<i32>, 44 │+ commenter: Option<i32>, 45 45 │ content: String, 46 46 │ } 47 47 │ 48 48 │ #[derive(Clone, Debug, Identifiable, WundergraphEntity)] 49 49 │ #[table_name = "posts"] 50 50 │ #[primary_key(id)] 51 51 │ pub struct Post { 52 52 │ id: i32, 53 │- author: Option<HasOne<i32, User>>, 53 │+ author: Option<i32>, 54 54 │ title: String, 55 │- datetime: Option<chrono::naive::NaiveDateTime>, 55 │+ datetime: chrono::naive::NaiveDateTime, 56 56 │ content: Option<String>, 57 │- comments: HasMany<Comment, comments::post>, 58 57 │ } 59 58 │ 60 59 │ #[derive(Clone, Debug, Identifiable, WundergraphEntity)] 61 60 │ #[table_name = "users"] 62 61 │ #[primary_key(id)] 63 62 │ pub struct User { 64 63 │ id: i32, 65 64 │ name: String, 66 │- comments: HasMany<Comment, comments::commenter>, 67 │- posts: HasMany<Post, posts::author>, 68 65 │ } 69 66 │ 70 67 │ 71 68 │ 72 69 │ wundergraph::query_object!{ 81 78 │ #[derive(Insertable, juniper::GraphQLInputObject, Clone, Debug)] 82 79 │ #[graphql(scalar = "WundergraphScalarValue")] 83 80 │ #[table_name = "comments"] 84 81 │ pub struct NewComment { 82 │+ id: i32, 85 83 │ post: Option<i32>, 86 84 │ commenter: Option<i32>, 87 85 │ content: String, 88 86 │ } 89 87 │ 101 99 │ #[derive(Insertable, juniper::GraphQLInputObject, Clone, Debug)] 102 100 │ #[graphql(scalar = "WundergraphScalarValue")] 103 101 │ #[table_name = "posts"] 104 102 │ pub struct NewPost { 103 │+ id: i32, 105 104 │ author: Option<i32>, 106 105 │ title: String, 107 │- datetime: Option<chrono::naive::NaiveDateTime>, 108 106 │ content: Option<String>, 109 107 │ } 110 108 │ 111 109 │ #[derive(AsChangeset, Identifiable, juniper::GraphQLInputObject, Clone, Debug)] 112 110 │ #[graphql(scalar = "WundergraphScalarValue")] 115 113 │ pub struct PostChangeset { 116 114 │ id: i32, 117 115 │ author: Option<i32>, 118 116 │ title: String, 119 │- datetime: Option<chrono::naive::NaiveDateTime>, 117 │+ datetime: chrono::naive::NaiveDateTime, 120 118 │ content: Option<String>, 121 119 │ } 122 120 │ 123 121 │ #[derive(Insertable, juniper::GraphQLInputObject, Clone, Debug)] 124 122 │ #[graphql(scalar = "WundergraphScalarValue")] 125 123 │ #[table_name = "users"] 126 124 │ pub struct NewUser { 125 │+ id: i32, 127 126 │ name: String, 128 127 │ } 129 128 │ 130 129 │ #[derive(AsChangeset, Identifiable, juniper::GraphQLInputObject, Clone, Debug)] 131 130 │ #[graphql(scalar = "WundergraphScalarValue")] ────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── To update snapshots run `cargo insta review`

Basically all changes to the model structs here should not happen. That means we need to filter out somehow primary keys for the New* model structs, fix foreign key relations for the query models (HasOne/HasMany and figure out what's wrong with the datetime columns. (Maybe a NOT NULL to much?)

1ddf43f#diff-d2705466bf65af436f86cbf11c0635a8R133 makes datetime optional.

a0dc070 filters primary keys explicitly

p-alik · 2020-05-16T19:45:04Z

CI fails for sake of MacOs. I'll work through both insert handlers and appropriate MySQL-wundergraph_example.

weiznich · 2020-05-16T21:25:52Z

I've pushed and merged a fix for the CI failure in #49.

...cli/src/print_schema/snapshots/[email protected]

p-alik · 2020-05-18T15:29:55Z

...cli/src/print_schema/snapshots/[email protected]

@@ -107,6 +110,7 @@ pub struct NewPost {
    id: i32,
    author: Option<i32>,


actually NewPost shouldn't contain id.

p-alik · 2020-05-18T15:36:42Z

wundergraph_cli/Cargo.toml

@@ -17,7 +17,7 @@ diesel = "1.4"

 [dev-dependencies]
 dotenv = "0.15"
-insta = "0.12"
+insta = "0.16"


I'm afraid this commit has an impact on snapshot names: your snapshot was named tests__infer_schema.snap, but insta v0.16 seems to look for wundergraph_cli__print_schema__tests__infer_schema.snap

That's totally fine. Just move/regenerate the snapshot files to the new location.

Add corresponding feature flags to the wundergraph_derive, wundergraph and wundergraph_cli crate

import LimitDsl for sqlite and mysql Co-authored-by: Georg Semmler <[email protected]>

p-alik · 2020-06-15T12:12:25Z

wundergraph_cli/src/print_schema/mod.rs

@@ -316,7 +420,7 @@ mod tests {
        println!("Started server");

        let client = reqwest::Client::new();
-        std::thread::sleep(std::time::Duration::from_secs(1));
+        std::thread::sleep(std::time::Duration::from_secs(5));


5 secs was used to run mysql tests locally. But since ci postgres test failed for same reason, I increased the sleep duration globally.

…weiznich-issue-3

p-alik · 2020-07-15T17:01:45Z

@weiznich, is there anything I've to do to move forward with this PR?

weiznich · 2020-07-16T14:05:55Z

@p-alik I've been quite busy in real life in the last weeks, so I had unfortunately no time to look into this yet. I will try to find some time on the weekend or next week.

Alexei Pastuchov and others added 9 commits April 22, 2020 11:31

Add mysql feature flags

baf05ae

Add corresponding feature flags to the wundergraph_derive, wundergraph and wundergraph_cli crate

Generate an impl for LoadingHandler for diesel::mysql::Mysql

702ddad

Generate an impl for WundergraphBelongsTo for diesel::mysql::Mysql

778fecb

Generate an impl for BuildFilterHelper for diesel::mysql::Mysql

6f76152

Implement ApplyOffset for diesel::mysql::Mysq

61641d2

wundergraph_example supports mysql

0da2814

[UNFINISHED DRAFT] Implement HandleInsert and HandleBatchInsert for d…

52759f9

…iesel::mysql::Mysql

weiznich reviewed May 16, 2020

View reviewed changes

p-alik force-pushed the issue-3 branch from 99d27ec to 5313ccd Compare May 16, 2020 19:21

p-alik force-pushed the issue-3 branch 3 times, most recently from 4d4fb2c to 094d06d Compare May 18, 2020 05:05

weiznich reviewed May 18, 2020

View reviewed changes

...cli/src/print_schema/snapshots/[email protected] Outdated Show resolved Hide resolved

p-alik force-pushed the issue-3 branch from f5c83a0 to 1ddf43f Compare May 18, 2020 15:14

p-alik commented May 18, 2020

View reviewed changes

p-alik force-pushed the issue-3 branch from 1ddf43f to a0dc070 Compare May 18, 2020 18:25

p-alik mentioned this pull request May 19, 2020

wundergraph_cli's print_schema::tests::round_trip test fails #50

Closed

Alexei Pastuchov and others added 8 commits June 5, 2020 13:56

Add mysql feature flags

a92f2c5

Add corresponding feature flags to the wundergraph_derive, wundergraph and wundergraph_cli crate

Generate an impl for LoadingHandler for diesel::mysql::Mysql

e8e1a41

Generate an impl for WundergraphBelongsTo for diesel::mysql::Mysql

add2c6d

Generate an impl for BuildFilterHelper for diesel::mysql::Mysql

0e82a5c

Implement ApplyOffset for diesel::mysql::Mysq

f781c32

Update wundergraph/src/query_builder/selection/offset.rs

0bd0c52

import LimitDsl for sqlite and mysql Co-authored-by: Georg Semmler <[email protected]>

rustfmt adds an empty like for sake of CI

bafc006

avoid warnings: trait objects without an explicit dyn are deprecated

463dafc

p-alik force-pushed the issue-3 branch from 0577341 to eb00a61 Compare June 5, 2020 12:23

p-alik mentioned this pull request Jun 11, 2020

Add support for MySql #3

Open

7 tasks

wundergraph_example supports MySql

858d82f

p-alik force-pushed the issue-3 branch from eb00a61 to d3f36fd Compare June 15, 2020 11:29

wundergraph_cli supports MySql

4ed4ca7

p-alik force-pushed the issue-3 branch from d3f36fd to 4ed4ca7 Compare June 15, 2020 11:55

p-alik commented Jun 15, 2020

View reviewed changes

Alexei Pastuchov added 2 commits June 15, 2020 21:37

Merge branch 'issue-3' of git://github.com/weiznich/wundergraph into …

494511e

…weiznich-issue-3

Merge branch 'weiznich-issue-3' into issue-3

b1e714b

p-alik force-pushed the issue-3 branch from e201642 to b1e714b Compare June 16, 2020 04:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add partly Mysql support #48

add partly Mysql support #48

p-alik commented May 15, 2020

weiznich left a comment

weiznich May 16, 2020

p-alik May 16, 2020

weiznich May 16, 2020

p-alik May 16, 2020

p-alik May 17, 2020

weiznich May 18, 2020

p-alik May 18, 2020

p-alik May 18, 2020

p-alik commented May 16, 2020

weiznich commented May 16, 2020

p-alik May 18, 2020

p-alik May 18, 2020

weiznich May 18, 2020

p-alik Jun 15, 2020

p-alik commented Jul 15, 2020

weiznich commented Jul 16, 2020

		@@ -107,6 +110,7 @@ pub struct NewPost {
		id: i32,
		author: Option<i32>,

add partly Mysql support #48

Are you sure you want to change the base?

add partly Mysql support #48

Conversation

p-alik commented May 15, 2020

weiznich 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-alik commented May 16, 2020

weiznich commented May 16, 2020

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

p-alik commented Jul 15, 2020

weiznich commented Jul 16, 2020