Skip to content

Commit 45e31cc

Browse files
alumnialumni
authored
feat: implement column comments for SAP HANA (#10502)
* feat: implement column comments for SAP HANA * test: improve schema tests for SAP HANA
1 parent 7e9cead commit 45e31cc

File tree

8 files changed

+132
-126
lines changed

8 files changed

+132
-126
lines changed

‎src/driver/sap/SapDriver.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export class SapDriver implements Driver {
186186
cacheTime: "bigint",
187187
cacheDuration: "integer",
188188
cacheQuery: "nvarchar(5000)" as any,
189-
cacheResult: "text",
189+
cacheResult: "nclob",
190190
metadataType: "nvarchar",
191191
metadataDatabase: "nvarchar",
192192
metadataSchema: "nvarchar",
@@ -769,7 +769,8 @@ export class SapDriver implements Driver {
769769
this.getColumnLength(columnMetadata)) ||
770770
tableColumn.precision !== columnMetadata.precision ||
771771
tableColumn.scale !== columnMetadata.scale ||
772-
// || tableColumn.comment !== columnMetadata.comment || // todo
772+
tableColumn.comment !==
773+
this.escapeComment(columnMetadata.comment) ||
773774
(!tableColumn.isGenerated &&
774775
hanaNullComapatibleDefault !== tableColumn.default) || // we included check for generated here, because generated columns already can have default values
775776
tableColumn.isPrimary !== columnMetadata.isPrimary ||
@@ -841,4 +842,15 @@ export class SapDriver implements Driver {
841842
)
842843
}
843844
}
845+
846+
/**
847+
* Escapes a given comment.
848+
*/
849+
protected escapeComment(comment?: string) {
850+
if (!comment) return comment
851+
852+
comment = comment.replace(/\u0000/g, "") // Null bytes aren't allowed in comments
853+
854+
return comment
855+
}
844856
}

‎src/driver/sap/SapQueryRunner.ts‎

Lines changed: 66 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,19 +1263,48 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
12631263
oldColumn.name = newColumn.name
12641264
}
12651265

1266-
if (this.isColumnChanged(oldColumn, newColumn)) {
1266+
if (this.isColumnChanged(oldColumn, newColumn, true)) {
12671267
upQueries.push(
12681268
new Query(
12691269
`ALTER TABLE ${this.escapePath(
12701270
table,
1271-
)} ALTER (${this.buildCreateColumnSql(newColumn)})`,
1271+
)} ALTER (${this.buildCreateColumnSql(
1272+
newColumn,
1273+
!(
1274+
oldColumn.default === null ||
1275+
oldColumn.default === undefined
1276+
),
1277+
!oldColumn.isNullable,
1278+
)})`,
12721279
),
12731280
)
12741281
downQueries.push(
12751282
new Query(
12761283
`ALTER TABLE ${this.escapePath(
12771284
table,
1278-
)} ALTER (${this.buildCreateColumnSql(oldColumn)})`,
1285+
)} ALTER (${this.buildCreateColumnSql(
1286+
oldColumn,
1287+
!(
1288+
newColumn.default === null ||
1289+
newColumn.default === undefined
1290+
),
1291+
!newColumn.isNullable,
1292+
)})`,
1293+
),
1294+
)
1295+
} else if (oldColumn.comment !== newColumn.comment) {
1296+
upQueries.push(
1297+
new Query(
1298+
`COMMENT ON COLUMN ${this.escapePath(table)}."${
1299+
oldColumn.name
1300+
}" IS ${this.escapeComment(newColumn.comment)}`,
1301+
),
1302+
)
1303+
downQueries.push(
1304+
new Query(
1305+
`COMMENT ON COLUMN ${this.escapePath(table)}."${
1306+
newColumn.name
1307+
}" IS ${this.escapeComment(oldColumn.comment)}`,
12791308
),
12801309
)
12811310
}
@@ -1427,74 +1456,6 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
14271456
}
14281457
}
14291458

1430-
if (newColumn.default !== oldColumn.default) {
1431-
if (
1432-
newColumn.default !== null &&
1433-
newColumn.default !== undefined
1434-
) {
1435-
upQueries.push(
1436-
new Query(
1437-
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
1438-
newColumn.name
1439-
}" ${this.connection.driver.createFullType(
1440-
newColumn,
1441-
)} DEFAULT ${newColumn.default})`,
1442-
),
1443-
)
1444-
1445-
if (
1446-
oldColumn.default !== null &&
1447-
oldColumn.default !== undefined
1448-
) {
1449-
downQueries.push(
1450-
new Query(
1451-
`ALTER TABLE ${this.escapePath(
1452-
table,
1453-
)} ALTER ("${
1454-
oldColumn.name
1455-
}" ${this.connection.driver.createFullType(
1456-
oldColumn,
1457-
)} DEFAULT ${oldColumn.default})`,
1458-
),
1459-
)
1460-
} else {
1461-
downQueries.push(
1462-
new Query(
1463-
`ALTER TABLE ${this.escapePath(
1464-
table,
1465-
)} ALTER ("${
1466-
oldColumn.name
1467-
}" ${this.connection.driver.createFullType(
1468-
oldColumn,
1469-
)} DEFAULT NULL)`,
1470-
),
1471-
)
1472-
}
1473-
} else if (
1474-
oldColumn.default !== null &&
1475-
oldColumn.default !== undefined
1476-
) {
1477-
upQueries.push(
1478-
new Query(
1479-
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
1480-
newColumn.name
1481-
}" ${this.connection.driver.createFullType(
1482-
newColumn,
1483-
)} DEFAULT NULL)`,
1484-
),
1485-
)
1486-
downQueries.push(
1487-
new Query(
1488-
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
1489-
oldColumn.name
1490-
}" ${this.connection.driver.createFullType(
1491-
oldColumn,
1492-
)} DEFAULT ${oldColumn.default})`,
1493-
),
1494-
)
1495-
}
1496-
}
1497-
14981459
await this.executeQueries(upQueries, downQueries)
14991460
this.replaceCachedTable(table, clonedTable)
15001461
}
@@ -2766,7 +2727,9 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
27662727
dbColumn["DEFAULT_VALUE"]
27672728
}
27682729
}
2769-
tableColumn.comment = "" // dbColumn["COLUMN_COMMENT"];
2730+
if (dbColumn["COMMENTS"]) {
2731+
tableColumn.comment = dbColumn["COMMENTS"]
2732+
}
27702733
if (dbColumn["character_set_name"])
27712734
tableColumn.charset =
27722735
dbColumn["character_set_name"]
@@ -3292,6 +3255,19 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
32923255
)
32933256
}
32943257

3258+
/**
3259+
* Escapes a given comment so it's safe to include in a query.
3260+
*/
3261+
protected escapeComment(comment?: string) {
3262+
if (!comment) {
3263+
return "NULL"
3264+
}
3265+
3266+
comment = comment.replace(/'/g, "''").replace(/\u0000/g, "") // Null bytes aren't allowed in comments
3267+
3268+
return `'${comment}'`
3269+
}
3270+
32953271
/**
32963272
* Escapes given table or view path.
32973273
*/
@@ -3305,57 +3281,37 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
33053281
return `"${tableName}"`
33063282
}
33073283

3308-
/**
3309-
* Concat database name and schema name to the foreign key name.
3310-
* Needs because FK name is relevant to the schema and database.
3311-
*/
3312-
protected buildForeignKeyName(
3313-
fkName: string,
3314-
schemaName: string | undefined,
3315-
dbName: string | undefined,
3316-
): string {
3317-
let joinedFkName = fkName
3318-
if (schemaName) joinedFkName = schemaName + "." + joinedFkName
3319-
if (dbName) joinedFkName = dbName + "." + joinedFkName
3320-
3321-
return joinedFkName
3322-
}
3323-
3324-
/**
3325-
* Removes parenthesis around default value.
3326-
* Sql server returns default value with parenthesis around, e.g.
3327-
* ('My text') - for string
3328-
* ((1)) - for number
3329-
* (newsequentialId()) - for function
3330-
*/
3331-
protected removeParenthesisFromDefault(defaultValue: any): any {
3332-
if (defaultValue.substr(0, 1) !== "(") return defaultValue
3333-
const normalizedDefault = defaultValue.substr(
3334-
1,
3335-
defaultValue.lastIndexOf(")") - 1,
3336-
)
3337-
return this.removeParenthesisFromDefault(normalizedDefault)
3338-
}
3339-
33403284
/**
33413285
* Builds a query for create column.
33423286
*/
3343-
protected buildCreateColumnSql(column: TableColumn) {
3287+
protected buildCreateColumnSql(
3288+
column: TableColumn,
3289+
explicitDefault?: boolean,
3290+
explicitNullable?: boolean,
3291+
) {
33443292
let c =
33453293
`"${column.name}" ` + this.connection.driver.createFullType(column)
33463294
if (column.charset) c += " CHARACTER SET " + column.charset
33473295
if (column.collation) c += " COLLATE " + column.collation
3348-
if (column.default !== undefined && column.default !== null)
3349-
// DEFAULT must be placed before NOT NULL
3296+
if (column.default !== undefined && column.default !== null) {
33503297
c += " DEFAULT " + column.default
3351-
if (column.isNullable !== true && !column.isGenerated)
3298+
} else if (explicitDefault) {
3299+
c += " DEFAULT NULL"
3300+
}
3301+
if (!column.isGenerated) {
33523302
// NOT NULL is not supported with GENERATED
3353-
c += " NOT NULL"
3303+
if (column.isNullable !== true) c += " NOT NULL"
3304+
else if (explicitNullable) c += " NULL"
3305+
}
33543306
if (
33553307
column.isGenerated === true &&
33563308
column.generationStrategy === "increment"
3357-
)
3309+
) {
33583310
c += " GENERATED ALWAYS AS IDENTITY"
3311+
}
3312+
if (column.comment) {
3313+
c += ` COMMENT ${this.escapeComment(column.comment)}`
3314+
}
33593315

33603316
return c
33613317
}

‎test/functional/columns/comments/columns-comments.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ describe("columns > comments", () => {
1414
async () =>
1515
(connections = await createTestingConnections({
1616
entities: [Test],
17-
// Only supported on postgres, cockroachdb, and mysql
18-
enabledDrivers: ["postgres", "cockroachdb", "mysql"],
17+
// Only supported on cockroachdb, mysql, postgres, and sap
18+
enabledDrivers: ["cockroachdb", "mysql", "postgres", "sap"],
1919
})),
2020
)
2121
beforeEach(() => reloadTestingDatabases(connections))

‎test/functional/commands/migration-create.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,16 @@ describe("commands - migration create", () => {
2525
let connectionOptionsReader: ConnectionOptionsReader
2626
let baseConnectionOptions: DataSourceOptions
2727

28-
const enabledDrivers = [
29-
"postgres",
28+
const enabledDrivers: DatabaseType[] = [
29+
"better-sqlite3",
30+
"cockroachdb",
31+
"mariadb",
3032
"mssql",
3133
"mysql",
32-
"mariadb",
33-
"sqlite",
34-
"better-sqlite3",
3534
"oracle",
36-
"cockroachdb",
37-
] as DatabaseType[]
35+
"postgres",
36+
"sqlite",
37+
]
3838

3939
// simulate args: `npm run typeorm migration:run -- -n test-migration -d test-directory`
4040
const testHandlerArgs = (options: Record<string, any>) => ({
@@ -70,7 +70,7 @@ describe("commands - migration create", () => {
7070
})
7171

7272
afterEach(async () => {
73-
getConnectionOptionsStub.restore()
73+
getConnectionOptionsStub?.restore()
7474
})
7575

7676
it("should write regular empty migration file when no option is passed", async () => {

‎test/functional/schema-builder/add-column.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ describe("schema builder > add column", () => {
3333
}
3434

3535
let stringType = "varchar"
36-
if (connection.driver.options.type === "spanner") {
36+
if (connection.driver.options.type === "sap") {
37+
stringType = "nvarchar"
38+
} else if (connection.driver.options.type === "spanner") {
3739
stringType = "string"
3840
}
3941

0 commit comments

Comments
 (0)