From 6cd5102810cfec354085d55881bddd42c95cc4a4 Mon Sep 17 00:00:00 2001 From: talmacschen-arch Date: Fri, 30 Oct 2026 02:55:05 +0800 Subject: [PATCH 1/2] Back up foreign data wrapper objects on Cloudberry retrieveFDWObjects gates foreign data wrapper, foreign server and user mapping backup on "GPDB 6+ or Cloudberry". A misplaced negation -- !(IsGPDB && AtLeast6) || IsCBDB() instead of !((IsGPDB && AtLeast6) || IsCBDB()) -- makes the guard return early on Cloudberry, so none of these objects are backed up there. Foreign tables are backed up through a separate path, so a Cloudberry backup produces CREATE FOREIGN TABLE statements referencing servers it never dumped, and the restore fails with: ERROR: server "..." does not exist Fix the negation so Cloudberry backs up foreign data wrappers, servers and user mappings, matching the "GPDB 6+ or Cloudberry" pattern used in every other hunk of the commit that introduced this guard. Add an end-to-end test that backs up and restores a foreign data wrapper, server, user mapping and foreign table and asserts they exist after restore. --- backup/wrappers.go | 9 +++++++- end_to_end/end_to_end_suite_test.go | 35 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/backup/wrappers.go b/backup/wrappers.go index 33704b0e..7caf8424 100644 --- a/backup/wrappers.go +++ b/backup/wrappers.go @@ -418,7 +418,14 @@ func retrieveCasts(sortables *[]Sortable, metadataMap MetadataMap) { } func retrieveFDWObjects(sortables *[]Sortable, metadataMap MetadataMap) { - if !(connectionPool.Version.IsGPDB() && connectionPool.Version.AtLeast("6")) || connectionPool.Version.IsCBDB() { + // Foreign data wrappers, servers and user mappings exist on GPDB 6+ and on + // Cloudberry. The condition must skip only when the server is neither, i.e. + // negate the whole "GPDB 6+ OR Cloudberry" predicate. A misplaced negation + // here previously skipped these objects on Cloudberry, so their CREATE + // SERVER / FOREIGN DATA WRAPPER / USER MAPPING statements were omitted from + // the backup and any foreign table that depended on them failed to restore + // with "server ... does not exist". + if !((connectionPool.Version.IsGPDB() && connectionPool.Version.AtLeast("6")) || connectionPool.Version.IsCBDB()) { return } retrieveForeignDataWrappers(sortables, metadataMap) diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index 468ee686..becee1a7 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -850,6 +850,41 @@ var _ = Describe("backup and restore end to end tests", func() { Expect(tocStruct.GlobalEntries[0].ObjectType).To(Equal(toc.OBJ_SESSION_GUC)) }) }) + Describe("foreign data wrapper objects tests", func() { + It("backs up and restores foreign data wrappers, servers, user mappings and foreign tables", func() { + // Regression test for a bug where retrieveFDWObjects skipped FDW + // objects on Cloudberry: the foreign table was backed up but its + // server was not, so the restore failed with "server does not exist". + if !((backupConn.Version.IsGPDB() && backupConn.Version.AtLeast("6")) || backupConn.Version.IsCBDB()) { + Skip("Foreign data wrappers require GPDB 6+ or Cloudberry") + } + testhelper.AssertQueryRuns(backupConn, "CREATE FOREIGN DATA WRAPPER fdw;") + defer testhelper.AssertQueryRuns(backupConn, "DROP FOREIGN DATA WRAPPER fdw CASCADE;") + testhelper.AssertQueryRuns(backupConn, "CREATE SERVER fdw_server FOREIGN DATA WRAPPER fdw;") + testhelper.AssertQueryRuns(backupConn, "CREATE USER MAPPING FOR CURRENT_USER SERVER fdw_server;") + testhelper.AssertQueryRuns(backupConn, "CREATE FOREIGN TABLE public.fdw_table (i int) SERVER fdw_server;") + + output := gpbackup(gpbackupPath, backupHelperPath, "--backup-dir", backupDir, "--metadata-only") + timestamp := getBackupTimestamp(string(output)) + + gprestore(gprestorePath, restoreHelperPath, timestamp, + "--backup-dir", backupDir, + "--redirect-db", "restoredb", + "--metadata-only") + defer testhelper.AssertQueryRuns(restoreConn, "DROP FOREIGN DATA WRAPPER fdw CASCADE;") + + // Without the fix the server (and its dependents) would be missing. + serverCount := dbconn.MustSelectString(restoreConn, + "SELECT count(*) FROM pg_foreign_server WHERE srvname = 'fdw_server'") + Expect(serverCount).To(Equal("1")) + mappingCount := dbconn.MustSelectString(restoreConn, + "SELECT count(*) FROM pg_user_mappings WHERE srvname = 'fdw_server'") + Expect(mappingCount).To(Equal("1")) + foreignTableCount := dbconn.MustSelectString(restoreConn, + "SELECT count(*) FROM pg_class WHERE relname = 'fdw_table' AND relkind = 'f'") + Expect(foreignTableCount).To(Equal("1")) + }) + }) Describe(`On Error Continue`, func() { It(`gprestore continues when encountering errors during data load with --single-data-file and --on-error-continue`, func() { if segmentCount != 3 { From b3b4c6a649c21869799d7d50496a44018198126c Mon Sep 17 00:00:00 2001 From: talmacschen-arch Date: Sat, 31 Oct 2026 23:34:10 +0800 Subject: [PATCH 2/2] Emit COMMENT ON SERVER, not COMMENT ON FOREIGN SERVER Now that foreign servers are backed up on Cloudberry, a server carrying a comment is emitted by GetCommentStatement as COMMENT ON FOREIGN SERVER IS '...'; which is a syntax error on restore (ERROR: syntax error at or near "SERVER"): the correct form is COMMENT ON SERVER. GetOwnerStatement already maps OBJ_FOREIGN_SERVER to OBJ_SERVER for ALTER ... OWNER; apply the same mapping in GetCommentStatement. The bug predates this series (it affects GPDB 6 too) but was unreachable on Cloudberry until foreign servers started being backed up. Verified end to end on a live Cloudberry 2.1.0 cluster: a foreign server with a comment now backs up as COMMENT ON SERVER and restores cleanly, with the comment preserved. Extend the FDW end-to-end test with a COMMENT ON SERVER and assert it survives restore. --- backup/predata_acl.go | 8 +++++++- end_to_end/end_to_end_suite_test.go | 8 ++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/backup/predata_acl.go b/backup/predata_acl.go index b6d7076e..81353179 100644 --- a/backup/predata_acl.go +++ b/backup/predata_acl.go @@ -488,6 +488,12 @@ func (obj ObjectMetadata) GetOwnerStatement(objectName string, objectType string } func (obj ObjectMetadata) GetCommentStatement(objectName string, objectType string, owningTable string) string { + typeStr := objectType + // COMMENT uses "SERVER", not "FOREIGN SERVER" (mirrors GetOwnerStatement). + // Emitting "COMMENT ON FOREIGN SERVER ..." is a syntax error on restore. + if objectType == toc.OBJ_FOREIGN_SERVER { + typeStr = toc.OBJ_SERVER + } commentStr := "" tableStr := "" if owningTable != "" { @@ -495,7 +501,7 @@ func (obj ObjectMetadata) GetCommentStatement(objectName string, objectType stri } if obj.Comment != "" { escapedComment := utils.EscapeSingleQuotes(obj.Comment) - commentStr = fmt.Sprintf("COMMENT ON %s %s%s IS '%s';", objectType, objectName, tableStr, escapedComment) + commentStr = fmt.Sprintf("COMMENT ON %s %s%s IS '%s';", typeStr, objectName, tableStr, escapedComment) } return commentStr } diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index becee1a7..012c7888 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -861,6 +861,9 @@ var _ = Describe("backup and restore end to end tests", func() { testhelper.AssertQueryRuns(backupConn, "CREATE FOREIGN DATA WRAPPER fdw;") defer testhelper.AssertQueryRuns(backupConn, "DROP FOREIGN DATA WRAPPER fdw CASCADE;") testhelper.AssertQueryRuns(backupConn, "CREATE SERVER fdw_server FOREIGN DATA WRAPPER fdw;") + // A server comment must be emitted as "COMMENT ON SERVER", not + // "COMMENT ON FOREIGN SERVER", or the restore aborts with a syntax error. + testhelper.AssertQueryRuns(backupConn, "COMMENT ON SERVER fdw_server IS 'fdw server comment';") testhelper.AssertQueryRuns(backupConn, "CREATE USER MAPPING FOR CURRENT_USER SERVER fdw_server;") testhelper.AssertQueryRuns(backupConn, "CREATE FOREIGN TABLE public.fdw_table (i int) SERVER fdw_server;") @@ -883,6 +886,11 @@ var _ = Describe("backup and restore end to end tests", func() { foreignTableCount := dbconn.MustSelectString(restoreConn, "SELECT count(*) FROM pg_class WHERE relname = 'fdw_table' AND relkind = 'f'") Expect(foreignTableCount).To(Equal("1")) + // The server comment must survive restore; a "COMMENT ON FOREIGN + // SERVER" statement would have aborted the restore instead. + serverComment := dbconn.MustSelectString(restoreConn, + "SELECT coalesce(obj_description(oid, 'pg_foreign_server'), '') FROM pg_foreign_server WHERE srvname = 'fdw_server'") + Expect(serverComment).To(Equal("fdw server comment")) }) }) Describe(`On Error Continue`, func() {