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

feat: remove hdb-pool dependency from HANA driver #10786

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion README-zh_CN.md
Expand Up @@ -202,7 +202,6 @@ await timber.remove();
```
npm config set @sap:registry https://npm.sap.com
npm i @sap/hana-client
npm i hdb-pool
```

##### TypeScript 配置
Expand Down
1 change: 0 additions & 1 deletion README.md
Expand Up @@ -214,7 +214,6 @@ await timber.remove()

```
npm install @sap/hana-client
npm install hdb-pool
```

_SAP Hana support made possible by the sponsorship of [Neptune Software](https://www.neptune-software.com/)._
Expand Down
1 change: 0 additions & 1 deletion README_ko.md
Expand Up @@ -177,7 +177,6 @@ await timber.remove();

```
npm i @sap/hana-client
npm i hdb-pool
```

*[Neptune Software](https://www.neptune-software.com/)의 후원으로 SAP Hana 지원이 가능해졌다.*
Expand Down
36 changes: 31 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions package.json
Expand Up @@ -149,9 +149,8 @@
},
"peerDependencies": {
"@google-cloud/spanner": "^5.18.0",
"@sap/hana-client": "^2.12.25",
"@sap/hana-client": "^2.20.15",
"better-sqlite3": "^7.1.2 || ^8.0.0 || ^9.0.0",
"hdb-pool": "^0.1.6",
"ioredis": "^5.0.4",
"mongodb": "^5.8.0",
"mssql": "^9.1.1 || ^10.0.1",
Expand All @@ -176,9 +175,6 @@
"better-sqlite3": {
"optional": true
},
"hdb-pool": {
"optional": true
},
"ioredis": {
"optional": true
},
Expand Down
20 changes: 16 additions & 4 deletions src/driver/postgres/PostgresQueryRunner.ts
Expand Up @@ -601,10 +601,22 @@ export class PostgresQueryRunner
downQueries.push(this.dropIndexSql(table, index))
})
}

if (table.comment) {
upQueries.push(new Query("COMMENT ON TABLE " + this.escapePath(table) + " IS '" + table.comment + "'"));
downQueries.push(new Query("COMMENT ON TABLE " + this.escapePath(table) + " IS NULL"));
upQueries.push(
new Query(
"COMMENT ON TABLE " +
this.escapePath(table) +
" IS '" +
table.comment +
"'",
),
)
downQueries.push(
new Query(
"COMMENT ON TABLE " + this.escapePath(table) + " IS NULL",
),
)
}

await this.executeQueries(upQueries, downQueries)
Expand Down Expand Up @@ -4744,7 +4756,7 @@ export class PostgresQueryRunner

newComment = this.escapeComment(newComment)
const comment = this.escapeComment(table.comment)

if (newComment === comment) {
return
}
Expand Down
66 changes: 26 additions & 40 deletions src/driver/sap/SapConnectionOptions.ts
Expand Up @@ -18,54 +18,40 @@ export interface SapConnectionOptions
readonly schema?: string

/**
* The driver objects
* This defaults to require("hdb-pool")
* By default, implicit connection pooling is disabled.
*/
readonly driver?: any
readonly pooling?: boolean

/**
* The driver objects
* This defaults to require("@sap/hana-client")
* The maximum pool size for a specific connection string or option.
* By default value is 0, meaning that there is no limit.
* DO NOT use this if pooling=false
*/
readonly hanaClientDriver?: any
readonly maxPoolSize?: number

/**
* Pool options.
* If set to true, the Node.js driver specifies that connections
* in the connection pool should be tested for viability before being reused.
* DO NOT use this if pooling=false
*/
readonly pool?: {
/**
* Max number of connections.
*/
readonly max?: number
readonly poolingCheck?: boolean

/**
* Minimum number of connections.
*/
readonly min?: number

/**
* Maximum number of waiting requests allowed. (default=0, no limit).
*/
readonly maxWaitingRequests?: number
/**
* Max milliseconds a request will wait for a resource before timing out. (default=5000)
*/
readonly requestTimeout?: number
/**
* How often to run resource timeout checks. (default=0, disabled)
*/
readonly checkInterval?: number
/**
* Idle timeout
*/
readonly idleTimeout?: number
/**
* Specifies the maximum time, in seconds,
* that the connection is cached in the implicit connection pool.
* A value of 0 causes implicit pooled connections to be cached permanently.
* DO NOT use this if pooling=false
*/
readonly connectionLifetime?: number

/**
* Function handling errors thrown by drivers pool.
* Defaults to logging error with `warn` level.
*/
readonly poolErrorHandler?: (err: any) => any
}
/**
* Sub-pool name.
*/
readonly poolKey?: string

readonly poolSize?: never
/**
* Aborts communication attempts to the server
* after the specified timeout.
*/
readonly communicationTimeout?: number
}
89 changes: 32 additions & 57 deletions src/driver/sap/SapDriver.ts
Expand Up @@ -47,10 +47,16 @@ export class SapDriver implements Driver {
*/
client: any

/**
* Hana Client instance.
*/
hanaClient: any

/**
* Hana Client streaming extension.
*/
streamClient: any

/**
* Pool for master database.
*/
Expand Down Expand Up @@ -246,55 +252,37 @@ export class SapDriver implements Driver {
async connect(): Promise<void> {
// HANA connection info
const dbParams = {
hostName: this.options.host,
port: this.options.port,
userName: this.options.username,
password: this.options.password,
serverNode: `${this.options.host}:${this.options.port}`,
uid: this.options.username,
pwd: this.options.password,
...this.options.extra,
}

if (this.options.database) dbParams.databaseName = this.options.database
if (this.options.encrypt) dbParams.encrypt = this.options.encrypt
if (this.options.sslValidateCertificate)
dbParams.validateCertificate = this.options.sslValidateCertificate
dbParams.sslValidateCertificate =
this.options.sslValidateCertificate
if (this.options.key) dbParams.key = this.options.key
if (this.options.cert) dbParams.cert = this.options.cert
if (this.options.ca) dbParams.ca = this.options.ca

// pool options
const options: any = {
min:
this.options.pool && this.options.pool.min
? this.options.pool.min
: 1,
max:
this.options.pool && this.options.pool.max
? this.options.pool.max
: 10,
}

if (this.options.pool && this.options.pool.checkInterval)
options.checkInterval = this.options.pool.checkInterval
if (this.options.pool && this.options.pool.maxWaitingRequests)
options.maxWaitingRequests = this.options.pool.maxWaitingRequests
if (this.options.pool && this.options.pool.requestTimeout)
options.requestTimeout = this.options.pool.requestTimeout
if (this.options.pool && this.options.pool.idleTimeout)
options.idleTimeout = this.options.pool.idleTimeout
this.master = this.hanaClient.createPool(dbParams, this.options)
if (!this.database || !this.schema) {
const queryRunner = this.createQueryRunner("master")

const { logger } = this.connection
if (!this.database) {
this.database = await queryRunner.getCurrentDatabase()
}

const poolErrorHandler =
options.poolErrorHandler ||
((error: any) =>
logger.log("warn", `SAP Hana pool raised an error. ${error}`))
this.client.eventEmitter.on("poolError", poolErrorHandler)
if (!this.schema) {
this.schema = await queryRunner.getCurrentSchema()
}

// create the pool
this.master = this.client.createPool(dbParams, options)
await queryRunner.release()
}

if (!this.database || !this.schema) {
const queryRunner = await this.createQueryRunner("master")
const queryRunner = this.createQueryRunner("master")

if (!this.database) {
this.database = await queryRunner.getCurrentDatabase()
Expand Down Expand Up @@ -712,9 +700,6 @@ export class SapDriver implements Driver {
insertResult
) {
value = insertResult
// } else if (generatedColumn.generationStrategy === "uuid") {
// console.log("getting db value:", generatedColumn.databaseName);
// value = generatedColumn.getEntityValue(uuidMap);
}

return OrmUtils.mergeDeep(
Expand All @@ -729,14 +714,14 @@ export class SapDriver implements Driver {
}

/**
* Differentiate columns of this table and columns from the given column metadatas columns
* Differentiate columns of this table and columns from the given column metadata columns
* and returns only changed.
*/
findChangedColumns(
tableColumns: TableColumn[],
columnMetadatas: ColumnMetadata[],
columnMetadata: ColumnMetadata[],
): ColumnMetadata[] {
return columnMetadatas.filter((columnMetadata) => {
return columnMetadata.filter((columnMetadata) => {
const tableColumn = tableColumns.find(
(c) => c.name === columnMetadata.databaseName,
)
Expand All @@ -758,7 +743,7 @@ export class SapDriver implements Driver {
// console.log("==========================================");

const normalizeDefault = this.normalizeDefault(columnMetadata)
const hanaNullComapatibleDefault =
const hanaNullCompatibleDefault =
normalizeDefault == null ? undefined : normalizeDefault

return (
Expand All @@ -772,7 +757,7 @@ export class SapDriver implements Driver {
tableColumn.comment !==
this.escapeComment(columnMetadata.comment) ||
(!tableColumn.isGenerated &&
hanaNullComapatibleDefault !== tableColumn.default) || // we included check for generated here, because generated columns already can have default values
hanaNullCompatibleDefault !== tableColumn.default) || // we included check for generated here, because generated columns already can have default values
tableColumn.isPrimary !== columnMetadata.isPrimary ||
tableColumn.isNullable !== columnMetadata.isNullable ||
tableColumn.isUnique !==
Expand Down Expand Up @@ -820,20 +805,10 @@ export class SapDriver implements Driver {
*/
protected loadDependencies(): void {
try {
const client = this.options.driver || PlatformTools.load("hdb-pool")
this.client = client
} catch (e) {
// todo: better error for browser env
throw new DriverPackageNotInstalledError("SAP Hana", "hdb-pool")
}

try {
if (!this.options.hanaClientDriver) {
PlatformTools.load("@sap/hana-client")
this.streamClient = PlatformTools.load(
"@sap/hana-client/extension/Stream",
)
}
this.hanaClient = PlatformTools.load("@sap/hana-client")
this.streamClient = PlatformTools.load(
"@sap/hana-client/extension/Stream",
)
} catch (e) {
// todo: better error for browser env
throw new DriverPackageNotInstalledError(
Expand Down
4 changes: 0 additions & 4 deletions src/driver/sap/SapQueryRunner.ts
Expand Up @@ -88,10 +88,6 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
release(): Promise<void> {
this.isReleased = true

if (this.databaseConnection) {
return this.driver.master.release(this.databaseConnection)
}

return Promise.resolve()
}

Expand Down