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

feat(adapter-d1): set maxBindValues: 100 #24242

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented May 20, 2024

This PR contributes to https://github.com/prisma/team-orm/issues/1070.

Please merge after prisma/prisma-engines#4878 is released.
This should fix the issues mentioned in https://github.com/prisma/team-orm/issues/1070#issuecomment-2079497069 as well (TODO: check after this is merged and well-integrated).

Copy link
Contributor

github-actions bot commented May 20, 2024

size-limit report 📦

Path Size
packages/client/runtime/library.js 179.82 KB (0%)
packages/client/runtime/library.d.ts 81 B (0%)
packages/client/runtime/binary.js 600.89 KB (0%)
packages/client/runtime/binary.d.ts 26 B (0%)
packages/client/runtime/edge.js 159.02 KB (0%)
packages/client/runtime/edge-esm.js 158.91 KB (0%)
packages/client/runtime/wasm.js 114.9 KB (0%)
packages/client/runtime/index-browser.js 33.77 KB (0%)
packages/client/runtime/index-browser.d.ts 89 B (0%)
packages/cli/build/index.js 2.1 MB (0%)
packages/client/prisma-client-0.0.0.tgz 2.93 MB (0%)
packages/cli/prisma-0.0.0.tgz 3.73 MB (0%)
packages/bundle-size/da-workers-libsql/output.tgz 858.11 KB (0%)
packages/bundle-size/da-workers-neon/output.tgz 935.04 KB (0%)
packages/bundle-size/da-workers-pg/output.tgz 953.61 KB (0%)
packages/bundle-size/da-workers-pg-worker/output.tgz 909.33 KB (0%)
packages/bundle-size/da-workers-planetscale/output.tgz 871.06 KB (0%)
packages/bundle-size/da-workers-d1/output.tgz 830.35 KB (0%)

Copy link

codspeed-hq bot commented May 20, 2024

CodSpeed Performance Report

Merging #24242 will not alter performance

Comparing integration/fix-sqlite-d1-max-bind-values (84acdf8) with integration/fix-sqlite-d1-max-bind-values (4d058ed)

Summary

✅ 3 untouched benchmarks

@jkomyno jkomyno added this to the 5.15.0 milestone May 20, 2024
@jkomyno jkomyno marked this pull request as ready for review May 20, 2024 16:07
@jkomyno jkomyno requested a review from a team as a code owner May 20, 2024 16:07
@jkomyno jkomyno requested review from Jolg42 and removed request for a team May 20, 2024 16:07
Copy link
Member

@Jolg42 Jolg42 left a comment

Choose a reason for hiding this comment

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

Do we have some test(s) for this now? (For D1 and other adapters)

@jkomyno
Copy link
Contributor Author

jkomyno commented May 31, 2024

Do we have some test(s) for this now? (For D1 and other adapters)

Not that I could find, except the prisma-engines one, query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs.

@Jolg42
Copy link
Member

Jolg42 commented May 31, 2024

https://github.com/prisma/prisma-engines/blob/main/query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_7434.rs excludes D1 from what I see, so this fix for D1 is untested.

Could you add a test somewhere? in functional tests would be nice maybe?

@Jolg42
Copy link
Member

Jolg42 commented May 31, 2024

I remember that I actually some tests in this PR, I guess that's enough? Could you check @jkomyno ?

@jkomyno jkomyno changed the base branch from main to deps/engines-5.15.0-26.9599c67575e2ecde6e2aa528c1f8e28752413cd0 May 31, 2024 14:32
…52413cd0' into integration/fix-sqlite-d1-max-bind-values
@jkomyno jkomyno merged commit 6359463 into deps/engines-5.15.0-26.9599c67575e2ecde6e2aa528c1f8e28752413cd0 May 31, 2024
9 checks passed
@jkomyno jkomyno deleted the integration/fix-sqlite-d1-max-bind-values branch May 31, 2024 14:33
@jkomyno jkomyno restored the integration/fix-sqlite-d1-max-bind-values branch June 3, 2024 08:08
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.

None yet

2 participants