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.

Sign up for GitHub

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

Jump to bottom

refactor: remove order by clause when counting rows to improve performance #2030

Merged
merged 1 commit into from
Jan 10, 2024
Merged

refactor: remove order by clause when counting rows to improve performance #2030

merged 1 commit into from
Jan 10, 2024

Conversation

shogo-nakano-desu
Copy link
Contributor

@shogo-nakano-desu shogo-nakano-desu commented Dec 21, 2023 •
edited
Loading

Purpose of this PR

To enhance performance. I have added a process to remove the order by clause in the num_items method of the Paginator trait, which is not needed to count rows.

Performance Measurement Results

Upon executing queries on test data available to me, a performance improvement of approximately three times was observed when the order by clause was removed. (Unfortunately, I cannot provide the data used here, but I hope for your understanding.) Testing environment is PostgreSQL 14.7.

@shogo-nakano-desu shogo-nakano-desu changed the title refactor: add clear_order_by for num_items refactor: add clear_order_by for num_items to improve performance Dec 21, 2023
@shogo-nakano-desu shogo-nakano-desu changed the title refactor: add clear_order_by for num_items to improve performance refactor: remove order by clause when counting rows to improve performance Jan 3, 2024
@shogo-nakano-desu
Copy link
Contributor Author

@tyt2y3
First and foremost, I would like to express my gratitude for creating and maintaining such an excellent crate. Thank you very much.

At your convenience, could you please review this PR? We are currently using sea-orm in our production environment, and the changes proposed in this PR are expected to significantly contribute to resolving performance bottlenecks. Your assistance in reviewing would be greatly appreciated. Thank you for your time and consideration.

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 9, 2024 •
edited
Loading

Thank you for the contribution. I think this is the right thing to do. Just being curious, what performance gains you saw on a typical vs worst case query?

@shogo-nakano-desu
Copy link
Contributor Author

shogo-nakano-desu commented Jan 9, 2024 •
edited
Loading

Thank you so much for your review.

Table info (Our testing environment)

  • About 15 columns
  • About 8M rows
  • The field used in the ORDER BY clause does not have an index.
    ＊In our production environment, there are approximately 50M rows and the number is increasing rapidly.

Result in our testing environment

Worst case

Execution Time: 2740 ms -> 840 ms

Typycal case

In the production environment, handling a significantly larger volume of data is expected. Therefore, it is anticipated that the worst-case scenario in terms of data quantity in the test environment would be equivalent to a typical case in the production environment. As a result, we have not measured the typical case in the test environment.

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 9, 2024

Interesting, good to know!

The field used in the ORDER BY clause does not have an index.

That's usually the culprit haha

@shogo-nakano-desu
Copy link
Contributor Author

Now I am pushing a commit to fix Clippy error. It will be reflected when GitHub status is back to normal. https://www.githubstatus.com/

Screenshot 2024-01-09 at 22 38 25

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 9, 2024

GitHub is back now

@shogo-nakano-desu
Copy link
Contributor Author

Great, I pushed my commits. Now I think this PR will pass CI.

shogo-nakano-desu
shogo-nakano-desu commented Jan 9, 2024
View reviewed changes
src/executor/paginator.rs
Comment on lines -74 to +79
self.query.clone().reset_limit().reset_offset().to_owned(),
self.query
.clone()
.reset_limit()
.reset_offset()
.clear_order_by()
.to_owned(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only these lines are the main change. Others are changes to fix clippy&compile errors.

@shogo-nakano-desu shogo-nakano-desu marked this pull request as draft January 9, 2024 23:48
@shogo-nakano-desu
Copy link
Contributor Author

shogo-nakano-desu commented Jan 9, 2024 •
edited
Loading

I have to fix error: associated function accepts is never used clippy errors in src/driver/sqlx_xxx.rs
Fixed 571b51d

@shogo-nakano-desu shogo-nakano-desu marked this pull request as ready for review January 9, 2024 23:58
@tyt2y3
Copy link
Member

tyt2y3 commented Jan 10, 2024

Sad, those clippy warnings seem to be very confusing

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 10, 2024

Can you reset hard this PR to 3ef07ab ? I will merge that first.
We can work on the clippy fixes later.

@shogo-nakano-desu shogo-nakano-desu force-pushed the refactor/paginator-num-items branch from 571b51d to 3ef07ab Compare January 10, 2024 10:04
@shogo-nakano-desu
Copy link
Contributor Author

Sure, that's a good idea. I will create another PR to fix clippy errors.

@shogo-nakano-desu
Copy link
Contributor Author

Can you reset hard this PR to 3ef07ab ?

Done

@tyt2y3 tyt2y3 merged commit 3dc66aa into SeaQL:master Jan 10, 2024
30 of 31 checks passed
@shogo-nakano-desu shogo-nakano-desu mentioned this pull request Jan 10, 2024
Merged
@github-actions GitHub Actions
Copy link

🎉 Released In 0.12.11 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

maxdeviant referenced this pull request in zed-industries/zed Jul 27, 2024
@renovate
Update Rust crate sea-orm to v0.12.15 (#15343)
c7a78fa 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [sea-orm](https://www.sea-ql.org/SeaORM)
([source](https://github.com/SeaQL/sea-orm)) | dev-dependencies |
patch | `0.12.3` -> `0.12.15` |
| [sea-orm](https://www.sea-ql.org/SeaORM)
([source](https://github.com/SeaQL/sea-orm)) | dependencies | patch |
`0.12.3` -> `0.12.15` |

---

### Release Notes

<details>
<summary>SeaQL/sea-orm (sea-orm)</summary>

###
[`v0.12.15`](https://github.com/SeaQL/sea-orm/releases/tag/0.12.15)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.14...0.12.15)

##### Enhancements

- `DerivePartialModel` macro attribute `entity` now supports `syn::Type`
[https://github.com/SeaQL/sea-orm/pull/2137](https://github.com/SeaQL/sea-orm/pull/2137)

```rust

#[derive(DerivePartialModel)]
#[sea_orm(entity = "<entity::Model as ModelTrait>::Entity")]
struct EntityNameNotAIdent {
    #[sea_orm(from_col = "foo2")]
    _foo: i32,
    #[sea_orm(from_col = "bar2")]
    _bar: String,
}
```

- Added `RelationDef::from_alias()`
[https://github.com/SeaQL/sea-orm/pull/2146](https://github.com/SeaQL/sea-orm/pull/2146)

```rust
assert_eq!(
    cake::Entity::find()
        .join_as(
            JoinType::LeftJoin,
            cake_filling::Relation::Cake.def().rev(),
            cf.clone()
        )
        .join(
            JoinType::LeftJoin,
            cake_filling::Relation::Filling.def().from_alias(cf)
        )
        .build(DbBackend::MySql)
        .to_string(),
    [
        "SELECT `cake`.`id`, `cake`.`name` FROM `cake`",
        "LEFT JOIN `cake_filling` AS `cf` ON `cake`.`id` = `cf`.`cake_id`",
        "LEFT JOIN `filling` ON `cf`.`filling_id` = `filling`.`id`",
    ]
    .join(" ")
);
```

###
[`v0.12.14`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#01214---2024-02-05)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.12...0.12.14)

- Added feature flag `sqlite-use-returning-for-3_35` to use SQLite's
returning
[https://github.com/SeaQL/sea-orm/pull/2070](https://github.com/SeaQL/sea-orm/pull/2070)
- Added Loco example
[https://github.com/SeaQL/sea-orm/pull/2092](https://github.com/SeaQL/sea-orm/pull/2092)

###
[`v0.12.12`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#01212---2024-01-22)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.11...0.12.12)

##### Bug Fixes

- \[sea-orm-cli] Fix entity generation for non-alphanumeric enum
variants[https://github.com/SeaQL/sea-orm/pull/1821](https://github.com/SeaQL/sea-orm/pull/1821)1
- \[sea-orm-cli] Fix entity generation for relations with composite
keys[https://github.com/SeaQL/sea-orm/pull/2071](https://github.com/SeaQL/sea-orm/pull/2071)1

##### Enhancements

-   Added `ConnectOptions::test_before_acquire`

###
[`v0.12.11`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#01211---2024-01-14)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.10...0.12.11)

##### New Features

- Added `desc` to `Cursor` paginator
[https://github.com/SeaQL/sea-orm/pull/2037](https://github.com/SeaQL/sea-orm/pull/2037)

##### Enhancements

- Improve query performance of `Paginator`'s `COUNT` query
[https://github.com/SeaQL/sea-orm/pull/2030](https://github.com/SeaQL/sea-orm/pull/2030)
- Added SQLx slow statements logging to `ConnectOptions`
[https://github.com/SeaQL/sea-orm/pull/2055](https://github.com/SeaQL/sea-orm/pull/2055)
- Added `QuerySelect::lock_with_behavior`
[https://github.com/SeaQL/sea-orm/pull/1867](https://github.com/SeaQL/sea-orm/pull/1867)

##### Bug Fixes

- \[sea-orm-macro] Qualify types in `DeriveValueType` macro
[https://github.com/SeaQL/sea-orm/pull/2054](https://github.com/SeaQL/sea-orm/pull/2054)

##### House keeping

- Fix clippy warnings on 1.75
[https://github.com/SeaQL/sea-orm/pull/2057](https://github.com/SeaQL/sea-orm/pull/2057)

###
[`v0.12.10`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#01210---2023-12-14)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.9...0.12.10)

##### New Features

- \[sea-orm-macro] Comment attribute for Entity (`#[sea_orm(comment =
"action")]`); `create_table_from_entity` supports comment
[https://github.com/SeaQL/sea-orm/pull/2009](https://github.com/SeaQL/sea-orm/pull/2009)
- Added "proxy" (feature flag `proxy`) to database backend
[https://github.com/SeaQL/sea-orm/pull/1881](https://github.com/SeaQL/sea-orm/pull/1881),
[https://github.com/SeaQL/sea-orm/pull/2000](https://github.com/SeaQL/sea-orm/pull/2000)

##### Enhancements

- Cast enums in `is_in` and `is_not_in`
[https://github.com/SeaQL/sea-orm/pull/2002](https://github.com/SeaQL/sea-orm/pull/2002)

##### Upgrades

- Updated `sea-query` to `0.30.5`
https://github.com/SeaQL/sea-query/releases/tag/0.30.5

###
[`v0.12.9`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0129---2023-12-08)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.8...0.12.9)

##### Enhancements

- Add source annotations to errors
[https://github.com/SeaQL/sea-orm/pull/1999](https://github.com/SeaQL/sea-orm/pull/1999)

##### Upgrades

- Updated `sea-query` to `0.30.4`
https://github.com/SeaQL/sea-query/releases/tag/0.30.4

###
[`v0.12.8`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0128---2023-12-04)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.7...0.12.8)

##### Enhancements

- Implement `StatementBuilder` for `sea_query::WithQuery`
[https://github.com/SeaQL/sea-orm/issues/1960](https://github.com/SeaQL/sea-orm/issues/1960)

##### Upgrades

- Upgrade `axum` example to `0.7`
[https://github.com/SeaQL/sea-orm/pull/1984](https://github.com/SeaQL/sea-orm/pull/1984)

###
[`v0.12.7`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0127---2023-11-22)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.6...0.12.7)

##### Enhancements

- Added method `expr_as_` that accepts `self`
[https://github.com/SeaQL/sea-orm/pull/1979](https://github.com/SeaQL/sea-orm/pull/1979)

##### Upgrades

- Updated `sea-query` to `0.30.3`
https://github.com/SeaQL/sea-query/releases/tag/0.30.3

###
[`v0.12.6`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0126---2023-11-13)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.5...0.12.6)

##### New Features

- Added `#[sea_orm(skip)]` for `FromQueryResult` derive macro
[https://github.com/SeaQL/sea-orm/pull/1954](https://github.com/SeaQL/sea-orm/pull/1954)

###
[`v0.12.5`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0125---2023-11-12)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.4...0.12.5)

##### Bug Fixes

- \[sea-orm-cli] Fix duplicated active enum use statements on generated
entities[https://github.com/SeaQL/sea-orm/pull/1953](https://github.com/SeaQL/sea-orm/pull/1953)3
- \[sea-orm-cli] Added `--enum-extra-derives`
[https://github.com/SeaQL/sea-orm/pull/1934](https://github.com/SeaQL/sea-orm/pull/1934)
- \[sea-orm-cli] Added `--enum-extra-attributes`
[https://github.com/SeaQL/sea-orm/pull/1952](https://github.com/SeaQL/sea-orm/pull/1952)

###
[`v0.12.4`](https://github.com/SeaQL/sea-orm/blob/HEAD/CHANGELOG.md#0124---2023-10-19)

[Compare
Source](https://github.com/SeaQL/sea-orm/compare/0.12.3...0.12.4)

##### New Features

- Add support for root JSON arrays
[https://github.com/SeaQL/sea-orm/pull/1898](https://github.com/SeaQL/sea-orm/pull/1898)
Now the following works (requires the `json-array` / `postgres-array`
feature)!

```rust

#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
#[sea_orm(table_name = "json_struct_vec")]
pub struct Model {
    #[sea_orm(primary_key)]
    pub id: i32,
    #[sea_orm(column_type = "Json")]
    pub struct_vec: Vec<JsonColumn>,
}

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, FromJsonQueryResult)]
pub struct JsonColumn {
    pub value: String,
}
```

##### Enhancements

- Loader: use `ValueTuple` as hash key
[https://github.com/SeaQL/sea-orm/pull/1868](https://github.com/SeaQL/sea-orm/pull/1868)

##### Upgrades

- Updated `sea-query` to `0.30.2`
https://github.com/SeaQL/sea-query/releases/tag/0.30.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 3pm on Wednesday" in timezone
America/New_York, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

Release Notes:

- N/A

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@shogo-nakano-desu @tyt2y3