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

fix: support 1-m relevance ordering #4915

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Weakky
Copy link
Member

@Weakky Weakky commented Jun 10, 2024

Copy link

codspeed-hq bot commented Jun 10, 2024

CodSpeed Performance Report

Merging #4915 will not alter performance

Comparing fix/1-1-order-by-relevance (69fba67) with main (6f3b8db)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jun 10, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.043MiB 2.043MiB 342.000B
Postgres (gzip) 813.981KiB 813.869KiB 115.000B
Mysql 2.013MiB 2.013MiB 322.000B
Mysql (gzip) 800.150KiB 800.252KiB -105.000B
Sqlite 1.914MiB 1.914MiB 51.000B
Sqlite (gzip) 762.030KiB 762.107KiB -79.000B

@Weakky Weakky added this to the 5.16.0 milestone Jun 11, 2024
@Weakky Weakky marked this pull request as ready for review June 11, 2024 14:01
@Weakky Weakky requested a review from a team as a code owner June 11, 2024 14:01
@Weakky Weakky requested review from janpio and removed request for a team June 11, 2024 14:01
let parent_alias = self.parent_alias.clone();
let joins: Vec<AliasedJoin> = self.compute_one2m_join(&order_by.path, parent_alias.as_ref(), ctx);
Copy link
Member Author

Choose a reason for hiding this comment

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

Relations weren't taken into account at all. We're now gathering the path (which is an array of relation fields) that leads to the fields we try to order by on. We compute the needed joins, and then we order by the last field, aliased by the last join alias. This is the same process as regular relational ordering on scalar field.

@reubenjh
Copy link

You're an absolute hero

@@ -376,6 +376,92 @@ async fn on_many_fields_with_aggr_and_pagination(runner: Runner) -> TestResult<(
Ok(())
}

async fn on_1m_relation_field(runner: Runner) -> TestResult<()> {
Copy link
Member Author

Choose a reason for hiding this comment

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

add tests where we hop through several to-one relations

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.

Prisma text search "contains" with Join is not working _relevance error when utilizing orderBy relations
2 participants