Skip to content

Commit

Permalink
feat: implement column comments for SAP HANA (#10502)
Browse files Browse the repository at this point in the history 
* feat: implement column comments for SAP HANA

* test: improve schema tests for SAP HANA
  • Loading branch information
@alumni
alumni committed Dec 29, 2023
1 parent 7e9cead commit 45e31cc
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 126 deletions.
16 changes: 14 additions & 2 deletions src/driver/sap/SapDriver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class SapDriver implements Driver {
cacheTime: "bigint",
cacheDuration: "integer",
cacheQuery: "nvarchar(5000)" as any,
cacheResult: "text",
cacheResult: "nclob",
metadataType: "nvarchar",
metadataDatabase: "nvarchar",
metadataSchema: "nvarchar",
Expand Down Expand Up @@ -769,7 +769,8 @@ export class SapDriver implements Driver {
this.getColumnLength(columnMetadata)) ||
tableColumn.precision !== columnMetadata.precision ||
tableColumn.scale !== columnMetadata.scale ||
// || tableColumn.comment !== columnMetadata.comment || // todo
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
tableColumn.isPrimary !== columnMetadata.isPrimary ||
Expand Down Expand Up @@ -841,4 +842,15 @@ export class SapDriver implements Driver {
)
}
}

/**
* Escapes a given comment.
*/
protected escapeComment(comment?: string) {
if (!comment) return comment

comment = comment.replace(/\u0000/g, "") // Null bytes aren't allowed in comments

return comment
}
}
176 changes: 66 additions & 110 deletions src/driver/sap/SapQueryRunner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1263,19 +1263,48 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
oldColumn.name = newColumn.name
}

if (this.isColumnChanged(oldColumn, newColumn)) {
if (this.isColumnChanged(oldColumn, newColumn, true)) {
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(
table,
)} ALTER (${this.buildCreateColumnSql(newColumn)})`,
)} ALTER (${this.buildCreateColumnSql(
newColumn,
!(
oldColumn.default === null ||
oldColumn.default === undefined
),
!oldColumn.isNullable,
)})`,
),
)
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(
table,
)} ALTER (${this.buildCreateColumnSql(oldColumn)})`,
)} ALTER (${this.buildCreateColumnSql(
oldColumn,
!(
newColumn.default === null ||
newColumn.default === undefined
),
!newColumn.isNullable,
)})`,
),
)
} else if (oldColumn.comment !== newColumn.comment) {
upQueries.push(
new Query(
`COMMENT ON COLUMN ${this.escapePath(table)}."${
oldColumn.name
}" IS ${this.escapeComment(newColumn.comment)}`,
),
)
downQueries.push(
new Query(
`COMMENT ON COLUMN ${this.escapePath(table)}."${
newColumn.name
}" IS ${this.escapeComment(oldColumn.comment)}`,
),
)
}
Expand Down Expand Up @@ -1427,74 +1456,6 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
}
}

if (newColumn.default !== oldColumn.default) {
if (
newColumn.default !== null &&
newColumn.default !== undefined
) {
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
newColumn.name
}" ${this.connection.driver.createFullType(
newColumn,
)} DEFAULT ${newColumn.default})`,
),
)

if (
oldColumn.default !== null &&
oldColumn.default !== undefined
) {
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(
table,
)} ALTER ("${
oldColumn.name
}" ${this.connection.driver.createFullType(
oldColumn,
)} DEFAULT ${oldColumn.default})`,
),
)
} else {
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(
table,
)} ALTER ("${
oldColumn.name
}" ${this.connection.driver.createFullType(
oldColumn,
)} DEFAULT NULL)`,
),
)
}
} else if (
oldColumn.default !== null &&
oldColumn.default !== undefined
) {
upQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
newColumn.name
}" ${this.connection.driver.createFullType(
newColumn,
)} DEFAULT NULL)`,
),
)
downQueries.push(
new Query(
`ALTER TABLE ${this.escapePath(table)} ALTER ("${
oldColumn.name
}" ${this.connection.driver.createFullType(
oldColumn,
)} DEFAULT ${oldColumn.default})`,
),
)
}
}

await this.executeQueries(upQueries, downQueries)
this.replaceCachedTable(table, clonedTable)
}
Expand Down Expand Up @@ -2766,7 +2727,9 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
dbColumn["DEFAULT_VALUE"]
}
}
tableColumn.comment = "" // dbColumn["COLUMN_COMMENT"];
if (dbColumn["COMMENTS"]) {
tableColumn.comment = dbColumn["COMMENTS"]
}
if (dbColumn["character_set_name"])
tableColumn.charset =
dbColumn["character_set_name"]
Expand Down Expand Up @@ -3292,6 +3255,19 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
)
}

/**
* Escapes a given comment so it's safe to include in a query.
*/
protected escapeComment(comment?: string) {
if (!comment) {
return "NULL"
}

comment = comment.replace(/'/g, "''").replace(/\u0000/g, "") // Null bytes aren't allowed in comments

return `'${comment}'`
}

/**
* Escapes given table or view path.
*/
Expand All @@ -3305,57 +3281,37 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
return `"${tableName}"`
}

/**
* Concat database name and schema name to the foreign key name.
* Needs because FK name is relevant to the schema and database.
*/
protected buildForeignKeyName(
fkName: string,
schemaName: string | undefined,
dbName: string | undefined,
): string {
let joinedFkName = fkName
if (schemaName) joinedFkName = schemaName + "." + joinedFkName
if (dbName) joinedFkName = dbName + "." + joinedFkName

return joinedFkName
}

/**
* Removes parenthesis around default value.
* Sql server returns default value with parenthesis around, e.g.
* ('My text') - for string
* ((1)) - for number
* (newsequentialId()) - for function
*/
protected removeParenthesisFromDefault(defaultValue: any): any {
if (defaultValue.substr(0, 1) !== "(") return defaultValue
const normalizedDefault = defaultValue.substr(
1,
defaultValue.lastIndexOf(")") - 1,
)
return this.removeParenthesisFromDefault(normalizedDefault)
}

/**
* Builds a query for create column.
*/
protected buildCreateColumnSql(column: TableColumn) {
protected buildCreateColumnSql(
column: TableColumn,
explicitDefault?: boolean,
explicitNullable?: boolean,
) {
let c =
`"${column.name}" ` + this.connection.driver.createFullType(column)
if (column.charset) c += " CHARACTER SET " + column.charset
if (column.collation) c += " COLLATE " + column.collation
if (column.default !== undefined && column.default !== null)
// DEFAULT must be placed before NOT NULL
if (column.default !== undefined && column.default !== null) {
c += " DEFAULT " + column.default
if (column.isNullable !== true && !column.isGenerated)
} else if (explicitDefault) {
c += " DEFAULT NULL"
}
if (!column.isGenerated) {
// NOT NULL is not supported with GENERATED
c += " NOT NULL"
if (column.isNullable !== true) c += " NOT NULL"
else if (explicitNullable) c += " NULL"
}
if (
column.isGenerated === true &&
column.generationStrategy === "increment"
)
) {
c += " GENERATED ALWAYS AS IDENTITY"
}
if (column.comment) {
c += ` COMMENT ${this.escapeComment(column.comment)}`
}

return c
}
Expand Down
4 changes: 2 additions & 2 deletions test/functional/columns/comments/columns-comments.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe("columns > comments", () => {
async () =>
(connections = await createTestingConnections({
entities: [Test],
// Only supported on postgres, cockroachdb, and mysql
enabledDrivers: ["postgres", "cockroachdb", "mysql"],
// Only supported on cockroachdb, mysql, postgres, and sap
enabledDrivers: ["cockroachdb", "mysql", "postgres", "sap"],
})),
)
beforeEach(() => reloadTestingDatabases(connections))
Expand Down
16 changes: 8 additions & 8 deletions test/functional/commands/migration-create.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ describe("commands - migration create", () => {
let connectionOptionsReader: ConnectionOptionsReader
let baseConnectionOptions: DataSourceOptions

const enabledDrivers = [
"postgres",
const enabledDrivers: DatabaseType[] = [
"better-sqlite3",
"cockroachdb",
"mariadb",
"mssql",
"mysql",
"mariadb",
"sqlite",
"better-sqlite3",
"oracle",
"cockroachdb",
] as DatabaseType[]
"postgres",
"sqlite",
]

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

afterEach(async () => {
getConnectionOptionsStub.restore()
getConnectionOptionsStub?.restore()
})

it("should write regular empty migration file when no option is passed", async () => {
Expand Down
4 changes: 3 additions & 1 deletion test/functional/schema-builder/add-column.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ describe("schema builder > add column", () => {
}

let stringType = "varchar"
if (connection.driver.options.type === "spanner") {
if (connection.driver.options.type === "sap") {
stringType = "nvarchar"
} else if (connection.driver.options.type === "spanner") {
stringType = "string"
}

Expand Down
Loading

0 comments on commit 45e31cc

Please sign in to comment.