fix: hana preparations use map#1682
Conversation
|
Duplicate from: #1496 |
BobdenOs
left a comment
BobdenOs
left a comment
There was a problem hiding this comment.
This approach is insufficient. We already have a PR which solves the problem #1496
currently there is a bug in the hdb driver that prevents a connection to re use a prepared statement after an session variable is updated. Once that is resolved it will be possible to merge the proper fix.
| this.dbc.statements.push(stmt) | ||
| // we store the prepared statements for caching | ||
| // and to release them on commit/rollback all at once | ||
| let stmt = this.ensureDBC().statements.get(sql); |
There was a problem hiding this comment.
This is fundamentally flawed, because it is possible to run the same sql statement in "parallel" at this level which will start throwing errors when the same statement instance is called with exec / execute as the statement has state. Therefor it is required to create a queue for each statement until it is finished with an exec / execute.
| const outParameters = isAsync ? [{ PARAMETER_NAME: 'ASYNC_CALL_ID' }] : await this._getProcedureMetadata(name, schema) | ||
| const ps = await this.prepare(query) | ||
| // make procedure preparations unique to avoid caching issues with internal procedures | ||
| const ps = await this.prepare(query+'--'+this.ensureDBC().statements.length) |
There was a problem hiding this comment.
By changing the SQL it does not just miss the local statement cache, but it will also miss the HANA execution plan cache. Which means that this can cause a dramatic overall response time impact as this force HANA to fully parse through the statement and all its dependencies which depends upon the complexity of the procedure definition.
|
Thanks, somehow I didn't find your PR when we encountered this. Sorry for that. closing the PR and moving discussion over to #1496 |
2 participants
Addresses #1681
Use a Map to store prepared statements and reuse them when possible.
Add a unique comment for procedure calls to prevent reuse for those, as some builtin procedures like acquiring application locks do not work with already prepared statements.
Please note that a similar fix would be possible for sqlite as well. For Postgres it depends on the used driver as some are already doing this preparation caching for you.