Skip to content

Commit ba0e9dc

Browse files
bplunkett-stripebplunkett-stripe
committed
Mat views indexdes
1 parent 083748e commit ba0e9dc

File tree

2 files changed

+119
-14
lines changed

2 files changed

+119
-14
lines changed

‎pkg/diff/materialized_view_sql_generator_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package diff
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stripe/pg-schema-diff/internal/schema"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestMaterializedViewSQLGenerator_Add_WithTablespace(t *testing.T) {
11+
generator := newMaterializedViewSQLVertexGenerator()
12+
13+
mv := schema.MaterializedView{
14+
SchemaQualifiedName: schema.SchemaQualifiedName{
15+
SchemaName: "public",
16+
EscapedName: "\"test_view\"",
17+
},
18+
ViewDefinition: "SELECT id, name FROM users",
19+
Tablespace: "custom_tablespace",
20+
Options: map[string]string{},
21+
TableDependencies: []schema.TableDependency{
22+
{
23+
SchemaQualifiedName: schema.SchemaQualifiedName{
24+
SchemaName: "public",
25+
EscapedName: "\"users\"",
26+
},
27+
},
28+
},
29+
}
30+
31+
result, err := generator.Add(mv)
32+
assert.NoError(t, err)
33+
assert.Len(t, result.vertices, 1)
34+
35+
expectedDDL := `CREATE MATERIALIZED VIEW "public"."test_view" TABLESPACE "custom_tablespace" AS
36+
SELECT id, name FROM users`
37+
assert.Equal(t, expectedDDL, result.vertices[0].statements[0].DDL)
38+
}
39+
40+
func TestMaterializedViewSQLGenerator_Add_WithoutTablespace(t *testing.T) {
41+
generator := newMaterializedViewSQLVertexGenerator()
42+
43+
mv := schema.MaterializedView{
44+
SchemaQualifiedName: schema.SchemaQualifiedName{
45+
SchemaName: "public",
46+
EscapedName: "\"test_view\"",
47+
},
48+
ViewDefinition: "SELECT id, name FROM users",
49+
Tablespace: "", // No tablespace
50+
Options: map[string]string{},
51+
TableDependencies: []schema.TableDependency{
52+
{
53+
SchemaQualifiedName: schema.SchemaQualifiedName{
54+
SchemaName: "public",
55+
EscapedName: "\"users\"",
56+
},
57+
},
58+
},
59+
}
60+
61+
result, err := generator.Add(mv)
62+
assert.NoError(t, err)
63+
assert.Len(t, result.vertices, 1)
64+
65+
expectedDDL := `CREATE MATERIALIZED VIEW "public"."test_view" AS
66+
SELECT id, name FROM users`
67+
assert.Equal(t, expectedDDL, result.vertices[0].statements[0].DDL)
68+
}

‎pkg/diff/sql_generator.go

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -225,15 +225,24 @@ func buildSchemaDiff(old, new schema.Schema) (schemaDiff, bool, error) {
225225
if err != nil {
226226
return schemaDiff{}, false, fmt.Errorf("diffing tables: %w", err)
227227
}
228-
229228
newSchemaTablesByName := buildSchemaObjByNameMap(new.Tables)
230229
addedTablesByName := buildSchemaObjByNameMap(tableDiffs.adds)
231230
deletedTablesByName := buildSchemaObjByNameMap(tableDiffs.deletes)
232231
tableDiffsByName := buildDiffByNameMap[schema.Table, tableDiff](tableDiffs.alters)
232+
233+
materializedViewDiffs, err := diffLists(old.MaterializedViews, new.MaterializedViews, func(old, new schema.MaterializedView, _, _ int) (diff materializedViewDiff, requiresRecreation bool, error error) {
234+
return buildMaterializedViewDiff(deletedTablesByName, tableDiffsByName, old, new)
235+
})
236+
if err != nil {
237+
return schemaDiff{}, false, fmt.Errorf("diffing materialized views: %w", err)
238+
}
239+
addedMatViewsByName := buildSchemaObjByNameMap(materializedViewDiffs.adds)
240+
233241
indexesDiff, err := diffLists(old.Indexes, new.Indexes, func(oldIndex, newIndex schema.Index, _, _ int) (indexDiff, bool, error) {
234242
return buildIndexDiff(indexDiffConfig{
235243
newSchemaTablesByName: newSchemaTablesByName,
236244
addedTablesByName: addedTablesByName,
245+
addedMatViewsByName: addedMatViewsByName,
237246
oldSchemaIndexesByName: buildSchemaObjByNameMap(old.Indexes),
238247
newSchemaIndexesByName: buildSchemaObjByNameMap(new.Indexes),
239248
}, oldIndex, newIndex)
@@ -319,13 +328,6 @@ func buildSchemaDiff(old, new schema.Schema) (schemaDiff, bool, error) {
319328
return schemaDiff{}, false, fmt.Errorf("diffing views: %w", err)
320329
}
321330

322-
materializedViewDiffs, err := diffLists(old.MaterializedViews, new.MaterializedViews, func(old, new schema.MaterializedView, _, _ int) (diff materializedViewDiff, requiresRecreation bool, error error) {
323-
return buildMaterializedViewDiff(deletedTablesByName, tableDiffsByName, old, new)
324-
})
325-
if err != nil {
326-
return schemaDiff{}, false, fmt.Errorf("diffing materialized views: %w", err)
327-
}
328-
329331
return schemaDiff{
330332
oldAndNew: oldAndNew[schema.Schema]{
331333
old: old,
@@ -430,6 +432,7 @@ func buildTableDiff(oldTable, newTable schema.Table, _, _ int) (diff tableDiff,
430432
type indexDiffConfig struct {
431433
newSchemaTablesByName map[string]schema.Table
432434
addedTablesByName map[string]schema.Table
435+
addedMatViewsByName map[string]schema.MaterializedView
433436

434437
// oldSchemaIndexesByName and newSchemaIndexesByName by name are hackaround because the diff function does not yet support hierarchies
435438
oldSchemaIndexesByName map[string]schema.Index
@@ -456,6 +459,10 @@ func buildIndexDiff(deps indexDiffConfig, old, new schema.Index) (diff indexDiff
456459
// re-created). In other words, an index must be re-created if the owning table is re-created
457460
return indexDiff{}, true, nil
458461
}
462+
if _, isOnNewMatView := deps.addedMatViewsByName[new.OwningTable.GetName()]; isOnNewMatView {
463+
// If the materialized view is new, then the index must be re-created
464+
return indexDiff{}, true, nil
465+
}
459466

460467
if old.ParentIdx == nil {
461468
// If the old index didn't belong to a partitioned index (and the new index does), we can resolve the parent
@@ -531,6 +538,8 @@ func (s schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
531538
tablesInNewSchemaByName := buildSchemaObjByNameMap(diff.new.Tables)
532539
deletedTablesByName := buildSchemaObjByNameMap(diff.tableDiffs.deletes)
533540
addedTablesByName := buildSchemaObjByNameMap(diff.tableDiffs.adds)
541+
deletedMatViewsByName := buildSchemaObjByNameMap(diff.materializedViewDiffs.deletes)
542+
addedMatViewsByName := buildSchemaObjByNameMap(diff.materializedViewDiffs.adds)
534543
functionsInNewSchemaByName := buildSchemaObjByNameMap(diff.new.Functions)
535544

536545
namedSchemaStatements, err := diff.namedSchemaDiffs.resolveToSQLGroupedByEffect(&namedSchemaSQLGenerator{})
@@ -576,9 +585,13 @@ func (s schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
576585
partialGraph = concatPartialGraphs(partialGraph, renameConflictingIndexesPartialGraph)
577586

578587
indexGenerator := legacyToNewSqlVertexGenerator[schema.Index, indexDiff](&indexSQLVertexGenerator{
579-
deletedTablesByName: deletedTablesByName,
580-
addedTablesByName: addedTablesByName,
581-
tablesInNewSchemaByName: tablesInNewSchemaByName,
588+
deletedTablesByName: deletedTablesByName,
589+
addedTablesByName: addedTablesByName,
590+
tablesInNewSchemaByName: tablesInNewSchemaByName,
591+
592+
deletedMatViewsByName: deletedMatViewsByName,
593+
addedMatViewsByName: addedMatViewsByName,
594+
582595
indexesInNewSchemaByName: buildSchemaObjByNameMap(diff.new.Indexes),
583596

584597
renameSQLVertexGenerator: renameConflictingIndexesGenerator,
@@ -1567,6 +1580,12 @@ type indexSQLVertexGenerator struct {
15671580
// tablesInNewSchemaByName is a map of table name to tables (and partitions) in the new schema.
15681581
// These tables are not necessarily new. This is used to identify if the table is partitioned
15691582
tablesInNewSchemaByName map[string]schema.Table
1583+
1584+
// deletedMatViewsByName is a map of materialized view name to the delete materialized view
1585+
deletedMatViewsByName map[string]schema.MaterializedView
1586+
// addedMatViewsByName is a map of materialiezd view name to added materialized view.
1587+
addedMatViewsByName map[string]schema.MaterializedView
1588+
15701589
// indexesInNewSchemaByName is a map of index name to the index
15711590
// This is used to identify the parent index is a primary key
15721591
indexesInNewSchemaByName map[string]schema.Index
@@ -1586,6 +1605,9 @@ func (isg *indexSQLVertexGenerator) Add(index schema.Index) ([]Statement, error)
15861605
if _, isNewTable := isg.addedTablesByName[index.OwningTable.GetName()]; isNewTable {
15871606
stmts = stripMigrationHazards(stmts...)
15881607
}
1608+
if _, isNewMatView := isg.addedMatViewsByName[index.OwningTable.GetName()]; isNewMatView {
1609+
stmts = stripMigrationHazards(stmts...)
1610+
}
15891611
return stmts, nil
15901612
}
15911613

@@ -1656,9 +1678,12 @@ func (isg *indexSQLVertexGenerator) addIdxStmtsWithHazards(index schema.Index) (
16561678
}
16571679

16581680
func (isg *indexSQLVertexGenerator) Delete(index schema.Index) ([]Statement, error) {
1659-
_, tableWasDeleted := isg.deletedTablesByName[index.OwningTable.GetName()]
1660-
// An index will be dropped if its owning table is dropped.
1661-
if tableWasDeleted {
1681+
if _, tableWasDeleted := isg.deletedTablesByName[index.OwningTable.GetName()]; tableWasDeleted {
1682+
// An index will be dropped if its owning table is dropped.
1683+
return nil, nil
1684+
}
1685+
if _, matViewWasDeleted := isg.deletedMatViewsByName[index.OwningTable.GetName()]; matViewWasDeleted {
1686+
// An index will be dropped if its owning materialized view is dropped.
16621687
return nil, nil
16631688
}
16641689

@@ -1832,7 +1857,11 @@ func (isg *indexSQLVertexGenerator) GetAddAlterDependencies(index, _ schema.Inde
18321857

18331858
func (isg *indexSQLVertexGenerator) GetDeleteDependencies(index schema.Index) ([]dependency, error) {
18341859
dependencies := []dependency{
1860+
// Technically, only the table -or- materialized view will exist, but there is no harm in having a dependency
1861+
// on both.
18351862
mustRun(isg.GetSQLVertexId(index, diffTypeDelete)).after(buildTableVertexId(index.OwningTable, diffTypeDelete)),
1863+
mustRun(isg.GetSQLVertexId(index, diffTypeDelete)).after(buildMaterializedViewVertexId(index.OwningTable, diffTypeDelete)),
1864+
18361865
// Drop the index after it has been potentially renamed
18371866
mustRun(isg.GetSQLVertexId(index, diffTypeDelete)).after(buildRenameConflictingIndexVertexId(index.GetSchemaQualifiedName(), diffTypeAddAlter)),
18381867
}
@@ -1850,12 +1879,20 @@ func (isg *indexSQLVertexGenerator) GetDeleteDependencies(index schema.Index) ([
18501879

18511880
func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index schema.Index) []dependency {
18521881
// This could be cleaner if start sorting columns separately in the graph
1882+
// TODO(bplunkett) - can below just be switched to deleted tables by name
18531883
parentTable, ok := isg.tablesInNewSchemaByName[index.OwningTable.GetName()]
18541884
if !ok {
18551885
// If the parent table is deleted, we don't need to worry about making the index statement come
18561886
// before any alters
18571887
return nil
18581888
}
1889+
if _, deletedMatView := isg.deletedMatViewsByName[index.OwningTable.GetName()]; deletedMatView {
1890+
// If the parent materialized view is deleted, we don't need to worry about making the index statement come
1891+
// before any alters.
1892+
return nil
1893+
}
1894+
1895+
// TODO(bplunkett) - add conditinoal switches for materialized views
18591896

18601897
// These dependencies will force the index deletion statement to come before the table AddAlter
18611898
addAlterColumnDeps := []dependency{

0 commit comments

Comments
 (0)