Skip to content

Commit 0d3183f

Browse files
committed
Fix postgres and CockroachDB diff cases that write failing SQL around moves, renames, roles and generated columns
Postgres: - Count the brackets of a custom type's SQL type as array dimensions. - Drop and create the foreign keys of a recreated generated column one time each, and create its indexes and unique constraints one time. - Rename a view in its old schema before it moves, and move it with its old kind. - Drop a recreated view after its rename or move. - Run the policy drops, foreign key drops and row level security statements of a moved table after `SET SCHEMA`. - Run `DROP ROLE` after the policy statements. - Run `DROP EXPRESSION` first and write no `USING` clause for a generated column. - Find the enum columns that a format 7 snapshot stores without a type schema when the enum is recreated, with the s06 tests and fixtures. CockroachDB: - Drop a view that reads a renamed or moved table before the table statement and create it after it. Replace `recreate_view` with a `drop_view` that has a `cause`, in the diff, in push and in `cockroachExplain`. - Write both statements for a view or a sequence that moves and gets a new name, and look up the old view of a moved view that is recreated. - Name the table in an index rename or drop when two tables hold the index name. - Write `RENAME CONSTRAINT` for a check rename. - Select the row of the renamed check, index, primary key, foreign key or policy by its table. - Change the column type when only the type schema or the array dimensions change, and count the brackets of a custom type's SQL type. - Run table renames and moves before the statements that name the table in its new place. - Create the foreign keys of a recreated generated column again. - Run `DROP ROLE` after the policy statements.
1 parent cca6ce3 commit 0d3183f

28 files changed

Lines changed: 1853 additions & 131 deletions

‎drizzle-kit/src/cli/commands/push-cockroach.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export const suggestions = async (db: DB, jsonStatements: JsonStatement[], hints
186186

187187
const filtered = jsonStatements.filter((it) => {
188188
// discussion -
189-
if (it.type === 'recreate_view') return false;
189+
if (it.type === 'drop_view' && it.cause) return false;
190190

191191
/*
192192
drizzle-kit push does not handle alternations of views definitions

‎drizzle-kit/src/cli/views.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,8 @@ export const cockroachExplain = (st: StatementCockroach) => {
507507
if (d.columns) cause += `│ columns: [${d.columns.from.join(',')}] -> [${d.columns.to.join(',')}]\n`;
508508
}
509509

510-
if (st.type === 'recreate_view') {
511-
const { from, to } = st;
510+
if (st.type === 'drop_view' && st.cause) {
511+
const { view: from, cause: to } = st;
512512

513513
const key = `${to.schema}.${to.name}`;
514514
title += `${key} view changed:`;

‎drizzle-kit/src/dialects/cockroach/convertor.ts‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,6 @@ const moveViewConvertor = convertor('move_view', (st) => {
6262
return `ALTER${view.materialized ? ' MATERIALIZED' : ''} VIEW ${from} SET SCHEMA "${toSchema}";`;
6363
});
6464

65-
const recreateViewConvertor = convertor('recreate_view', (st) => {
66-
const drop = dropViewConvertor.convert({ view: st.from }) as string;
67-
const create = createViewConvertor.convert({ view: st.to }) as string;
68-
return [drop, create];
69-
});
70-
7165
const createTableConvertor = convertor('create_table', (st) => {
7266
const { schema, name, columns, pk, checks, policies, isRlsEnabled, indexes } = st.table;
7367

@@ -245,24 +239,30 @@ const alterColumnConvertor = convertor('alter_column', (st) => {
245239

246240
const key = column.schema !== 'public' ? `"${column.schema}"."${column.table}"` : `"${column.table}"`;
247241

242+
// `hasNewType` also counts a change of only `typeSchema`, because an enum with the same name in another
243+
// schema is another type. A change of only `dimensions` gets a plain cast. Cockroach has no cast between
244+
// most scalar types and their array types, so the user writes that conversion into the USING clause.
245+
const hasNewType = !!(diff.type || diff.typeSchema || diff.dimensions);
248246
// TODO need to recheck this
249-
const recreateDefault = diff.type && (isEnum || wasEnum) && (diff.$left.default);
247+
const recreateDefault = hasNewType && (isEnum || wasEnum || diff.dimensions) && (diff.$left.default);
250248
if (recreateDefault) {
251249
statements.push(`ALTER TABLE ${key} ALTER COLUMN "${column.name}" DROP DEFAULT;`);
252250
}
253251

254-
if (diff.type) {
252+
if (hasNewType) {
255253
const typeSchema = column.typeSchema && column.typeSchema !== 'public' ? `"${column.typeSchema}".` : '';
256254
const textProxy = wasEnum && isEnum ? 'text::' : ''; // using enum1::text::enum2
257255
const suffix = isEnum
258256
? ` USING "${column.name}"::${textProxy}${typeSchema}"${column.type}"${'[]'.repeat(column.dimensions)}`
257+
: diff.dimensions
258+
? ` USING "${column.name}"::${column.type}${'[]'.repeat(column.dimensions)}`
259259
: '';
260260

261-
const type = diff.typeSchema?.to && diff.typeSchema.to !== 'public'
262-
? `"${diff.typeSchema.to}"."${diff.type.to}"`
261+
const type = typeSchema
262+
? `${typeSchema}"${column.type}"`
263263
: isEnum
264-
? `"${diff.type.to}"`
265-
: diff.type.to;
264+
? `"${column.type}"`
265+
: column.type;
266266

267267
statements.push(
268268
`ALTER TABLE ${key} ALTER COLUMN "${column.name}" SET DATA TYPE ${type}${
@@ -387,21 +387,25 @@ const dropIndexConvertor = convertor('drop_index', (st) => {
387387
const key = index.schema !== 'public'
388388
? `"${index.schema}".`
389389
: '';
390+
const table = st.isNameShared ? `"${index.table}"@` : '';
390391
391-
return `DROP INDEX ${key}"${st.index.name}"${cascade};`;
392+
return `DROP INDEX ${key}${table}"${st.index.name}"${cascade};`;
392393
});
393394
394395
const recreateIndexConvertor = convertor('recreate_index', (st) => {
395396
const { diff } = st;
396-
const drop = dropIndexConvertor.convert({ index: diff.$right }) as string;
397+
// `isNameShared` is false because the index renames and the index drops run before this statement. At this
398+
// point only one table of the schema holds the name of this index.
399+
const drop = dropIndexConvertor.convert({ index: diff.$right, isNameShared: false }) as string;
397400
const create = createIndexConvertor.convert({ index: diff.$right, newTable: false }) as string;
398401
return [drop, create];
399402
});
400403
401404
const renameIndexConvertor = convertor('rename_index', (st) => {
402-
const key = st.schema !== 'public' ? `"${st.schema}"."${st.from}"` : `"${st.from}"`;
405+
const key = st.schema !== 'public' ? `"${st.schema}".` : '';
406+
const table = st.isNameShared ? `"${st.table}"@` : '';
403407
404-
return `ALTER INDEX ${key} RENAME TO "${st.to}";`;
408+
return `ALTER INDEX ${key}${table}"${st.from}" RENAME TO "${st.to}";`;
405409
});
406410
407411
const addPrimaryKeyConvertor = convertor('add_pk', (st) => {
@@ -723,7 +727,6 @@ const convertors = [
723727
dropViewConvertor,
724728
renameViewConvertor,
725729
moveViewConvertor,
726-
recreateViewConvertor,
727730
createTableConvertor,
728731
dropTableConvertor,
729732
renameTableConvertor,

0 commit comments

Comments
 (0)