On Fri, Oct 13, 2023 at 11:02:39AM +0200, Daniel Gustafsson wrote:
> That's a more compelling reason IMO. I'm not sure if I prefer the
> GUC_check_errdetail-like approach better, I would for sure not be opposed to
> reviewing a version of the patch doing it that way.
>
> Tung Nguyen: are you interested in updating the patch along these lines
> suggested by Nathan?
I gave it a try.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 7183005bb6786b63a5fd96ba5101849eb6f203e5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Fri, 5 Jan 2024 17:01:10 -0600
Subject: [PATCH v4 1/1] add support for emitting errdetail from archive module
check-configured callback
---
contrib/basic_archive/basic_archive.c | 8 +++++++-
src/backend/postmaster/pgarch.c | 8 +++++++-
src/include/archive/archive_module.h | 8 ++++++++
3 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/contrib/basic_archive/basic_archive.c b/contrib/basic_archive/basic_archive.c
index 804567e919..2c8721ebf6 100644
--- a/contrib/basic_archive/basic_archive.c
+++ b/contrib/basic_archive/basic_archive.c
@@ -161,7 +161,13 @@ check_archive_directory(char **newval, void **extra, GucSource source)
static bool
basic_archive_configured(ArchiveModuleState *state)
{
- return archive_directory != NULL && archive_directory[0] != '\0';
+ if (archive_directory == NULL || archive_directory[0] == '\0')
+ {
+ arch_module_check_errdetail("basic_archive.archive_directory is not set.");
+ return false;
+ }
+
+ return true;
}
/*
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 67693b0580..f663965d89 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -91,6 +91,7 @@ typedef struct PgArchData
} PgArchData;
char *XLogArchiveLibrary = "";
+char *arch_module_check_errdetail_string;
/* ----------
@@ -408,12 +409,17 @@ pgarch_ArchiverCopyLoop(void)
*/
HandlePgArchInterrupts();
+ /* Reset variables that might be set by the callback */
+ arch_module_check_errdetail_string = NULL;
+
/* can't do anything if not configured ... */
if (ArchiveCallbacks->check_configured_cb != NULL &&
!ArchiveCallbacks->check_configured_cb(archive_module_state))
{
ereport(WARNING,
- (errmsg("archive_mode enabled, yet archiving is not configured")));
+ (errmsg("archive_mode enabled, yet archiving is not configured"),
+ arch_module_check_errdetail_string ?
+ errdetail_internal("%s", arch_module_check_errdetail_string) : 0));
return;
}
diff --git a/src/include/archive/archive_module.h b/src/include/archive/archive_module.h
index fd59b9faf4..763af76e54 100644
--- a/src/include/archive/archive_module.h
+++ b/src/include/archive/archive_module.h
@@ -56,4 +56,12 @@ typedef const ArchiveModuleCallbacks *(*ArchiveModuleInit) (void);
extern PGDLLEXPORT const ArchiveModuleCallbacks *_PG_archive_module_init(void);
+/* Support for messages reported from archive module callbacks. */
+
+extern PGDLLIMPORT char *arch_module_check_errdetail_string;
+
+#define arch_module_check_errdetail \
+ pre_format_elog_string(errno, TEXTDOMAIN), \
+ arch_module_check_errdetail_string = format_elog_string
+
#endif /* _ARCHIVE_MODULE_H */
--
2.25.1
