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

SAP Hana Driver does not drop prepared statement #7344

Closed
2 of 21 tasks
tmilos opened this issue Feb 5, 2021 · 6 comments · Fixed by #7748
Closed
2 of 21 tasks

SAP Hana Driver does not drop prepared statement #7344

tmilos opened this issue Feb 5, 2021 · 6 comments · Fixed by #7748

Comments

@tmilos
Copy link

tmilos commented Feb 5, 2021

Issue Description

After certain amount of db queries, an error occurs due to exceeded maximum number of prepared statements

Expected Behavior

Single connection should be able to execute infinite number of queries and prepared statements should be closed/dropped when db lib does do it automatically.

Actual Behavior

Error message obtained

Error: exceed maximum number of prepared statements: the number of prepared statements per connection cannot exceed the max statements

Steps to Reproduce

Setup a nestjs with typeorm simple app on SAP hana db, define one entity, and in a loop run inserts for that entity. This brakes after ~34k of inserts

import { InjectRepository } from '@nestjs/typeorm';

  constructor(
    @InjectRepository(Schedule)
    private readonly repo: Repository<Schedule>,
  ) {
  }

for (let i=0; i<=200000; i++) {
      const ent = new Entity({...});
      await this.repo.insert(ent);
}

My Environment

Dependency Version
Operating System MacOS BigSur
Node.js version v12.20.1
Typescript version v3.8.3
TypeORM version v0.2.24
@sap/hana-client version 2.6.58
hdb-pool version 0.1.4
@nestjs/typeorm version 7.0.0

Additional Context

SapQueryRunner.query() is doing databaseConnection.prepare() but not doing stmt.drop() in the callback. In my test, after modifying local copy of the file with

// first thing in the prepare callback
statement.drop((err) => {
  if (err) {
    _this.driver.connection.logger.logQueryError(err, query, parameters, _this);
    resolveChain();
    return fail(new index_1.QueryFailedError(query, parameters, err));
  }
})

Note that patch is done on the compiled js in my local node_modules... Typescript would look different ofc.

I am able to go pass the 34k of inserts w/out the error state above. Let it running all the way up to 200k

Please verify error handling stated above is ok - if I'm gonna do the PR

Relevant Database Driver(s)

  • aurora-data-api
  • aurora-data-api-pg
  • better-sqlite3
  • cockroachdb
  • cordova
  • expo
  • mongodb
  • mysql
  • nativescript
  • oracle
  • postgres
  • react-native
  • sap
  • sqlite
  • sqlite-abstract
  • sqljs
  • sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time, and I know how to start.
  • Yes, I have the time, but I don't know how to start. I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.
@tmilos
Copy link
Author

tmilos commented Feb 11, 2021

Can we get some update here? If I do the PR, will it be merged?

@1i2hs
Copy link

1i2hs commented Apr 6, 2021

Any updates on this issue? This gives quite a big performance impact on applications using SAP HANA...🥲

@tmilos
Copy link
Author

tmilos commented Apr 6, 2021

No updates I can see. I could do a PR to fix it, just wanna get some heads up first, not to waist time producing a rejected PR

@imnotjames
Copy link
Contributor

Or write the PR. It's not a huge change to make?

@imnotjames
Copy link
Contributor

I created a PR. Can you test that it corrects the issue?

@tmilos
Copy link
Author

tmilos commented Jun 18, 2021

Added review, think drop in finally might execute before callback is called. I tested it with drop as first thing in the callback

statement.drop((err) => {
if (err) {
_this.driver.connection.logger.logQueryError(err, query, parameters, _this);
resolveChain();
return fail(new index_1.QueryFailedError(query, parameters, err));
}
})

Sorry, used the phone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants