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

Jump to bottom

qe: Implement isolation levels for batch transactions #3199

Merged
merged 2 commits into from Sep 20, 2022
Merged

qe: Implement isolation levels for batch transactions #3199

merged 2 commits into from Sep 20, 2022

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Sep 16, 2022

Add optional isolation_level field to MultiDoc and propogate it up
to connector. This mostly reuses iTx isolation level logic, implemented
on connector level.
Test-wise, we also test them in a same way iTx are tested right now:
check that valid value is accepted, invalid is rejected and rely on
quaint tests to do the right thing.

@SevInf SevInf added this to the 4.4.0 milestone Sep 16, 2022
SevInf added a commit to prisma/prisma that referenced this pull request Sep 16, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
d7e45ee 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed. Empty `transaction` field now
  means that request is not running in transaction.

  For testing we are checking that engine correctly generates corresponding SQL.
  Properly testing different isolation level effects would be very
  difficult, especially in case of batch transactions, where we can not
  execute any code between the queries.

Ref #9678
@SevInf SevInf mentioned this pull request Sep 16, 2022
Merged
@SevInf SevInf marked this pull request as ready for review September 16, 2022 15:36
garrensmith
garrensmith suggested changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@garrensmith garrensmith left a comment

Choose a reason for hiding this comment

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

Just some small changes. But this looks really good. Nice work.

query-engine/connector-test-kit-rs/query-engine-tests/tests/new/interactive_tx.rs Show resolved Hide resolved
query-engine/core/src/executor/interpreting_executor.rs Show resolved Hide resolved
query-engine/core/src/executor/interpreting_executor.rs Outdated Show resolved Hide resolved
query-engine/request-handlers/src/graphql/body.rs Outdated Show resolved Hide resolved
@SevInf
qe: Implement isolation levels for batch transactions
bd57fc4 
Add optional `isolation_level` field to `MultiDoc` and propogate it up
to connector. This mostly reuses iTx isolation level logic, implemented
on connector level.
Test-wise, we also test them in a same way iTx are tested right now:
check that valid value is accepted, invalid is rejected and rely on
quaint tests to do the right thing.
@SevInf
Address review feedback
3eb13fb
SevInf added a commit to prisma/prisma that referenced this pull request Sep 19, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
d88c2fd 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed. Empty `transaction` field now
  means that request is not running in transaction.

  For testing we are checking that engine correctly generates corresponding SQL.
  Properly testing different isolation level effects would be very
  difficult, especially in case of batch transactions, where we can not
  execute any code between the queries.

Ref #9678
garrensmith
garrensmith approved these changes Sep 20, 2022
View reviewed changes
Copy link
Contributor

@garrensmith garrensmith left a comment

Choose a reason for hiding this comment

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

Nicely done. Thanks.

@SevInf SevInf merged commit 94146ed into main Sep 20, 2022
@SevInf SevInf deleted the feat/batch-isolation-level branch September 20, 2022 10:44
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
5a42e05 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed. Empty `transaction` field now
  means that request is not running in transaction.

  For testing we are checking that engine correctly generates corresponding SQL.
  Properly testing different isolation level effects would be very
  difficult, especially in case of batch transactions, where we can not
  execute any code between the queries.

Ref #9678
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
d3bc88a 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed from everywhere expcept
  middleware public APIs. `transaction` field now indicates that the
  request should run in transaction.

For testing we are checking that engine correctly generates corresponding SQL.
Properly testing different isolation level effects would be very
difficult, especially in case of batch transactions, where we can not
execute any code between the queries.

Ref #9678
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
9e86e88 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed from everywhere expcept
  middleware public APIs. `transaction` field now indicates that the
  request should run in transaction.

For testing we are checking that engine correctly generates corresponding SQL.
Properly testing different isolation level effects would be very
difficult, especially in case of batch transactions, where we can not
execute any code between the queries.

Ref #9678
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
fec107c 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed from everywhere expcept
  middleware public APIs. `transaction` field now indicates that the
  request should run in transaction.

For testing we are checking that engine correctly generates corresponding SQL.
Properly testing different isolation level effects would be very
difficult, especially in case of batch transactions, where we can not
execute any code between the queries.

Ref #9678
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions
9ba009e 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed from everywhere expcept
  middleware public APIs. `transaction` field now indicates that the
  request should run in transaction.

For testing we are checking that engine correctly generates corresponding SQL.
Properly testing different isolation level effects would be very
difficult, especially in case of batch transactions, where we can not
execute any code between the queries.

Ref #9678
SevInf added a commit to prisma/prisma that referenced this pull request Sep 20, 2022
@SevInf
feat(client): Implement isolation levels for bathc transactions (#15362)
a3a2177 
Engines part: prisma/prisma-engines#3199

Batch `$transaction` call will now receive optional options
objects with `isolationLevel` property, allowing to pick any level,
supporting by current transaction. Internally, refactors a bunch of
things about the way transaction info is stored in `PrismaPromise`,
`Request` and `InternalRequest` types.

- All transaction-related properties are now grouped under `transaction`
  key. Distiction between iTx and batch transaction is done explcitly
  via discriminated union type.
- `runInTransaction` property is removed from everywhere expcept
  middleware public APIs. `transaction` field now indicates that the
  request should run in transaction.

For testing we are checking that engine correctly generates corresponding SQL.
Properly testing different isolation level effects would be very
difficult, especially in case of batch transactions, where we can not
execute any code between the queries.

Ref #9678
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garrensmith garrensmith garrensmith approved these changes

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@SevInf @garrensmith