Skip to content

fix: hana preparations use map #1682

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.

Sign up for GitHub

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

Closed
kasimos wants to merge 2 commits into from
Closed

fix: hana preparations use map #1682

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2f9504
fix: hana preparations use map
kasimos Jul 14, 2026
f530093
fix indent
kasimos Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions hana/lib/HANAService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,14 @@ class HANAService extends SQLService {

// prepare and exec are both implemented inside the drivers
prepare(sql, hasBlobs) {
const stmt = this.ensureDBC().prepare(sql, hasBlobs)
// we store the statements, to release them on commit/rollback all at once
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);

@BobdenOs BobdenOs Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

if (!stmt) {
const newPS = this.ensureDBC().prepare(sql, hasBlobs)
this.ensureDBC().statements.set(sql, newPS)
stmt = newPS
}
return stmt
}

Expand Down Expand Up @@ -1358,7 +1363,8 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction}) ERROR ON E
async onCall({ query, data }, name, schema) {
const isAsync = /\sASYNC\s*$/.test(query)
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)

@BobdenOs BobdenOs Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

const ret = this.ensureDBC() && await ps.proc(data, outParameters)
return isAsync ? ret.ASYNC_CALL_ID[0] : ret
}
Expand All @@ -1385,7 +1391,7 @@ SELECT ${mixing} FROM JSON_TABLE(SRC.JSON, '$' COLUMNS(${extraction}) ERROR ON E

onBEGIN() {
DEBUG?.('BEGIN')
if (this.dbc) this.dbc.statements = []
if (this.dbc) this.dbc.statements = new Map()
return this.dbc?.begin()
}

Expand Down