From 7b707e54c7a80372ef07c7b405f579160c339372 Mon Sep 17 00:00:00 2001 From: talmacschen-arch Date: Thu, 29 Oct 2026 21:27:50 +0800 Subject: [PATCH 1/2] Deduplicate external leaf partitions in the metadata table list Under --leaf-partition-data, SplitTablesByPartitionType appends an external (foreign-table) leaf partition to metadataTables twice on Cloudberry / GPDB 7+: - the external-partition branch appends it (this branch exists only to add the _ext_part_ suffix for GPDB 6 and earlier), and - the common GPDB7+/CBDB branch, which emits every table, appends it again. Both copies carry the same UniqueID, so TopologicalSort later produces a sorted slice shorter than its input and aborts predata with: [CRITICAL]:-Dependency resolution failed; ... This is a bug, please report. (stack: TopologicalSort <- backupDependentObjects <- backupPredata). The "Not yet visited" list in the verbose log is empty, which rules out a real dependency cycle and points at a dropped, de-duplicated object. It reproduces on any database that has an external leaf partition when backing up with --leaf-partition-data, with or without --single-data-file. Restrict the external-partition branch to GPDB 6 and earlier, where the suffix is needed and the common branch does not run. On GPDB7+/CBDB the external leaf partition is now emitted once by the common branch, exactly like a native leaf partition. Add a unit test asserting that an external leaf partition appears in the metadata list exactly once under --leaf-partition-data on GPDB7+/CBDB; it fails (length 3) before this change and passes after. --- backup/predata_relations.go | 15 ++++++++----- backup/predata_relations_other_test.go | 31 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/backup/predata_relations.go b/backup/predata_relations.go index 4de2b542..aa54937d 100644 --- a/backup/predata_relations.go +++ b/backup/predata_relations.go @@ -41,12 +41,15 @@ func SplitTablesByPartitionType(tables []Table, includeList []options.Relation) } for _, table := range tables { - if table.IsExternal && table.PartitionLevelInfo.Level == "l" { - if connectionPool.Version.IsGPDB() && connectionPool.Version.Before("7") { - // GPDB7+ has different conventions for external partitions - // and does not need the suffix added - table.Name = AppendExtPartSuffix(table.Name) - } + if table.IsExternal && table.PartitionLevelInfo.Level == "l" && + connectionPool.Version.IsGPDB() && connectionPool.Version.Before("7") { + // GPDB6 and earlier: external leaf partitions need the + // _ext_part_ suffix and are emitted as their own metadata table + // here. GPDB7+/CBDB have different conventions for external + // partitions and emit every table (including external leaf + // partitions) in the block below, so they must not also be + // appended here or they would be duplicated. + table.Name = AppendExtPartSuffix(table.Name) metadataTables = append(metadataTables, table) } partType := table.PartitionLevelInfo.Level diff --git a/backup/predata_relations_other_test.go b/backup/predata_relations_other_test.go index e52a7b91..3cf38945 100644 --- a/backup/predata_relations_other_test.go +++ b/backup/predata_relations_other_test.go @@ -592,6 +592,37 @@ GRANT ALL ON TABLE shamwow.shazam TO testrole;`} structmatcher.ExpectStructsToMatch(&expectedTables[1], &metadataTables[1]) }) }) + Context("leafPartitionData with an external leaf partition", func() { + It("adds an external leaf partition to the metadata list exactly once on GPDB7+/CBDB", func() { + // On GPDB7+/CBDB every table (including external leaf + // partitions) is emitted once by the common branch. The + // external leaf partition must not also be appended by the + // GPDB6-only external-partition branch, or it would appear + // twice and later break dependency sorting with a spurious + // "Dependency resolution failed" error. + testhelper.SetDBVersion(connectionPool, "7.0.0") + includeList = []options.Relation{} + tables = []backup.Table{ + { + Relation: backup.Relation{Oid: 1, Schema: "public", Name: "part_parent1_prt_ext"}, + TableDefinition: backup.TableDefinition{PartitionLevelInfo: backup.PartitionLevelInfo{Level: "l"}, IsExternal: true}, + }, + { + Relation: backup.Relation{Oid: 2, Schema: "public", Name: "part_parent1_prt_normal"}, + TableDefinition: backup.TableDefinition{PartitionLevelInfo: backup.PartitionLevelInfo{Level: "l"}}, + }, + } + _ = cmdFlags.Set(options.LEAF_PARTITION_DATA, "true") + _ = cmdFlags.Set(options.INCLUDE_RELATION, "") + + metadataTables, _ := backup.SplitTablesByPartitionType(tables, includeList) + + Expect(metadataTables).To(HaveLen(2)) + names := []string{metadataTables[0].FQN(), metadataTables[1].FQN()} + sort.Strings(names) + Expect(names).To(Equal([]string{"public.part_parent1_prt_ext", "public.part_parent1_prt_normal"})) + }) + }) }) Describe("AppendExtPartSuffix", func() { It("adds a suffix to an unquoted external partition table", func() { From 32d134f7c00e70c587cfa6a52f5d1d6ba033c51c Mon Sep 17 00:00:00 2001 From: talmacschen-arch Date: Sat, 31 Oct 2026 23:38:54 +0800 Subject: [PATCH 2/2] Broaden external-leaf-partition dedup test coverage Address review feedback on the unit test for the external leaf partition deduplication fix: - Add a Cloudberry (IsCBDB) variant. The bug was reported on Cloudberry, which takes the same GPDB7+ code path, but the original test only set a GPDB 7.0.0 version and never exercised the IsCBDB arm. - Add a variant with a non-empty includeList and --leaf-partition-data off. The duplication happens in the LEAF_PARTITION_DATA || len(includeList) > 0 branch, so --include-table alone (without --leaf-partition-data) triggered it too; this guards that path. All three variants (GPDB7+ with --leaf-partition-data, Cloudberry with --leaf-partition-data, GPDB7+ with a non-empty includeList) fail with length 3 before the fix and pass after it. --- backup/predata_relations_other_test.go | 61 ++++++++++++++++++++------ 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/backup/predata_relations_other_test.go b/backup/predata_relations_other_test.go index 3cf38945..71f53f1e 100644 --- a/backup/predata_relations_other_test.go +++ b/backup/predata_relations_other_test.go @@ -10,6 +10,7 @@ import ( "github.com/apache/cloudberry-backup/options" "github.com/apache/cloudberry-backup/testutils" "github.com/apache/cloudberry-backup/toc" + "github.com/apache/cloudberry-go-libs/dbconn" "github.com/apache/cloudberry-go-libs/structmatcher" "github.com/apache/cloudberry-go-libs/testhelper" @@ -593,16 +594,16 @@ GRANT ALL ON TABLE shamwow.shazam TO testrole;`} }) }) Context("leafPartitionData with an external leaf partition", func() { - It("adds an external leaf partition to the metadata list exactly once on GPDB7+/CBDB", func() { - // On GPDB7+/CBDB every table (including external leaf - // partitions) is emitted once by the common branch. The - // external leaf partition must not also be appended by the - // GPDB6-only external-partition branch, or it would appear - // twice and later break dependency sorting with a spurious - // "Dependency resolution failed" error. - testhelper.SetDBVersion(connectionPool, "7.0.0") - includeList = []options.Relation{} - tables = []backup.Table{ + // On GPDB7+/CBDB every table (including external leaf partitions) is + // emitted once by the common branch. The external leaf partition must + // not also be appended by the GPDB6-only external-partition branch, or + // it would appear twice and later break dependency sorting with a + // spurious "Dependency resolution failed" error. This affects any + // backup that enters the LEAF_PARTITION_DATA || len(includeList) > 0 + // branch, i.e. --leaf-partition-data OR --include-table. + var extLeafTables []backup.Table + BeforeEach(func() { + extLeafTables = []backup.Table{ { Relation: backup.Relation{Oid: 1, Schema: "public", Name: "part_parent1_prt_ext"}, TableDefinition: backup.TableDefinition{PartitionLevelInfo: backup.PartitionLevelInfo{Level: "l"}, IsExternal: true}, @@ -612,15 +613,47 @@ GRANT ALL ON TABLE shamwow.shazam TO testrole;`} TableDefinition: backup.TableDefinition{PartitionLevelInfo: backup.PartitionLevelInfo{Level: "l"}}, }, } - _ = cmdFlags.Set(options.LEAF_PARTITION_DATA, "true") - _ = cmdFlags.Set(options.INCLUDE_RELATION, "") - - metadataTables, _ := backup.SplitTablesByPartitionType(tables, includeList) + }) + // assertEmittedOnce runs SplitTablesByPartitionType and asserts the + // external leaf partition appears exactly once (length 2, not 3). + // Before the fix this fails with length 3 (the external leaf twice). + assertEmittedOnce := func(includeList []options.Relation) { + metadataTables, _ := backup.SplitTablesByPartitionType(extLeafTables, includeList) Expect(metadataTables).To(HaveLen(2)) names := []string{metadataTables[0].FQN(), metadataTables[1].FQN()} sort.Strings(names) Expect(names).To(Equal([]string{"public.part_parent1_prt_ext", "public.part_parent1_prt_normal"})) + } + It("adds an external leaf partition to the metadata list exactly once on GPDB7+ with --leaf-partition-data", func() { + testhelper.SetDBVersion(connectionPool, "7.0.0") + _ = cmdFlags.Set(options.LEAF_PARTITION_DATA, "true") + _ = cmdFlags.Set(options.INCLUDE_RELATION, "") + + assertEmittedOnce([]options.Relation{}) + }) + It("adds an external leaf partition to the metadata list exactly once on Cloudberry with --leaf-partition-data", func() { + // The bug was reported on Cloudberry (IsCBDB), which takes the + // same GPDB7+ code path; cover it explicitly, not just GPDB 7.0.0. + cbdbVersion := dbconn.NewVersion("2.0.0") + cbdbVersion.Type = dbconn.CBDB + connectionPool.Version = cbdbVersion + _ = cmdFlags.Set(options.LEAF_PARTITION_DATA, "true") + _ = cmdFlags.Set(options.INCLUDE_RELATION, "") + + assertEmittedOnce([]options.Relation{}) + }) + It("adds an external leaf partition to the metadata list exactly once on GPDB7+ with a non-empty includeList and --leaf-partition-data off", func() { + // The same branch is entered via len(includeList) > 0, so the bug + // is not specific to --leaf-partition-data: --include-table alone + // triggered it too. + testhelper.SetDBVersion(connectionPool, "7.0.0") + _ = cmdFlags.Set(options.LEAF_PARTITION_DATA, "false") + _ = cmdFlags.Set(options.INCLUDE_RELATION, "public.part_parent1_prt_ext") + + assertEmittedOnce([]options.Relation{ + {Oid: 1, Schema: "public", Name: "part_parent1_prt_ext"}, + }) }) }) })