mirror of
https://github.com/chartdb/chartdb.git
synced 2025-10-23 07:11:56 +00:00
fix(sql-import): support ALTER TABLE ALTER COLUMN TYPE in PostgreSQL importer (#895)
This commit is contained in:
Guy Ben-Aharon
GitHub
@@ -0,0 +1,118 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { fromPostgres } from '../postgresql';
|
||||
|
||||
describe('PostgreSQL ALTER TABLE ALTER COLUMN TYPE', () => {
|
||||
it('should handle ALTER TABLE ALTER COLUMN TYPE statements', async () => {
|
||||
const sql = `
|
||||
CREATE SCHEMA IF NOT EXISTS "public";
|
||||
|
||||
CREATE TABLE "public"."table_12" (
|
||||
"id" SERIAL,
|
||||
"field1" varchar(200),
|
||||
"field2" varchar(200),
|
||||
"field3" varchar(200),
|
||||
PRIMARY KEY ("id")
|
||||
);
|
||||
|
||||
ALTER TABLE table_12 ALTER COLUMN field1 TYPE VARCHAR(254);
|
||||
ALTER TABLE table_12 ALTER COLUMN field2 TYPE VARCHAR(254);
|
||||
ALTER TABLE table_12 ALTER COLUMN field3 TYPE VARCHAR(254);
|
||||
`;
|
||||
|
||||
const result = await fromPostgres(sql);
|
||||
|
||||
expect(result.tables).toHaveLength(1);
|
||||
const table = result.tables[0];
|
||||
|
||||
expect(table.name).toBe('table_12');
|
||||
expect(table.columns).toHaveLength(4); // id, field1, field2, field3
|
||||
|
||||
// Check that the columns have the updated type
|
||||
const field1 = table.columns.find((col) => col.name === 'field1');
|
||||
expect(field1).toBeDefined();
|
||||
expect(field1?.type).toBe('VARCHAR(254)'); // Should be updated from 200 to 254
|
||||
|
||||
const field2 = table.columns.find((col) => col.name === 'field2');
|
||||
expect(field2).toBeDefined();
|
||||
expect(field2?.type).toBe('VARCHAR(254)');
|
||||
|
||||
const field3 = table.columns.find((col) => col.name === 'field3');
|
||||
expect(field3).toBeDefined();
|
||||
expect(field3?.type).toBe('VARCHAR(254)');
|
||||
});
|
||||
|
||||
it('should handle various ALTER COLUMN TYPE scenarios', async () => {
|
||||
const sql = `
|
||||
CREATE TABLE test_table (
|
||||
id INTEGER PRIMARY KEY,
|
||||
name VARCHAR(50),
|
||||
age SMALLINT,
|
||||
score NUMERIC(5,2)
|
||||
);
|
||||
|
||||
-- Change varchar length
|
||||
ALTER TABLE test_table ALTER COLUMN name TYPE VARCHAR(100);
|
||||
|
||||
-- Change numeric type
|
||||
ALTER TABLE test_table ALTER COLUMN age TYPE INTEGER;
|
||||
|
||||
-- Change precision
|
||||
ALTER TABLE test_table ALTER COLUMN score TYPE NUMERIC(10,4);
|
||||
`;
|
||||
|
||||
const result = await fromPostgres(sql);
|
||||
|
||||
const table = result.tables[0];
|
||||
|
||||
const nameCol = table.columns.find((col) => col.name === 'name');
|
||||
expect(nameCol?.type).toBe('VARCHAR(100)');
|
||||
|
||||
const ageCol = table.columns.find((col) => col.name === 'age');
|
||||
expect(ageCol?.type).toBe('INTEGER');
|
||||
|
||||
const scoreCol = table.columns.find((col) => col.name === 'score');
|
||||
expect(scoreCol?.type).toBe('NUMERIC(10,4)');
|
||||
});
|
||||
|
||||
it('should handle multiple type changes on the same column', async () => {
|
||||
const sql = `
|
||||
CREATE SCHEMA IF NOT EXISTS "public";
|
||||
|
||||
CREATE TABLE "public"."table_12" (
|
||||
"id" SERIAL,
|
||||
"field1" varchar(200),
|
||||
"field2" varchar(200),
|
||||
"field3" varchar(200),
|
||||
PRIMARY KEY ("id")
|
||||
);
|
||||
|
||||
ALTER TABLE table_12 ALTER COLUMN field1 TYPE VARCHAR(254);
|
||||
ALTER TABLE table_12 ALTER COLUMN field2 TYPE VARCHAR(254);
|
||||
ALTER TABLE table_12 ALTER COLUMN field3 TYPE VARCHAR(254);
|
||||
ALTER TABLE table_12 ALTER COLUMN field1 TYPE BIGINT;
|
||||
`;
|
||||
|
||||
const result = await fromPostgres(sql);
|
||||
|
||||
expect(result.tables).toHaveLength(1);
|
||||
const table = result.tables[0];
|
||||
|
||||
expect(table.name).toBe('table_12');
|
||||
expect(table.schema).toBe('public');
|
||||
expect(table.columns).toHaveLength(4);
|
||||
|
||||
// Check that field1 has the final type (BIGINT), not the intermediate VARCHAR(254)
|
||||
const field1 = table.columns.find((col) => col.name === 'field1');
|
||||
expect(field1).toBeDefined();
|
||||
expect(field1?.type).toBe('BIGINT'); // Should be BIGINT, not VARCHAR(254)
|
||||
|
||||
// Check that field2 and field3 still have VARCHAR(254)
|
||||
const field2 = table.columns.find((col) => col.name === 'field2');
|
||||
expect(field2).toBeDefined();
|
||||
expect(field2?.type).toBe('VARCHAR(254)');
|
||||
|
||||
const field3 = table.columns.find((col) => col.name === 'field3');
|
||||
expect(field3).toBeDefined();
|
||||
expect(field3?.type).toBe('VARCHAR(254)');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,117 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { fromPostgres } from '../postgresql';
|
||||
|
||||
describe('PostgreSQL ALTER TABLE with Foreign Keys', () => {
|
||||
it('should handle ALTER TABLE ADD COLUMN followed by ALTER TABLE ADD FOREIGN KEY', async () => {
|
||||
const sql = `
|
||||
CREATE SCHEMA IF NOT EXISTS "public";
|
||||
|
||||
CREATE TABLE "public"."location" (
|
||||
"id" bigint NOT NULL,
|
||||
CONSTRAINT "pk_table_7_id" PRIMARY KEY ("id")
|
||||
);
|
||||
|
||||
-- Add new fields to existing location table
|
||||
ALTER TABLE location ADD COLUMN country_id INT;
|
||||
ALTER TABLE location ADD COLUMN state_id INT;
|
||||
ALTER TABLE location ADD COLUMN location_type_id INT;
|
||||
ALTER TABLE location ADD COLUMN city_id INT;
|
||||
ALTER TABLE location ADD COLUMN street TEXT;
|
||||
ALTER TABLE location ADD COLUMN block TEXT;
|
||||
ALTER TABLE location ADD COLUMN building TEXT;
|
||||
ALTER TABLE location ADD COLUMN floor TEXT;
|
||||
ALTER TABLE location ADD COLUMN apartment TEXT;
|
||||
ALTER TABLE location ADD COLUMN lat INT;
|
||||
ALTER TABLE location ADD COLUMN long INT;
|
||||
ALTER TABLE location ADD COLUMN elevation INT;
|
||||
ALTER TABLE location ADD COLUMN erp_site_id INT;
|
||||
ALTER TABLE location ADD COLUMN is_active TEXT;
|
||||
ALTER TABLE location ADD COLUMN remarks TEXT;
|
||||
|
||||
-- Create lookup tables
|
||||
CREATE TABLE country (
|
||||
id SERIAL PRIMARY KEY,
|
||||
name VARCHAR(100) NOT NULL,
|
||||
code VARCHAR(3) UNIQUE
|
||||
);
|
||||
|
||||
CREATE TABLE state (
|
||||
id SERIAL PRIMARY KEY,
|
||||
name VARCHAR(100) NOT NULL,
|
||||
country_id INT NOT NULL,
|
||||
FOREIGN KEY (country_id) REFERENCES country(id)
|
||||
);
|
||||
|
||||
CREATE TABLE location_type (
|
||||
id SERIAL PRIMARY KEY,
|
||||
name VARCHAR(100) NOT NULL
|
||||
);
|
||||
|
||||
CREATE TABLE city (
|
||||
id SERIAL PRIMARY KEY,
|
||||
name VARCHAR(100) NOT NULL,
|
||||
state_id INT NOT NULL,
|
||||
FOREIGN KEY (state_id) REFERENCES state(id)
|
||||
);
|
||||
|
||||
-- Add foreign key constraints from location to lookup tables
|
||||
ALTER TABLE location ADD CONSTRAINT fk_location_country
|
||||
FOREIGN KEY (country_id) REFERENCES country(id);
|
||||
ALTER TABLE location ADD CONSTRAINT fk_location_state
|
||||
FOREIGN KEY (state_id) REFERENCES state(id);
|
||||
ALTER TABLE location ADD CONSTRAINT fk_location_location_type
|
||||
FOREIGN KEY (location_type_id) REFERENCES location_type(id);
|
||||
ALTER TABLE location ADD CONSTRAINT fk_location_city
|
||||
FOREIGN KEY (city_id) REFERENCES city(id);
|
||||
`;
|
||||
|
||||
const result = await fromPostgres(sql);
|
||||
|
||||
const locationTable = result.tables.find((t) => t.name === 'location');
|
||||
|
||||
// Check tables
|
||||
expect(result.tables).toHaveLength(5); // location, country, state, location_type, city
|
||||
|
||||
// Check location table has all columns
|
||||
expect(locationTable).toBeDefined();
|
||||
expect(locationTable?.columns).toHaveLength(16); // id + 15 added columns
|
||||
|
||||
// Check foreign key relationships
|
||||
const locationRelationships = result.relationships.filter(
|
||||
(r) => r.sourceTable === 'location'
|
||||
);
|
||||
|
||||
// Should have 4 FKs from location to lookup tables + 2 from state/city
|
||||
expect(result.relationships.length).toBeGreaterThanOrEqual(6);
|
||||
|
||||
// Check specific foreign keys from location
|
||||
expect(
|
||||
locationRelationships.some(
|
||||
(r) =>
|
||||
r.sourceColumn === 'country_id' &&
|
||||
r.targetTable === 'country'
|
||||
)
|
||||
).toBe(true);
|
||||
|
||||
expect(
|
||||
locationRelationships.some(
|
||||
(r) =>
|
||||
r.sourceColumn === 'state_id' && r.targetTable === 'state'
|
||||
)
|
||||
).toBe(true);
|
||||
|
||||
expect(
|
||||
locationRelationships.some(
|
||||
(r) =>
|
||||
r.sourceColumn === 'location_type_id' &&
|
||||
r.targetTable === 'location_type'
|
||||
)
|
||||
).toBe(true);
|
||||
|
||||
expect(
|
||||
locationRelationships.some(
|
||||
(r) => r.sourceColumn === 'city_id' && r.targetTable === 'city'
|
||||
)
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -91,6 +91,7 @@ export interface AlterTableExprItem {
|
||||
action: string;
|
||||
resource?: string;
|
||||
type?: string;
|
||||
keyword?: string;
|
||||
constraint?: { constraint_type?: string };
|
||||
// Properties for ADD COLUMN
|
||||
column?:
|
||||
|
||||
@@ -1029,8 +1029,117 @@ export async function fromPostgres(
|
||||
// Process ALTER TABLE expressions
|
||||
if (alterTableStmt.expr && Array.isArray(alterTableStmt.expr)) {
|
||||
alterTableStmt.expr.forEach((expr: AlterTableExprItem) => {
|
||||
// Handle both 'add' action and 'add column' type
|
||||
if (expr.action === 'add' && expr.resource === 'column') {
|
||||
// Handle ALTER COLUMN TYPE
|
||||
if (expr.action === 'alter' && expr.resource === 'column') {
|
||||
// Extract column name
|
||||
let columnName: string | undefined;
|
||||
if (
|
||||
typeof expr.column === 'object' &&
|
||||
'column' in expr.column
|
||||
) {
|
||||
const innerColumn = expr.column.column;
|
||||
if (
|
||||
typeof innerColumn === 'object' &&
|
||||
'expr' in innerColumn &&
|
||||
innerColumn.expr?.value
|
||||
) {
|
||||
columnName = innerColumn.expr.value;
|
||||
} else if (typeof innerColumn === 'string') {
|
||||
columnName = innerColumn;
|
||||
}
|
||||
} else if (typeof expr.column === 'string') {
|
||||
columnName = expr.column;
|
||||
}
|
||||
|
||||
// Check if it's a TYPE change
|
||||
if (
|
||||
columnName &&
|
||||
expr.type === 'alter' &&
|
||||
expr.definition?.dataType
|
||||
) {
|
||||
// Find the column in the table and update its type
|
||||
const column = table.columns.find(
|
||||
(col) => (col as SQLColumn).name === columnName
|
||||
);
|
||||
if (column) {
|
||||
const definition = expr.definition;
|
||||
const rawDataType = String(definition.dataType);
|
||||
|
||||
// console.log('ALTER TYPE expr:', JSON.stringify(expr, null, 2));
|
||||
|
||||
// Normalize the type
|
||||
let normalizedType =
|
||||
normalizePostgreSQLType(rawDataType);
|
||||
|
||||
// Handle type parameters
|
||||
if (
|
||||
definition.scale !== undefined &&
|
||||
definition.scale !== null
|
||||
) {
|
||||
// For NUMERIC/DECIMAL with scale, length is actually precision
|
||||
const precision =
|
||||
definition.length ||
|
||||
definition.precision;
|
||||
normalizedType = `${normalizedType}(${precision},${definition.scale})`;
|
||||
} else if (
|
||||
definition.length !== undefined &&
|
||||
definition.length !== null
|
||||
) {
|
||||
normalizedType = `${normalizedType}(${definition.length})`;
|
||||
} else if (definition.precision !== undefined) {
|
||||
normalizedType = `${normalizedType}(${definition.precision})`;
|
||||
} else if (
|
||||
definition.suffix &&
|
||||
Array.isArray(definition.suffix) &&
|
||||
definition.suffix.length > 0
|
||||
) {
|
||||
const params = definition.suffix
|
||||
.map((s: unknown) => {
|
||||
if (
|
||||
typeof s === 'object' &&
|
||||
s !== null &&
|
||||
'value' in s
|
||||
) {
|
||||
return String(s.value);
|
||||
}
|
||||
return String(s);
|
||||
})
|
||||
.join(',');
|
||||
normalizedType = `${normalizedType}(${params})`;
|
||||
}
|
||||
|
||||
// Update the column type
|
||||
(column as SQLColumn).type = normalizedType;
|
||||
|
||||
// Update typeArgs if applicable
|
||||
if (
|
||||
definition.scale !== undefined &&
|
||||
definition.scale !== null
|
||||
) {
|
||||
// For NUMERIC/DECIMAL with scale
|
||||
const precision =
|
||||
definition.length ||
|
||||
definition.precision;
|
||||
(column as SQLColumn).typeArgs = {
|
||||
precision: precision,
|
||||
scale: definition.scale,
|
||||
};
|
||||
} else if (definition.length) {
|
||||
(column as SQLColumn).typeArgs = {
|
||||
length: definition.length,
|
||||
};
|
||||
} else if (definition.precision) {
|
||||
(column as SQLColumn).typeArgs = {
|
||||
precision: definition.precision,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
// Handle ADD COLUMN
|
||||
} else if (
|
||||
expr.action === 'add' &&
|
||||
expr.resource === 'column'
|
||||
) {
|
||||
// Handle ADD COLUMN directly from expr structure
|
||||
// Extract column name from the nested structure
|
||||
let columnName: string | undefined;
|
||||
@@ -1457,7 +1566,56 @@ export async function fromPostgres(
|
||||
} else if (stmt.type === 'alter' && !stmt.parsed) {
|
||||
// Handle ALTER TABLE statements that failed to parse
|
||||
|
||||
// First try to extract ADD COLUMN statements
|
||||
// First try to extract ALTER COLUMN TYPE statements
|
||||
const alterTypeMatch = stmt.sql.match(
|
||||
/ALTER\s+TABLE\s+(?:ONLY\s+)?(?:(?:"([^"]+)"|([^"\s.]+))\.)?(?:"([^"]+)"|([^"\s.(]+))\s+ALTER\s+COLUMN\s+(?:"([^"]+)"|([^"\s]+))\s+TYPE\s+([\w_]+(?:\([^)]*\))?(?:\[\])?)/i
|
||||
);
|
||||
|
||||
if (alterTypeMatch) {
|
||||
const schemaName =
|
||||
alterTypeMatch[1] || alterTypeMatch[2] || 'public';
|
||||
const tableName = alterTypeMatch[3] || alterTypeMatch[4];
|
||||
const columnName = alterTypeMatch[5] || alterTypeMatch[6];
|
||||
let columnType = alterTypeMatch[7];
|
||||
|
||||
const table = findTableWithSchemaSupport(
|
||||
tables,
|
||||
tableName,
|
||||
schemaName
|
||||
);
|
||||
if (table && columnName) {
|
||||
const column = (table.columns as SQLColumn[]).find(
|
||||
(col) => col.name === columnName
|
||||
);
|
||||
if (column) {
|
||||
// Normalize and update the type
|
||||
columnType = normalizePostgreSQLType(columnType);
|
||||
column.type = columnType;
|
||||
|
||||
// Extract and update typeArgs if present
|
||||
const typeMatch = columnType.match(
|
||||
/^(\w+)(?:\(([^)]+)\))?$/
|
||||
);
|
||||
if (typeMatch && typeMatch[2]) {
|
||||
const params = typeMatch[2]
|
||||
.split(',')
|
||||
.map((p) => p.trim());
|
||||
if (params.length === 1) {
|
||||
column.typeArgs = {
|
||||
length: parseInt(params[0]),
|
||||
};
|
||||
} else if (params.length === 2) {
|
||||
column.typeArgs = {
|
||||
precision: parseInt(params[0]),
|
||||
scale: parseInt(params[1]),
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Then try to extract ADD COLUMN statements
|
||||
const alterColumnMatch = stmt.sql.match(
|
||||
/ALTER\s+TABLE\s+(?:ONLY\s+)?(?:(?:"([^"]+)"|([^"\s.]+))\.)?(?:"([^"]+)"|([^"\s.(]+))\s+ADD\s+COLUMN\s+(?:"([^"]+)"|([^"\s]+))\s+([\w_]+(?:\([^)]*\))?(?:\[\])?)/i
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user