From c96466e6fbc9532d49fe2b29d9a058d84ce9cf59 Mon Sep 17 00:00:00 2001 From: Guy Ben-Aharon Date: Wed, 24 Dec 2025 12:58:03 +0200 Subject: [PATCH] fix: ddl import auto increment (#1035) --- .../sql-import/__tests__/sql-import.test.ts | 148 ++++++++++++++++++ .../postgresql/postgresql.ts | 55 ++++--- 2 files changed, 180 insertions(+), 23 deletions(-) diff --git a/src/lib/data/sql-import/__tests__/sql-import.test.ts b/src/lib/data/sql-import/__tests__/sql-import.test.ts index 5a527331..83d06325 100644 --- a/src/lib/data/sql-import/__tests__/sql-import.test.ts +++ b/src/lib/data/sql-import/__tests__/sql-import.test.ts @@ -410,4 +410,152 @@ CREATE TABLE playlists ( const customerIdField = fields?.find((f) => f.name === 'customer_id'); expect(customerIdField?.type.name).toBe('int'); }); + + it('should parse PostgreSQL table with GENERATED BY DEFAULT AS IDENTITY, decimal precision/scale, and various constraints', async () => { + const sql = ` + CREATE TABLE "accounting"."invoices" ( + "id" INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + "code" varchar(5) NOT NULL, + "number" int NOT NULL, + "fk_customer" int NOT NULL, + "fk_operation" int NOT NULL, + "fk_provider" int NOT NULL, + "issue_date" date NOT NULL, + "reference" varchar(128), + "suggested_amount" decimal(15,2), + "gross_amount" decimal(15,2) NOT NULL, + "fk_type" int NOT NULL, + "tax_rate_1" decimal(13,4), + "tax_rate_2" decimal(13,4), + "tax_rate_3" decimal(13,4), + "net_amount" decimal(15,2), + "paid_amount" decimal(15,2), + "payment_date" date, + "created_at" timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updated_at" timestamp, + "fk_created_by" int NOT NULL, + "fk_updated_by" int, + "fk_status" int NOT NULL DEFAULT 1 + ); + `; + + const diagram = await sqlImportToDiagram({ + sqlContent: sql, + sourceDatabaseType: DatabaseType.POSTGRESQL, + targetDatabaseType: DatabaseType.POSTGRESQL, + }); + + // Verify diagram structure + expect(diagram).toBeDefined(); + expect(diagram.databaseType).toBe(DatabaseType.POSTGRESQL); + + // Verify table was parsed + expect(diagram.tables).toHaveLength(1); + const table = diagram.tables?.[0]; + expect(table?.name).toBe('invoices'); + expect(table?.schema).toBe('accounting'); + + // Verify all fields were parsed (22 columns) + const fields = table?.fields; + expect(fields).toHaveLength(22); + + // Verify id - GENERATED BY DEFAULT AS IDENTITY should be: + // - type: integer (INT is normalized to integer) + // - primaryKey: true + // - nullable: false (primary keys are not nullable) + // - increment: true (GENERATED BY DEFAULT AS IDENTITY marks it as auto-increment) + const pkField = fields?.find((f) => f.name === 'id'); + expect(pkField).toBeDefined(); + expect(pkField?.type.name).toBe('int'); + expect(pkField?.primaryKey).toBe(true); + expect(pkField?.nullable).toBe(false); + expect(pkField?.increment).toBe(true); + + // Verify number - int NOT NULL (should NOT be auto-increment) + const numberField = fields?.find((f) => f.name === 'number'); + expect(numberField).toBeDefined(); + expect(numberField?.type.name).toBe('int'); + expect(numberField?.nullable).toBe(false); + expect(numberField?.increment).toBeFalsy(); + + // Verify code - varchar(5) NOT NULL + const codeField = fields?.find((f) => f.name === 'code'); + expect(codeField).toBeDefined(); + expect(codeField?.type.name).toBe('varchar'); + expect(codeField?.nullable).toBe(false); + expect(codeField?.characterMaximumLength).toBe('5'); + + // Verify reference - varchar(128) nullable + const referenceField = fields?.find((f) => f.name === 'reference'); + expect(referenceField).toBeDefined(); + expect(referenceField?.type.name).toBe('varchar'); + expect(referenceField?.nullable).toBe(true); + expect(referenceField?.characterMaximumLength).toBe('128'); + + // Verify gross_amount - decimal(15,2) NOT NULL + // decimal is normalized to numeric in PostgreSQL + const grossAmountField = fields?.find((f) => f.name === 'gross_amount'); + expect(grossAmountField).toBeDefined(); + expect(grossAmountField?.type.name).toBe('numeric'); + expect(grossAmountField?.type.id).toBe('numeric'); + expect(grossAmountField?.nullable).toBe(false); + expect(grossAmountField?.precision).toBe(15); + expect(grossAmountField?.scale).toBe(2); + + // Verify tax_rate_1 - decimal(13,4) nullable + const taxRate1Field = fields?.find((f) => f.name === 'tax_rate_1'); + expect(taxRate1Field).toBeDefined(); + expect(taxRate1Field?.type.name).toBe('numeric'); + expect(taxRate1Field?.nullable).toBe(true); + expect(taxRate1Field?.precision).toBe(13); + expect(taxRate1Field?.scale).toBe(4); + + // Verify issue_date - date NOT NULL + const issueDateField = fields?.find((f) => f.name === 'issue_date'); + expect(issueDateField).toBeDefined(); + expect(issueDateField?.type.name).toBe('date'); + expect(issueDateField?.nullable).toBe(false); + + // Verify payment_date - date nullable + const paymentDateField = fields?.find((f) => f.name === 'payment_date'); + expect(paymentDateField).toBeDefined(); + expect(paymentDateField?.type.name).toBe('date'); + expect(paymentDateField?.nullable).toBe(true); + + // Verify created_at - timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP + const createdAtField = fields?.find((f) => f.name === 'created_at'); + expect(createdAtField).toBeDefined(); + expect(createdAtField?.type.name).toBe('timestamp'); + expect(createdAtField?.nullable).toBe(false); + expect(createdAtField?.default).toBe('CURRENT_TIMESTAMP'); + + // Verify updated_at - timestamp nullable (no default) + const updatedAtField = fields?.find((f) => f.name === 'updated_at'); + expect(updatedAtField).toBeDefined(); + expect(updatedAtField?.type.name).toBe('timestamp'); + expect(updatedAtField?.nullable).toBe(true); + + // Verify fk_status - int NOT NULL DEFAULT 1 + const fkStatusField = fields?.find((f) => f.name === 'fk_status'); + expect(fkStatusField).toBeDefined(); + expect(fkStatusField?.type.name).toBe('int'); + expect(fkStatusField?.nullable).toBe(false); + expect(fkStatusField?.default).toBe('1'); + + // Verify fk_updated_by - int nullable (no NOT NULL constraint) + const fkUpdatedByField = fields?.find( + (f) => f.name === 'fk_updated_by' + ); + expect(fkUpdatedByField).toBeDefined(); + expect(fkUpdatedByField?.type.name).toBe('int'); + expect(fkUpdatedByField?.nullable).toBe(true); + + // Verify fk_created_by - int NOT NULL + const fkCreatedByField = fields?.find( + (f) => f.name === 'fk_created_by' + ); + expect(fkCreatedByField).toBeDefined(); + expect(fkCreatedByField?.type.name).toBe('int'); + expect(fkCreatedByField?.nullable).toBe(false); + }); }); diff --git a/src/lib/data/sql-import/dialect-importers/postgresql/postgresql.ts b/src/lib/data/sql-import/dialect-importers/postgresql/postgresql.ts index a8c39a04..d2fc3564 100644 --- a/src/lib/data/sql-import/dialect-importers/postgresql/postgresql.ts +++ b/src/lib/data/sql-import/dialect-importers/postgresql/postgresql.ts @@ -267,6 +267,23 @@ function isSerialTypeName(typeName: string): boolean { return SERIAL_TYPES.has(typeName.toUpperCase().split('(')[0]); } +/** + * Check if a specific column has GENERATED AS IDENTITY syntax in the SQL + * @param sql The SQL statement containing the column definition + * @param columnName The name of the column to check + * @returns true if the column has GENERATED AS IDENTITY + */ +function hasGeneratedIdentity(sql: string, columnName: string): boolean { + // Create a regex pattern to find the column definition + // Match the column name (quoted or unquoted) followed by its definition until the next comma or closing paren + const escapedName = columnName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const pattern = new RegExp( + `["']?${escapedName}["']?\\s+[^,)]*GENERATED\\s+(?:BY\\s+DEFAULT|ALWAYS)\\s+AS\\s+IDENTITY`, + 'i' + ); + return pattern.test(sql); +} + /** * Normalize PostgreSQL type syntax to lowercase canonical form. * This function handles parsing-level normalization only - it converts @@ -984,10 +1001,11 @@ export async function fromPostgres( columns.push({ name: columnName, type: finalDataType, - nullable: isSerialType - ? false - : columnDef.nullable?.type !== - 'not null', + nullable: + isSerialType || isPrimaryKey + ? false + : columnDef.nullable?.type !== + 'not null', primaryKey: isPrimaryKey || isSerialType, unique: columnDef.unique === 'unique', typeArgs: getTypeArgs(columnDef.definition), @@ -998,13 +1016,11 @@ export async function fromPostgres( isSerialType || columnDef.auto_increment === 'auto_increment' || - // Check if the SQL contains GENERATED IDENTITY for this column - (stmt.sql - .toUpperCase() - .includes('GENERATED') && - stmt.sql - .toUpperCase() - .includes('IDENTITY')), + // Check if the SQL contains GENERATED IDENTITY for this specific column + hasGeneratedIdentity( + stmt.sql, + columnName + ), }); } } else if (def.resource === 'constraint') { @@ -1403,12 +1419,7 @@ export async function fromPostgres( isSerialType || definition?.auto_increment === 'auto_increment' || - (stmt.sql - .toUpperCase() - .includes('GENERATED') && - stmt.sql - .toUpperCase() - .includes('IDENTITY')), + hasGeneratedIdentity(stmt.sql, columnName), }; // Add the column to the table if it doesn't already exist @@ -1528,12 +1539,10 @@ export async function fromPostgres( isSerialType || columnDef.auto_increment === 'auto_increment' || - (stmt.sql - .toUpperCase() - .includes('GENERATED') && - stmt.sql - .toUpperCase() - .includes('IDENTITY')), + hasGeneratedIdentity( + stmt.sql, + columnName + ), }; // Add the column to the table if it doesn't already exist