From 50e4516a1c59dafa5c1e32061ef7dd8a4da369d5 Mon Sep 17 00:00:00 2001 From: Makara Sok Date: Fri, 12 Jan 2024 10:18:06 -0500 Subject: [PATCH] Comment introspection implementation for mssql, mysql and postgres Signed-off-by: Makara Sok --- src/dialect/database-introspector.ts | 1 + src/dialect/mssql/mssql-introspector.ts | 23 +++++++ src/dialect/mysql/mysql-introspector.ts | 3 + src/dialect/postgres/postgres-introspector.ts | 4 +- src/dialect/sqlite/sqlite-introspector.ts | 1 + test/node/src/introspect.test.ts | 68 +++++++++++++++++++ test/node/src/schema.test.ts | 9 +++ test/node/src/test-setup.ts | 31 ++++++++- 8 files changed, 136 insertions(+), 4 deletions(-) diff --git a/src/dialect/database-introspector.ts b/src/dialect/database-introspector.ts index 2ef58ea10..75093b811 100644 --- a/src/dialect/database-introspector.ts +++ b/src/dialect/database-introspector.ts @@ -68,4 +68,5 @@ export interface ColumnMetadata { readonly isAutoIncrementing: boolean readonly isNullable: boolean readonly hasDefaultValue: boolean + readonly comment?: string } diff --git a/src/dialect/mssql/mssql-introspector.ts b/src/dialect/mssql/mssql-introspector.ts index 0cc4ab798..680455fc8 100644 --- a/src/dialect/mssql/mssql-introspector.ts +++ b/src/dialect/mssql/mssql-introspector.ts @@ -48,6 +48,13 @@ export class MssqlIntrospector implements DatabaseIntrospector { 'type_schemas.schema_id', 'types.schema_id' ) + .leftJoin( + 'sys.extended_properties as comments', + join => join + .onRef('comments.major_id', '=', 'tables.object_id') + .onRef('comments.minor_id', '=', 'columns.column_id') + .on('comments.name', '=', 'MS_Description') + ) .$if(!options.withInternalKyselyTables, (qb) => qb .where('tables.name', '!=', DEFAULT_MIGRATION_TABLE) @@ -74,6 +81,7 @@ export class MssqlIntrospector implements DatabaseIntrospector { 'types.is_nullable as type_is_nullable', 'types.name as type_name', 'type_schemas.name as type_schema_name', + 'comments.value as column_comment' ]) .unionAll( this.#db @@ -98,6 +106,13 @@ export class MssqlIntrospector implements DatabaseIntrospector { 'type_schemas.schema_id', 'types.schema_id' ) + .leftJoin( + 'sys.extended_properties as comments', + join => join + .onRef('comments.major_id', '=', 'views.object_id') + .onRef('comments.minor_id', '=', 'columns.column_id') + .on('comments.name', '=', 'MS_Description') + ) .select([ 'views.name as table_name', 'views.type as table_type', @@ -112,6 +127,7 @@ export class MssqlIntrospector implements DatabaseIntrospector { 'types.is_nullable as type_is_nullable', 'types.name as type_name', 'type_schemas.name as type_schema_name', + 'comments.value as column_comment' ]) ) .orderBy('table_schema_name') @@ -147,6 +163,7 @@ export class MssqlIntrospector implements DatabaseIntrospector { isNullable: rawColumn.column_is_nullable && rawColumn.type_is_nullable, name: rawColumn.column_name, + comment: rawColumn.column_comment ?? undefined, }) ) } @@ -204,6 +221,12 @@ interface MssqlSysTables { // scale: number system_type_id: number } + 'sys.extended_properties': { + major_id: number + minor_id: number + name: string + value: string + } 'sys.schemas': { name: string // principal_id: number diff --git a/src/dialect/mysql/mysql-introspector.ts b/src/dialect/mysql/mysql-introspector.ts index 19dd3f836..8af6118f8 100644 --- a/src/dialect/mysql/mysql-introspector.ts +++ b/src/dialect/mysql/mysql-introspector.ts @@ -50,6 +50,7 @@ export class MysqlIntrospector implements DatabaseIntrospector { 'columns.IS_NULLABLE', 'columns.DATA_TYPE', 'columns.EXTRA', + 'columns.COLUMN_COMMENT', ]) .where('columns.TABLE_SCHEMA', '=', sql`database()`) .orderBy('columns.TABLE_NAME') @@ -96,6 +97,7 @@ export class MysqlIntrospector implements DatabaseIntrospector { isNullable: it.IS_NULLABLE === 'YES', isAutoIncrementing: it.EXTRA.toLowerCase().includes('auto_increment'), hasDefaultValue: it.COLUMN_DEFAULT !== null, + comment: it.COLUMN_COMMENT === '' ? undefined : it.COLUMN_COMMENT }) ) @@ -117,4 +119,5 @@ interface RawColumnMetadata { IS_NULLABLE: 'YES' | 'NO' DATA_TYPE: string EXTRA: string + COLUMN_COMMENT: string } diff --git a/src/dialect/postgres/postgres-introspector.ts b/src/dialect/postgres/postgres-introspector.ts index 579afff51..369bbbfb4 100644 --- a/src/dialect/postgres/postgres-introspector.ts +++ b/src/dialect/postgres/postgres-introspector.ts @@ -57,7 +57,7 @@ export class PostgresIntrospector implements DatabaseIntrospector { 'ns.nspname as schema', 'typ.typname as type', 'dtns.nspname as type_schema', - + sql`col_description(a.attrelid, a.attnum)`.as('column_description'), // Detect if the column is auto incrementing by finding the sequence // that is created for `serial` and `bigserial` columns. this.#db @@ -126,6 +126,7 @@ export class PostgresIntrospector implements DatabaseIntrospector { isNullable: !it.not_null, isAutoIncrementing: !!it.auto_incrementing, hasDefaultValue: it.has_default, + comment: it.column_description ?? undefined }) ) @@ -148,4 +149,5 @@ interface RawColumnMetadata { type: string type_schema: string auto_incrementing: boolean | null + column_description: string | null } diff --git a/src/dialect/sqlite/sqlite-introspector.ts b/src/dialect/sqlite/sqlite-introspector.ts index e86cfa67a..d375ac6da 100644 --- a/src/dialect/sqlite/sqlite-introspector.ts +++ b/src/dialect/sqlite/sqlite-introspector.ts @@ -94,6 +94,7 @@ export class SqliteIntrospector implements DatabaseIntrospector { isNullable: !col.notnull, isAutoIncrementing: col.name === autoIncrementCol, hasDefaultValue: col.dflt_value != null, + comment: undefined })), } } diff --git a/test/node/src/introspect.test.ts b/test/node/src/introspect.test.ts index f2fe4e116..96904c63a 100644 --- a/test/node/src/introspect.test.ts +++ b/test/node/src/introspect.test.ts @@ -94,6 +94,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: true, + comment: undefined }, { name: 'first_name', @@ -102,6 +103,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'middle_name', @@ -110,6 +112,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { @@ -119,6 +122,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'gender', @@ -127,6 +131,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'marital_status', @@ -135,6 +140,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'children', @@ -143,6 +149,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: true, + comment: undefined }, ], }, @@ -158,6 +165,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: true, + comment: undefined }, { name: 'name', @@ -166,6 +174,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'owner_id', @@ -174,6 +183,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'species', @@ -182,6 +192,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, ], }, @@ -197,6 +208,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: true, + comment: undefined }, { name: 'name', @@ -205,6 +217,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'pet_id', @@ -213,6 +226,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'price', @@ -221,6 +235,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: 'Price in USD' }, ], }, @@ -236,6 +251,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, ], }, @@ -251,6 +267,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: true, + comment: undefined }, { dataType: 'species', @@ -259,6 +276,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'spcies', + comment: undefined }, ], }, @@ -276,6 +294,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'first_name', @@ -283,6 +302,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'middle_name', @@ -290,6 +310,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'last_name', @@ -297,6 +318,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { @@ -305,6 +327,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'marital_status', @@ -312,6 +335,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'children', @@ -319,6 +343,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: true, + comment: undefined }, ], }, @@ -333,6 +358,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'name', @@ -340,6 +366,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'owner_id', @@ -347,6 +374,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'species', @@ -354,6 +382,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, ], }, @@ -368,6 +397,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'name', @@ -375,6 +405,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'pet_id', @@ -382,6 +413,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'price', @@ -389,6 +421,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: 'Price in USD' }, ], }, @@ -403,6 +436,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'name', + comment: undefined }, ], }, @@ -421,6 +455,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'children', + comment: undefined }, { dataType: 'varchar', @@ -429,6 +464,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'first_name', + comment: undefined }, { dataType: 'varchar', @@ -437,6 +473,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'gender', + comment: undefined }, { dataType: 'int', @@ -445,6 +482,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: true, isNullable: false, name: 'id', + comment: undefined }, { dataType: 'varchar', @@ -453,6 +491,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'last_name', + comment: undefined }, { dataType: 'varchar', @@ -461,6 +500,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'marital_status', + comment: undefined }, { dataType: 'varchar', @@ -469,6 +509,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'middle_name', + comment: undefined }, ], }, @@ -484,6 +525,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: true, isNullable: false, name: 'id', + comment: undefined }, { dataType: 'varchar', @@ -492,6 +534,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'name', + comment: undefined }, { dataType: 'int', @@ -500,6 +543,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'owner_id', + comment: undefined }, { dataType: 'varchar', @@ -508,6 +552,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'species', + comment: undefined }, ], }, @@ -523,6 +568,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: true, isNullable: false, name: 'id', + comment: undefined }, { dataType: 'varchar', @@ -531,6 +577,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'name', + comment: undefined }, { dataType: 'int', @@ -539,6 +586,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'pet_id', + comment: undefined }, { dataType: 'float', @@ -547,6 +595,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'price', + comment: 'Price in USD', }, ], }, @@ -562,6 +611,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: false, name: 'name', + comment: undefined }, ], }, @@ -577,6 +627,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: true, isNullable: false, name: 'some_column', + comment: undefined }, { dataType: 'int', @@ -585,6 +636,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'some_column_plus_1', + comment: undefined }, ], }, @@ -601,6 +653,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'first_name', @@ -608,6 +661,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'middle_name', @@ -615,6 +669,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'last_name', @@ -622,6 +677,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { @@ -630,6 +686,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'marital_status', @@ -637,6 +694,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'children', @@ -644,6 +702,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: true, + comment: undefined }, ], }, @@ -657,6 +716,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'name', @@ -664,6 +724,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'owner_id', @@ -671,6 +732,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'species', @@ -678,6 +740,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, ], }, @@ -691,6 +754,7 @@ for (const dialect of DIALECTS) { isNullable: true, isAutoIncrementing: true, hasDefaultValue: false, + comment: undefined }, { name: 'name', @@ -698,6 +762,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'pet_id', @@ -705,6 +770,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, { name: 'price', @@ -712,6 +778,7 @@ for (const dialect of DIALECTS) { isNullable: false, isAutoIncrementing: false, hasDefaultValue: false, + comment: undefined }, ], }, @@ -725,6 +792,7 @@ for (const dialect of DIALECTS) { isAutoIncrementing: false, isNullable: true, name: 'name', + comment: undefined }, ], }, diff --git a/test/node/src/schema.test.ts b/test/node/src/schema.test.ts index 0541d0162..098d1190e 100644 --- a/test/node/src/schema.test.ts +++ b/test/node/src/schema.test.ts @@ -116,6 +116,7 @@ for (const dialect of DIALECTS) { await builder.execute() expect(await getColumnMeta('test.a')).to.eql({ + comment: undefined, dataType: 'int4', dataTypeSchema: 'pg_catalog', isAutoIncrementing: true, @@ -125,6 +126,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.b')).to.eql({ + comment: undefined, dataType: 'int4', dataTypeSchema: 'pg_catalog', isAutoIncrementing: false, @@ -134,6 +136,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.l')).to.eql({ + comment: undefined, dataType: 'bool', dataTypeSchema: 'pg_catalog', isAutoIncrementing: false, @@ -265,6 +268,7 @@ for (const dialect of DIALECTS) { await builder.execute() expect(await getColumnMeta('test.a')).to.eql({ + comment: undefined, dataType: 'int', isAutoIncrementing: true, isNullable: false, @@ -273,6 +277,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.b')).to.eql({ + comment: undefined, dataType: 'int', isAutoIncrementing: false, isNullable: true, @@ -281,6 +286,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.k')).to.eql({ + comment: undefined, dataType: 'tinyint', isAutoIncrementing: false, isNullable: false, @@ -443,6 +449,7 @@ for (const dialect of DIALECTS) { await builder.execute() expect(await getColumnMeta('test.a')).to.eql({ + comment: undefined, dataType: 'INTEGER', isAutoIncrementing: true, isNullable: false, @@ -451,6 +458,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.b')).to.eql({ + comment: undefined, dataType: 'INTEGER', isAutoIncrementing: false, isNullable: true, @@ -459,6 +467,7 @@ for (const dialect of DIALECTS) { }) expect(await getColumnMeta('test.l')).to.eql({ + comment: undefined, dataType: 'boolean', isAutoIncrementing: false, isNullable: false, diff --git a/test/node/src/test-setup.ts b/test/node/src/test-setup.ts index 96dbd7c0e..c2910c973 100644 --- a/test/node/src/test-setup.ts +++ b/test/node/src/test-setup.ts @@ -317,12 +317,37 @@ async function createDatabase( ) .addColumn('species', 'varchar(50)', (col) => col.notNull()) .execute() - - await createTableWithId(db.schema, dialect, 'toy') + + const createToyTableBase = createTableWithId(db.schema, dialect, 'toy') .addColumn('name', 'varchar(255)', (col) => col.notNull()) .addColumn('pet_id', 'integer', (col) => col.references('pet.id').notNull()) - .addColumn('price', 'double precision', (col) => col.notNull()) + + if (dialect === 'postgres') { + await createToyTableBase + .addColumn('price', 'double precision', (col) => col.notNull()) + .execute() + await sql`COMMENT ON COLUMN toy.price IS 'Price in USD';`.execute(db) + } + + if (dialect === 'mssql') { + await createToyTableBase + .addColumn('price', 'double precision', (col) => col.notNull()) + .execute() + await sql`EXECUTE sp_addextendedproperty N'MS_Description', N'Price in USD', N'SCHEMA', N'dbo', N'TABLE', 'toy', N'COLUMN', N'price'`.execute(db) + } + + if (dialect === 'mysql') { + await createToyTableBase + .addColumn('price', 'double precision', (col) => col.notNull().modifyEnd(sql`comment ${sql.lit('Price in USD')}`)) .execute() + } + + if (dialect === 'sqlite') { + // there is no way to add a comment + await createToyTableBase + .addColumn('price', 'double precision', (col) => col.notNull()) + .execute() + } await db.schema .createIndex('pet_owner_id_index')