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/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..012c7888 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -850,6 +850,49 @@ 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;") + // 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;") + + 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")) + // 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() { It(`gprestore continues when encountering errors during data load with --single-data-file and --on-error-continue`, func() { if segmentCount != 3 {