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: drop SAP statement after prepare per Hana client docs #7748

Merged

Conversation

imnotjames
Copy link
Contributor

@imnotjames imnotjames commented Jun 16, 2021

Description of change

each statement takes up a pool so if you query frequently you can
exhaust that pool of statement handles. this drops the statement
after we're done with it so we don't exhaust the pool of statements

fixes #7344

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

@imnotjames
Copy link
Contributor Author

unfortunately, we still don't test SAP Hana. I can barely get a server running to execute tests. Based off the documentation this seems reasonable, though?

@pleerock
Copy link
Member

Any additional test infrustructure is highly appreciated 🤞

@imnotjames
Copy link
Contributor Author

SAP Hana is very, very hard to get testing because of the licensing around Hana. It used to be easier but SAP restricted it further.

It now requires an account that has "purchased" the development kit for free in docker hub. This can be done but it requires attention any time a new version is released.

It also caused the CI runner to OOM in my tests.

src/driver/sap/SapQueryRunner.ts Outdated Show resolved Hide resolved
src/driver/sap/SapQueryRunner.ts Outdated Show resolved Hide resolved
@imnotjames imnotjames force-pushed the fix/7344/sap-prepared-statement-drop branch 2 times, most recently from 9cb9d28 to 8350ba6 Compare June 18, 2021 20:32
@imnotjames imnotjames marked this pull request as draft July 9, 2021 20:08
@imnotjames imnotjames force-pushed the fix/7344/sap-prepared-statement-drop branch from 8350ba6 to 6c09abd Compare July 11, 2021 02:32
@imnotjames imnotjames marked this pull request as ready for review July 11, 2021 02:39
each statement takes up a pool so if you query frequently you can
exhaust that pool of statement handles.  this drops the statement
after we're done with it so we don't exhaust the pool of statements
@imnotjames imnotjames force-pushed the fix/7344/sap-prepared-statement-drop branch from 6c09abd to 15ebc1e Compare July 11, 2021 03:09
@imnotjames imnotjames merged commit 8ca05b1 into typeorm:master Jul 11, 2021
@imnotjames imnotjames deleted the fix/7344/sap-prepared-statement-drop branch July 11, 2021 07:39
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.

SAP Hana Driver does not drop prepared statement
3 participants