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: allow clearing database inside a transaction #8529

Conversation

varanauskas
Copy link
Contributor

Possible fix for: #8527

Description of change

This is one of the possible fixes for the issue #8527

It allows to run clearDatabase inside a transaction by making clearDatabase to check if another transaction is already running and not creating a new transaction in that case

It currently is only developed for/tested with Postgres, if this is the right direction same fix could be applied for other query runners

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@pleerock
Copy link
Member

I think we can accept it.

It currently is only developed for/tested with Postgres, if this is the right direction same fix could be applied for other query runners

if we go with this fix, fix needs to be applied to all other query runners as well.

@pleerock
Copy link
Member

I think changes still do not cover all drivers.

@varanauskas
Copy link
Contributor Author

Yes, I could not successfully test the changes on other drivers so I did not submit them, I will try to diagnose why I couldn't run some of the docker container tests and submit changes for other drivers. (Any tips on testing with more exotic drivers like SAP Hana or Oracle XD?)

@pleerock
Copy link
Member

For SAP Hana there is no easy way... Hana's official site provides a trial for their product as I know. For Oracle and MSSQL you can simply commit and check if CI is failing. Since the code you are changing is straightforward, I think you can just copy/paste code in required places without mistakes and everything else should be fine.

@pleerock
Copy link
Member

I'm going to close this one because I'm doing unfinished PRs cleanup (to prevent conflicts). Please feel free to rebase it on latest master and reopen it with all requested changes applied. Thank you.

@pleerock pleerock closed this Feb 15, 2022
@varanauskas
Copy link
Contributor Author

Thank you, I will add the changes to other drivers and reopen

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

3 participants