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

Returning #292

Merged
merged 33 commits into from
Nov 17, 2021
Merged

Returning #292

merged 33 commits into from
Nov 17, 2021

Conversation

billy1624
Copy link
Member

@billy1624 billy1624 commented Nov 5, 2021

@billy1624 billy1624 self-assigned this Nov 5, 2021
@billy1624
Copy link
Member Author

Hey @tyt2y3, checkout the support of returning for Postgres.

src/executor/update.rs Show resolved Hide resolved
src/executor/update.rs Outdated Show resolved Hide resolved
@billy1624
Copy link
Member Author

Got this error on MariaDB 10.5+

[2021-11-09T04:31:54Z DEBUG sea_orm::driver::sqlx_mysql] INSERT INTO `active_enum` (`category`, `color`, `tea`) VALUES (NULL, NULL, NULL) RETURNING `id`, `category`, `color`, `tea`
[2021-11-09T04:31:54Z INFO  sqlx::query] INSERT INTO `active_enum` (`category`, …; rows: 1, elapsed: 1.526ms
    
    INSERT INTO
      `active_enum` (`category`, `color`, `tea`)
    VALUES
      (?, ?, ?) RETURNING `id`,
      `category`,
      `color`,
      `tea`
    
Error: Query("no column found for name: id")
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/59eed8a2aac0230a8b53e89d4e99d55912ba6b35/library/test/src/lib.rs:194:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@billy1624
Copy link
Member Author

Now I'm stuck on the SQLx problem... I'm thinking maybe we just support returning for Postgres at this stage. We can leave MariaDB & Sqlite as-is, i.e. one more round trip per insert / update, if it involve updating sqlx-core.

@billy1624
Copy link
Member Author

The issue not only affecting MariaDB 10.5+ but SQLite 3.35.0+

@billy1624
Copy link
Member Author

@billy1624
Copy link
Member Author

As discussed, we would only support RETURNING on Postgres at this stage. And leaving MySQL & SQLite for later.

@billy1624 billy1624 marked this pull request as ready for review November 10, 2021 07:34
@billy1624
Copy link
Member Author

Ready for review & merge @tyt2y3

src/executor/update.rs Show resolved Hide resolved
src/entity/base_entity.rs Outdated Show resolved Hide resolved
src/entity/base_entity.rs Show resolved Hide resolved
src/entity/base_entity.rs Show resolved Hide resolved
src/entity/base_entity.rs Show resolved Hide resolved
src/entity/base_entity.rs Show resolved Hide resolved
src/database/db_connection.rs Show resolved Hide resolved
let res = <Self::Entity as EntityTrait>::insert(am).exec(db).await?;
let found = <Self::Entity as EntityTrait>::find_by_id(res.last_insert_id)
.one(db)
let am = <Self::Entity as EntityTrait>::insert(am)
Copy link
Member

Choose a reason for hiding this comment

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

SO we need new test cases here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep!

Copy link
Member Author

Choose a reason for hiding this comment

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

Test cases added

@billy1624 billy1624 mentioned this pull request Nov 17, 2021
10 tasks
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.

Nice

@tyt2y3 tyt2y3 merged commit 0deeddd into master Nov 17, 2021
@tyt2y3 tyt2y3 deleted the returning branch November 17, 2021 09:41
@billy1624 billy1624 mentioned this pull request Nov 19, 2021
arpancodes pushed a commit to arpancodes/sea-orm that referenced this pull request Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support returning clause to avoid database hits
2 participants