diff --git a/CHANGELOG.md b/CHANGELOG.md index aa60d4dc86217fe3b1fe294ffb1832bf3b6ac5f4..9cef0a45ca3e7f223137af78230e793f004b8605 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,13 @@ See [features](./features.md) for overall progression of PostgreSQL Migrator. - Inspect `V$INSTANCE`. Requires new privilege `SELECT ON V_$INSTANCE`. -*UI:* +**Engine:** + +- Dump foreign keys. +- Drop foreign key on partitionned table. Annotate unimplemented. +- Rename foreign keys. + +**UI:** - Fix drawer annotation badge. - Scope search to current catalog. diff --git a/docker-compose.yml b/docker-compose.yml index 2a6e9d3090e13170cb5e4c308712894852fc85fc..811b1a5da83c01e22b6fc900740ede6a0488d563 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,6 +56,9 @@ services: PGHOST: "postgres" HISTFILE: "/workspace/test/.bash_history" depends_on: + # Don't depends on mysql because mysql.bats includes a SHUTDOWN. + # If bats depends on mysql, docker kills bats once mysql is down. + # Start mysql manually before running bats. As in .circle/config.yml and test/bats.sh. - "${ORAHOST-oracle23}" - postgres entrypoint: /workspace/test/cli/entrypoint.sh diff --git a/docs/references/dump.md b/docs/references/dump.md index 68f7ccb741d3e9a62856538be5cd1c5f4ee856e0..aa6e6c48763a9c14970880e0badc85c774faf0a6 100644 --- a/docs/references/dump.md +++ b/docs/references/dump.md @@ -73,3 +73,18 @@ Chunk size varies seconding source DBMS, see table below. | longblob | 128kB | PostgreSQL Migrator inlines long columns for tables with average row size below 64KiB. + + +## Table constraints + +PostgreSQL Migrator serializes creation of table constraints: + +- primary key +- unique keys, one by one. +- indexes, one by one. +- foreign keys, one by one. + +The dump scheduler parallelize constraints of different tables, +but not constraints on the same table. +This prevents deadlocks. +Each constraint runs in its own transaction. diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go index 3ff534b23c8ad81c01de795f57a18a94d79d642e..8e80f1d7bae3eeecd7b35c4201df7db37bd143ef 100644 --- a/internal/catalog/catalog.go +++ b/internal/catalog/catalog.go @@ -125,51 +125,99 @@ func (ctg Catalog) AlterTablesKeys(p dispatch.TaskAdder) { continue } - if len(table.Keys) == 0 { - continue - } - var reqs []string if project.Current.DumpParams.Data { reqs = append(reqs, table.BuildPath("copy")) } else if project.Current.DumpParams.PreData { reqs = append(reqs, table.BuildPath("create")) + } else { + reqs = append(reqs, "init") } - p.Add(database.ExecuteTask( - table.BuildPath("keys"), table.Priority(), reqs, - table.AlterKeys(), - "Create table keys.", - "name", qualifiedName(table.Schema, table.Name), - )) + for _, key := range table.Keys { + if Exclude(key.ObjectPath) { + slog.Debug("Skipping ALTER of excluded key.", "path", key.ObjectPath) + continue + } + id := table.BuildPath(key.BuildPath("create")) + p.Add(database.ExecuteTask( + id, table.Priority(), reqs, + key.Create(table), + "Create table key.", + "name", qualifiedName(table.Schema, table.Name), + "key", key.Name, + )) + // chain tasks of same table. + reqs = []string{id} + } + + p.Checkpoint(table.BuildPath("keys"), reqs) } } func (ctg Catalog) CreateIndexes(p dispatch.TaskAdder) { for _, table := range ctg.Tables { - var reqs []string - - if len(table.Keys) > 0 { - reqs = append(reqs, table.BuildPath("keys")) - } else if project.Current.DumpParams.Data { - reqs = append(reqs, table.BuildPath("copy")) - } else if project.Current.DumpParams.PreData { - reqs = append(reqs, table.BuildPath("create")) + if Exclude(table.ObjectPath) { + continue } + var reqs []string + reqs = append(reqs, table.BuildPath("keys")) + for _, idx := range table.Indexes { if Exclude(idx.ObjectPath) { slog.Debug("Skipping CREATE of excluded index.", "path", idx.ObjectPath) continue } + id := table.BuildPath(idx.BuildPath("create")) p.Add(database.ExecuteTask( - table.BuildPath(idx.BuildPath("create")), table.Priority(), reqs, + id, table.Priority(), reqs, table.CreateIndex(idx), "Create Index.", "table", qualifiedName(table.Schema, table.Name), "name", idx.Name, )) + // Serialize index creations on same table + reqs = []string{id} + } + + // Even if no index, create an index milestone. + p.Checkpoint(table.BuildPath("indexes"), reqs) + } +} + +func (ctg Catalog) AlterTablesForeignKeys(p dispatch.TaskAdder) { + for _, table := range ctg.Tables { + if Exclude(table.ObjectPath) { + continue } + + var reqs []string + // First foreign key depends on table indexes. + reqs = append(reqs, table.BuildPath("indexes")) + + for _, fk := range table.ForeignKeys { + if Exclude(fk.ObjectPath) { + slog.Debug("Skipping ALTER of excluded foreign key", "path", fk.ObjectPath) + continue + } + id := table.BuildPath(fk.BuildPath("create")) + // Also depends on foreign table indexes. + reqs = append(reqs, fk.ForeignPath("indexes")) + p.Add(database.ExecuteTask( + id, table.Priority(), reqs, + fk.Create(table), + "Create foreign key.", + "name", table, + "fk", fk, + )) + + // Next foreign key depends only on previous fk. + reqs = []string{id} + } + + // Even if no foreign key, create a foreign key milestone. + p.Checkpoint(table.BuildPath("fk"), reqs) } } diff --git a/internal/catalog/constraints.go b/internal/catalog/constraints.go index 7956f7b923f86c09d5830f518bb789db1baf1c43..4f658e3ef1530bdeaf9386182ab5398849a1ff4c 100644 --- a/internal/catalog/constraints.go +++ b/internal/catalog/constraints.go @@ -34,8 +34,15 @@ func (k Key) String() string { return b.String() } -func (k Key) BuildPath() string { - return fmt.Sprintf("Keys/%s", k.Name) +func (k Key) BuildPath(tail ...string) string { + return strings.Join(append([]string{"Keys", k.Name}, tail...), "/") +} + +func (k Key) Create(t Table) string { + return Render("create-key.sql", map[string]any{ + "Key": k, + "Table": t, + }) } func RowToKey(r *sql.Rows) (a Association, err error) { @@ -113,8 +120,8 @@ func (fk ForeignKey) String() string { return b.String() } -func (fk ForeignKey) BuildPath() string { - return fmt.Sprintf("ForeignKeys/%s", fk.Name) +func (fk ForeignKey) BuildPath(tail ...string) string { + return strings.Join(append([]string{"ForeignKeys", fk.Name}, tail...), "/") } func (fk ForeignKey) ForeignPath(tail ...string) string { @@ -123,6 +130,13 @@ func (fk ForeignKey) ForeignPath(tail ...string) string { return strings.Join(elem, "/") } +func (fk ForeignKey) Create(t Table) string { + return Render("create-foreign-key.sql", map[string]any{ + "Key": fk, + "Table": t, + }) +} + func RowToForeignKey(r *sql.Rows) (a Association, err error) { var fk ForeignKey fk.ObjectType = "ForeignKey" diff --git a/internal/catalog/sql/alter-table-keys.sql b/internal/catalog/sql/alter-table-keys.sql deleted file mode 100644 index 561befd257e0e37a2a91be71b843e4858c74c92b..0000000000000000000000000000000000000000 --- a/internal/catalog/sql/alter-table-keys.sql +++ /dev/null @@ -1,14 +0,0 @@ -{{ range .Keys }} -{{- if not (exclude .ObjectPath) }} --- --- Name: {{ $.Name }} {{ .Name }}; Type: CONSTRAINT; Schema: {{ $.Schema }}; Owner: - --- - -ALTER TABLE {{ identifier $.Schema $.Name }} - ADD CONSTRAINT {{ identifier .Name }}{{ if .Primary }} PRIMARY KEY{{ else if .Unique }} UNIQUE{{ end }} ( -{{- range $i, $s := .Columns }} -{{- if gt $i 0 }}, {{ end }} -{{- identifier . }} -{{- end }}); -{{- end }} -{{- end }} diff --git a/internal/catalog/sql/create-foreign-key.sql b/internal/catalog/sql/create-foreign-key.sql new file mode 100644 index 0000000000000000000000000000000000000000..5997bd3e385b5f55f94e34b212dfd95938bcc828 --- /dev/null +++ b/internal/catalog/sql/create-foreign-key.sql @@ -0,0 +1,15 @@ + +-- +-- Name: {{ .Table.Name }} {{ .Key.Name }}; Type: FK CONSTRAINT; Schema: {{ .Table.Schema }}; Owner: - +-- + +ALTER TABLE ONLY {{ identifier .Table.Schema .Table.Name }} + ADD CONSTRAINT {{ identifier .Key.Name }} FOREIGN KEY ( +{{- range $i, $s := .Key.Columns }} +{{- if gt $i 0 }}, {{ end }} +{{- identifier . }} +{{- end }}) REFERENCES {{ identifier .Key.ForeignTableSchema .Key.ForeignTableName }} ( +{{- range $i, $s := .Key.ForeignColumns }} +{{- if gt $i 0 }}, {{ end }} +{{- identifier . }} +{{- end }}); diff --git a/internal/catalog/sql/create-key.sql b/internal/catalog/sql/create-key.sql new file mode 100644 index 0000000000000000000000000000000000000000..cc5ffa3f649181dda938e740736bdd6f08b1c2e2 --- /dev/null +++ b/internal/catalog/sql/create-key.sql @@ -0,0 +1,11 @@ + +-- +-- Name: {{ .Table.Name }} {{ .Key.Name }}; Type: CONSTRAINT; Schema: {{ .Table.Schema }}; Owner: - +-- + +ALTER TABLE ONLY {{ identifier .Table.Schema .Table.Name }} + ADD CONSTRAINT {{ identifier .Key.Name }}{{ if .Key.Primary }} PRIMARY KEY{{ else if .Key.Unique }} UNIQUE{{ end }} ( +{{- range $i, $s := .Key.Columns }} +{{- if gt $i 0 }}, {{ end }} +{{- identifier . }} +{{- end }}); diff --git a/internal/catalog/table.go b/internal/catalog/table.go index adc7d4ca9f26d663273c1b3598bf54295fbb047f..49a19993b7f0fc13d85ceba3216f26a0fc92ea0f 100644 --- a/internal/catalog/table.go +++ b/internal/catalog/table.go @@ -90,10 +90,6 @@ func (t Table) CreateIndex(idx Index) string { }) } -func (t Table) AlterKeys() string { - return Render("alter-table-keys.sql", t) -} - func (t Table) AlterDefaults() string { return Render("alter-table-default.sql", t) } diff --git a/internal/convert/table.go b/internal/convert/table.go index ddafd130302b749e0224eb5d638fa10eec5ea7ca..e162fc2517a8eca961f4dac75bc78af451804e22 100644 --- a/internal/convert/table.go +++ b/internal/convert/table.go @@ -101,17 +101,32 @@ func Tables(srcTables []catalog.Table) []catalog.Table { t.Keys[i] = k } - t.ForeignKeys = make([]catalog.ForeignKey, len(src.ForeignKeys)) - for i, srcFK := range src.ForeignKeys { + var fkeys []catalog.ForeignKey + t.ForeignKeys = nil + for _, srcFK := range src.ForeignKeys { + ft := catalog.Find[catalog.Table](ctg, srcFK.ForeignPath()) + if ft.PartitionBy != "" { + t.Annotate("unimplemented conversion: foreign key to partitionned table", 1) + continue + } fk := srcFK // copy - fk.Name = Identifier(srcFK.ObjectPath, srcFK.Name) + if fk.RegenerateName { + for j, name := range fk.Columns { + fk.Columns[j] = colMap[name].Name + } + fk.Name = fmt.Sprintf("%s_%s_fkey", t.Name, strings.Join(fk.Columns, "_")) + slog.Debug("Regenerating foreign key identifier.", "from", srcFK.Name, "to", fk.Name, "path", srcFK.ObjectPath) + } else { + fk.Name = Identifier(srcFK.ObjectPath, srcFK.Name) + } fk.ObjectPath = t.BuildPath(fk.BuildPath()) SourceMap[fk.ObjectPath] = srcFK.ObjectPath // Apply local and foreign columns renaming. - ft := catalog.Find[catalog.Table](ctg, srcFK.ForeignPath()) for j, name := range srcFK.Columns { - fk.Columns[j] = colMap[name].Name + if !fk.RegenerateName { + fk.Columns[j] = colMap[name].Name + } fc := ft.Column(srcFK.ForeignColumns[j]) // Rerun foreign column renaming. @@ -121,12 +136,9 @@ func Tables(srcTables []catalog.Table) []catalog.Table { fk.ForeignTableName = Identifier(ft.ObjectPath, fk.ForeignTableName) fk.ForeignTableSchema = Identifier(fk.ForeignTableSchema, fk.ForeignTableSchema) fk.ObjectDefinition = fk.String() - t.ForeignKeys[i] = fk - } - - if len(src.ForeignKeys) > 0 { - t.Annotate("not implemented dump: foreign keys", 1) + fkeys = append(fkeys, fk) } + t.ForeignKeys = fkeys t.Indexes = nil indexes: diff --git a/internal/dispatch/mock.go b/internal/dispatch/mock.go index 1b3fa41dc513455eb706537f0a55975259e66456..3987b75a1fad17b2ee67163165b93a4c53bce7e1 100644 --- a/internal/dispatch/mock.go +++ b/internal/dispatch/mock.go @@ -2,6 +2,7 @@ package dispatch type TaskAdder interface { Add(Task) + Checkpoint(string, []string) } var _ TaskAdder = New() @@ -14,3 +15,6 @@ func (p Mock) Add(t Task) { id, _, _ := t.Describe() p[id] = t } + +func (Mock) Checkpoint(_ string, _ []string) { +} diff --git a/internal/dispatch/plan.go b/internal/dispatch/plan.go index 6df2d6db75222f5283abafb265e5e4dfe9a06b9b..bf9634f21f2ae3305343ca9cbc4458d36325ba30 100644 --- a/internal/dispatch/plan.go +++ b/internal/dispatch/plan.go @@ -75,6 +75,10 @@ func (p *Plan) Add(t Task) { } p.tasks[id] = t + // Remove reqs from this task to use this task instead. + p.pointedTasks = slices.DeleteFunc(p.pointedTasks, func(id string) bool { + return slices.Contains(reqs, id) + }) p.pointedTasks = append(p.pointedTasks, id) if len(reqs) == 0 { @@ -94,23 +98,25 @@ func (p *Plan) Add(t Task) { } } -// Checkpoint add a checkpoint to the plan +// Checkpoint add a waiter to the plan +// +// nil reqs creates a checkpoint on all previous tasks. // -// Checkpoint add a no-op task that require all the -// previously added tasks. -func (p *Plan) Checkpoint(id string) { - checkpoint := noOpTask{ +// Use this to wait for a section of plan. e.g. pre-data, data, etc. +func (p *Plan) Checkpoint(id string, reqs []string) { + if len(reqs) == 0 { + reqs = p.pointedTasks + p.pointedTasks = nil + } + checkpoint := &noOpTask{ Header: Header{ - Id: id, - Priority: 0, - Reqs: p.pointedTasks, + Id: id, + Reqs: reqs, }, } - // pointedTasks is emptied - p.pointedTasks = nil p.Add(checkpoint) - // checkpoint Has been added so it is the only task in pointedTasks - // for further checkpoint + // Adds checkpoint as a regular task to ends in pointedTasks + // for next checkpoint. } // Execute runs tasks concurrently diff --git a/internal/dispatch/plan_test.go b/internal/dispatch/plan_test.go index f96f726244bd8155c7c7b5a2068a8026262b94da..f6d55397117a1649f10a2a796ee62376e1a50a1d 100644 --- a/internal/dispatch/plan_test.go +++ b/internal/dispatch/plan_test.go @@ -210,7 +210,7 @@ func TestCheckpoint(t *testing.T) { }, fn: run, }) - p.Checkpoint("CP") + p.Checkpoint("CP", nil) p.Add(task{ Header: dispatch.Header{ Id: "e", @@ -230,7 +230,7 @@ func TestCheckpoint(t *testing.T) { }, fn: run, }) - p.Checkpoint("CP2") + p.Checkpoint("CP2", nil) p.Add(task{ Header: dispatch.Header{ Id: "g", diff --git a/internal/mysql/dump.go b/internal/mysql/dump.go index aae98f3e7d5869accea56fcd8bfcbe19a8056ca5..f7cd912eaf0352b1b1be858bffab8b8ded730f1c 100644 --- a/internal/mysql/dump.go +++ b/internal/mysql/dump.go @@ -16,7 +16,7 @@ import ( func (ctg *myctg) Dump(source any) *dispatch.Plan { p := dispatch.New() - p.Checkpoint("init") + p.Checkpoint("init", nil) ctg.Tables = slices.DeleteFunc(ctg.Tables, func(t catalog.Table) bool { src := catalog.Find[catalog.Table](source, convert.SourceMap[t.ObjectPath]) @@ -29,12 +29,12 @@ func (ctg *myctg) Dump(source any) *dispatch.Plan { if project.Current.DumpParams.PreData { ctg.predata(p) - p.Checkpoint("predata") + p.Checkpoint("predata", nil) } if project.Current.DumpParams.Data { ctg.data(p, source) - p.Checkpoint("data") + p.Checkpoint("data", nil) } if project.Current.DumpParams.PostData { @@ -67,8 +67,8 @@ func (ctg myctg) data(p *dispatch.Plan, source any) { func (ctg myctg) postdata(p *dispatch.Plan) { dispatch.Number = dispatch.PostData ctg.AlterTablesKeys(p) - // Indexes depend on table keys - ctg.CreateIndexes(p) + ctg.CreateIndexes(p) // Indexes depend on table keys + ctg.AlterTablesForeignKeys(p) // FK depends on indexes ctg.AlterTablesDefaults(p) } diff --git a/internal/oracle/convert.go b/internal/oracle/convert.go index db4d85ef5b827351a479756918bd9d7342ece55b..cf43a4965a16e7566b4f9c725fd6c6467ee6e4ba 100644 --- a/internal/oracle/convert.go +++ b/internal/oracle/convert.go @@ -27,6 +27,7 @@ func (ctg oractg) Convert() any { continue } tgt.Annotate("not implemented conversion: temporary table", 1) + break } tgt.Sequences = convert.Sequences(ctg.Sequences) diff --git a/internal/oracle/dump.go b/internal/oracle/dump.go index cc1b8f6a4f0cd2cfd3c6e87e00fc2deaea98f273..d462b3a6bd2912e2b67ac9b14e8da101609b516d 100644 --- a/internal/oracle/dump.go +++ b/internal/oracle/dump.go @@ -26,14 +26,14 @@ func (ctg *oractg) Dump(source any) *dispatch.Plan { ) p.Add(t) } - p.Checkpoint("init") + p.Checkpoint("init", nil) if project.Current.DumpParams.PreData { ctg.predata(p) - p.Checkpoint("predata") + p.Checkpoint("predata", nil) } if project.Current.DumpParams.Data { ctg.data(p, source) - p.Checkpoint("data") + p.Checkpoint("data", nil) if srcMdl.Before("12.1") { t := database.ExecuteSourceTask( "drop-package-support", []string{"data"}, @@ -81,7 +81,7 @@ func (ctg oractg) data(p *dispatch.Plan, source any) { func (ctg oractg) postdata(p *dispatch.Plan) { dispatch.Number = dispatch.PostData ctg.AlterTablesKeys(p) - // Indexes depend on table keys - ctg.CreateIndexes(p) + ctg.CreateIndexes(p) // Indexes depend on table keys + ctg.AlterTablesForeignKeys(p) // FK depends on indexes ctg.AlterTablesDefaults(p) } diff --git a/test/bats.sh b/test/bats.sh index 40acd84b3d649387234e049b3b9e5565953e142f..5e184b1ca6b41d91969f61d78ec0071e457a7e72 100755 --- a/test/bats.sh +++ b/test/bats.sh @@ -7,4 +7,5 @@ rm -rf test/results/ # Configure compose and bats to use dev databases. export ORAHOST=oracle18 # CI=true keeps results files. +docker compose up -d mysql exec docker compose run --rm -e CI=true bats "$@" diff --git a/test/cli/mysql.bats b/test/cli/mysql.bats index d3fa1a92b0f26ec2de550ed2e876827f2afa6cd2..8793063c758cd343ad431a75d13bc70f8bbeeaa5 100644 --- a/test/cli/mysql.bats +++ b/test/cli/mysql.bats @@ -200,4 +200,11 @@ teardown_file() { WHERE conname = 'actor_pkey'; EOSQL assert_output "t" + + run psql -tA <<-EOSQL + SELECT confrelid IS NOT NULL + FROM pg_catalog.pg_constraint + WHERE conname = 'address_city_id_fkey'; + EOSQL + assert_output "t" } diff --git a/test/cli/oracle.bats b/test/cli/oracle.bats index e65b67613d8768df30d8c6099f9902e41ba903fe..83a2df28fd98fc35dc12211c2f4b6988fa11c891 100644 --- a/test/cli/oracle.bats +++ b/test/cli/oracle.bats @@ -209,4 +209,11 @@ setup_file() { WHERE conname = 'pk_actor'; EOSQL assert_output "t" + + run psql -tA <<-EOSQL + SELECT confrelid IS NOT NULL + FROM pg_catalog.pg_constraint + WHERE conname = 'fk_address_city'; + EOSQL + assert_output "t" }