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

feat: projected dependencies in the compiler #5134

Merged
merged 9 commits into from
Jan 23, 2025
Merged

feat: projected dependencies in the compiler #5134

merged 9 commits into from
Jan 23, 2025

Conversation

jacek-prisma
Copy link
Contributor

@jacek-prisma jacek-prisma commented Jan 22, 2025 •
edited
Loading

Closes ORM-491 and ORM-470.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.120MiB 2.117MiB 3.056KiB
Postgres (gzip) 848.487KiB 848.033KiB 465.000B
Mysql 2.082MiB 2.079MiB 3.056KiB
Mysql (gzip) 834.399KiB 834.057KiB 350.000B
Sqlite 1.990MiB 1.987MiB 3.056KiB
Sqlite (gzip) 797.975KiB 797.122KiB 874.000B

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jan 22, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #5134 will not alter performance

Comparing feat/compiler-projected-dependencies (8d2b560) with main (eb5bc54)

Summary

✅ 11 untouched benchmarks

@jacek-prisma jacek-prisma marked this pull request as ready for review January 22, 2025 15:19
@jacek-prisma jacek-prisma requested a review from a team as a code owner January 22, 2025 15:19
@jacek-prisma jacek-prisma requested review from aqrln and removed request for a team January 22, 2025 15:19
jacek-prisma
jacek-prisma commented Jan 22, 2025
View reviewed changes
query-engine/query-builders/sql-query-builder/src/write.rs
@@ -48,15 +48,18 @@ pub fn create_records_nonempty(
selected_fields: Option<&ModelProjection>,
ctx: &Context<'_>,
) -> Insert<'static> {
let mut fields = affected_fields.iter().cloned().collect_vec();
fields.sort_by_key(|f| f.id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

needed to add a sort here, because affected_fields is a HashSet and produced randomly ordered columns in my tests

@FGoessler FGoessler added this to the 6.3.0 milestone Jan 23, 2025
FGoessler
FGoessler approved these changes Jan 23, 2025
View reviewed changes
Copy link
Contributor

@FGoessler FGoessler left a comment

Choose a reason for hiding this comment

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

Looks solid for the PoC level for me 👍

The only thing I am a bit worried about is whether we really derisked the whole topic around QueryGraphDependency variants enough. But that's a discussion beyond the scope of this PR.

@jacek-prisma jacek-prisma merged commit 2ad07dd into main Jan 23, 2025
367 checks passed
@jacek-prisma jacek-prisma deleted the feat/compiler-projected-dependencies branch January 23, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FGoessler FGoessler FGoessler approved these changes

@aqrln aqrln Awaiting requested review from aqrln aqrln is a code owner automatically assigned from prisma/orm-rust

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jacek-prisma @FGoessler