Skip to content

Commit 86ad24a

Browse files
songwdfusongwdfu
authored
add support for ereport_domain macro (pgcentralfoundation#2256)
**What is the problem** Postgres provides the TEXTDOMAIN macro and ereport_domain macro to allow a domain to be specified for log messages. It could be read from edata->domain. This might be a feature that people building extensions want to use (or misuse). Currently pgrx hardcodes the domain as NULL. It would be great if it could provide some way for people to specify it. Issue [here](pgcentralfoundation#2255) **How does this PR solves this** This is a small PR that modifies the `ErrorReport` struct to add the `domain` field, and defines the ereport_domain macros.
1 parent 87db532 commit 86ad24a

6 files changed

Lines changed: 213 additions & 8 deletions

File tree

‎pgrx-pg-sys/src/submodules/elog.rs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,127 @@ pub fn interrupt_pending() -> bool {
424424
unsafe { crate::InterruptPending != 0 }
425425
}
426426

427+
/// Send some kind of message to Postgres similar to the ereport macro, while specifying
428+
/// a text domain, analogous to Postgres' `ereport_domain` C macro.
429+
///
430+
/// The argument order is:
431+
/// - `log_level: [PgLogLevel]`
432+
/// - `error_code: [PgSqlErrorCode]`
433+
/// - `message: String`
434+
/// - (optional) `detail: String`
435+
///
436+
/// ## Examples
437+
///
438+
/// ```rust,no_run
439+
/// # use pgrx_pg_sys::ereport_domain;
440+
/// # use pgrx_pg_sys::elog::PgLogLevel;
441+
/// # use pgrx_pg_sys::errcodes::PgSqlErrorCode;
442+
/// ereport_domain!(PgLogLevel::ERROR, "my_extension", PgSqlErrorCode::ERRCODE_INTERNAL_ERROR, "oh noes!");
443+
/// ```
444+
///
445+
/// ```rust,no_run
446+
/// # use pgrx_pg_sys::ereport_domain;
447+
/// # use pgrx_pg_sys::elog::PgLogLevel;
448+
/// # use pgrx_pg_sys::errcodes::PgSqlErrorCode;
449+
/// ereport_domain!(PgLogLevel::LOG, "my_extension", PgSqlErrorCode::ERRCODE_SUCCESSFUL_COMPLETION, "translated message");
450+
/// ```
451+
#[macro_export]
452+
macro_rules! ereport_domain {
453+
(ERROR, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
454+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
455+
.set_domain($domain)
456+
$(.set_detail($detail))?
457+
.report($crate::elog::PgLogLevel::ERROR);
458+
unreachable!();
459+
};
460+
461+
(PANIC, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
462+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
463+
.set_domain($domain)
464+
$(.set_detail($detail))?
465+
.report($crate::elog::PgLogLevel::PANIC);
466+
unreachable!();
467+
};
468+
469+
(FATAL, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
470+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
471+
.set_domain($domain)
472+
$(.set_detail($detail))?
473+
.report($crate::elog::PgLogLevel::FATAL);
474+
unreachable!();
475+
};
476+
477+
(WARNING, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
478+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
479+
.set_domain($domain)
480+
$(.set_detail($detail))?
481+
.report($crate::elog::PgLogLevel::WARNING)
482+
};
483+
484+
(NOTICE, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
485+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
486+
.set_domain($domain)
487+
$(.set_detail($detail))?
488+
.report($crate::elog::PgLogLevel::NOTICE)
489+
};
490+
491+
(INFO, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
492+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
493+
.set_domain($domain)
494+
$(.set_detail($detail))?
495+
.report($crate::elog::PgLogLevel::INFO)
496+
};
497+
498+
(LOG, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
499+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
500+
.set_domain($domain)
501+
$(.set_detail($detail))?
502+
.report($crate::elog::PgLogLevel::LOG)
503+
};
504+
505+
(DEBUG5, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
506+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
507+
.set_domain($domain)
508+
$(.set_detail($detail))?
509+
.report($crate::elog::PgLogLevel::DEBUG5)
510+
};
511+
512+
(DEBUG4, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
513+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
514+
.set_domain($domain)
515+
$(.set_detail($detail))?
516+
.report($crate::elog::PgLogLevel::DEBUG4)
517+
};
518+
519+
(DEBUG3, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
520+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
521+
.set_domain($domain)
522+
$(.set_detail($detail))?
523+
.report($crate::elog::PgLogLevel::DEBUG3)
524+
};
525+
526+
(DEBUG2, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
527+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
528+
.set_domain($domain)
529+
$(.set_detail($detail))?
530+
.report($crate::elog::PgLogLevel::DEBUG2)
531+
};
532+
533+
(DEBUG1, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
534+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
535+
.set_domain($domain)
536+
$(.set_detail($detail))?
537+
.report($crate::elog::PgLogLevel::DEBUG1)
538+
};
539+
540+
($loglevel:expr, $domain:expr, $errcode:expr, $message:expr $(, $detail:expr)? $(,)?) => {
541+
$crate::panic::ErrorReport::new($errcode, $message, $crate::function_name!())
542+
.set_domain($domain)
543+
$(.set_detail($detail))?
544+
.report($loglevel);
545+
};
546+
}
547+
427548
/// If an interrupt is pending (perhaps a user-initiated "cancel query" message to this backend),
428549
/// this will safely abort the current transaction
429550
#[macro_export]

‎pgrx-pg-sys/src/submodules/ffi.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,11 @@ unsafe fn pg_guard_ffi_boundary_impl<T, F: FnOnce() -> T>(f: F) -> T {
213213
} else {
214214
CStr::from_ptr(errdata.message).to_string_lossy().to_string()
215215
};
216+
let domain = if errdata.domain.is_null() {
217+
None
218+
} else {
219+
{ Some(CStr::from_ptr(errdata.domain).to_string_lossy().to_string()) }
220+
};
216221
let detail = if errdata.detail.is_null() {
217222
None
218223
} else {
@@ -250,6 +255,7 @@ unsafe fn pg_guard_ffi_boundary_impl<T, F: FnOnce() -> T>(f: F) -> T {
250255
message,
251256
detail,
252257
hint,
258+
domain,
253259
location: ErrorReportLocation { file, funcname, line, col: 0, backtrace: None },
254260
},
255261
}))

‎pgrx-pg-sys/src/submodules/panic.rs‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ pub struct ErrorReport {
126126
pub(crate) message: String,
127127
pub(crate) hint: Option<String>,
128128
pub(crate) detail: Option<String>,
129+
pub(crate) domain: Option<String>,
129130
pub(crate) location: ErrorReportLocation,
130131
}
131132

@@ -206,6 +207,11 @@ impl ErrorReportWithLevel {
206207
self.inner.hint()
207208
}
208209

210+
/// Returns the domain of this error report, if set
211+
pub fn domain(&self) -> Option<&str> {
212+
self.inner.domain()
213+
}
214+
209215
/// Returns the name of the source file that generated this error report
210216
pub fn file(&self) -> &str {
211217
&self.inner.location.file
@@ -247,7 +253,14 @@ impl ErrorReport {
247253
let mut location: ErrorReportLocation = Location::caller().into();
248254
location.funcname = Some(funcname.to_string());
249255

250-
Self { sqlerrcode, message: message.into(), hint: None, detail: None, location }
256+
Self {
257+
sqlerrcode,
258+
message: message.into(),
259+
hint: None,
260+
detail: None,
261+
domain: None,
262+
location,
263+
}
251264
}
252265

253266
/// Create an [ErrorReport] which can be raised via Rust's [std::panic::panic_any()] or as
@@ -259,7 +272,14 @@ impl ErrorReport {
259272
message: S,
260273
location: ErrorReportLocation,
261274
) -> Self {
262-
Self { sqlerrcode, message: message.into(), hint: None, detail: None, location }
275+
Self {
276+
sqlerrcode,
277+
message: message.into(),
278+
hint: None,
279+
detail: None,
280+
domain: None,
281+
location,
282+
}
263283
}
264284

265285
/// Set the `detail` property, whose default is `None`
@@ -274,6 +294,16 @@ impl ErrorReport {
274294
self
275295
}
276296

297+
/// Set the `domain` property for message translation/internationalization, whose default is `None`.
298+
///
299+
/// This corresponds to the `domain` argument in Postgres' `ereport_domain` C macro.
300+
/// When set, the domain is passed to `errstart()` to indicate the gettext text domain
301+
/// for message translation.
302+
pub fn set_domain<S: Into<String>>(mut self, domain: S) -> Self {
303+
self.domain = Some(domain.into());
304+
self
305+
}
306+
277307
/// Returns the error message of this error report
278308
pub fn message(&self) -> &str {
279309
&self.message
@@ -289,6 +319,11 @@ impl ErrorReport {
289319
self.hint.as_deref()
290320
}
291321

322+
/// Returns the domain of this error report, if set
323+
pub fn domain(&self) -> Option<&str> {
324+
self.domain.as_deref()
325+
}
326+
292327
/// Report this [ErrorReport], which will ultimately be reported by Postgres at the specified [PgLogLevel]
293328
///
294329
/// If the provided `level` is >= [`PgLogLevel::ERROR`] this function will not return.
@@ -497,7 +532,7 @@ pub(crate) fn downcast_panic_payload(e: Box<dyn Any + Send>) -> CaughtError {
497532
/// trying to roll their own error handling.
498533
fn do_ereport(ereport: ErrorReportWithLevel) {
499534
const PERCENT_S: &CStr = c"%s";
500-
const DOMAIN: *const ::std::os::raw::c_char = std::ptr::null_mut();
535+
const DEFAULT_DOMAIN: *const ::std::os::raw::c_char = std::ptr::null_mut();
501536

502537
// the following code is definitely thread-unsafe -- not-the-main-thread can't be creating Postgres
503538
// ereports. Our secret `extern "C"` definitions aren't wrapped by #[pg_guard] so we need to
@@ -530,8 +565,15 @@ fn do_ereport(ereport: ErrorReportWithLevel) {
530565
}
531566

532567
let level = ereport.level();
568+
569+
// Use the domain from the ErrorReport if one was set, otherwise use the null default
570+
let domain_cstring = ereport.inner.domain.as_ref().map(|d| {
571+
std::ffi::CString::new(d.as_str()).expect("domain must not contain interior NUL bytes")
572+
});
573+
let domain_ptr = domain_cstring.as_ref().map_or(DEFAULT_DOMAIN, |c| c.as_ptr());
574+
533575
unsafe {
534-
if errstart(level as _, DOMAIN) {
576+
if errstart(level as _, domain_ptr) {
535577
let sqlerrcode = ereport.sql_error_code();
536578
let message = ereport.message().as_pg_cstr();
537579
let detail = ereport.detail_with_backtrace().as_pg_cstr();

‎pgrx-tests/src/tests/log_tests.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,42 @@ mod tests {
7474
pgrx::ereport!(PgLogLevel::ERROR, PgSqlErrorCode::ERRCODE_INTERNAL_ERROR, "ereport error")
7575
}
7676

77+
#[pg_test(error = "ereport error")]
78+
fn test_ereport_domain() {
79+
pgrx::ereport_domain!(
80+
PgLogLevel::ERROR,
81+
"test_extension_domain",
82+
PgSqlErrorCode::ERRCODE_INTERNAL_ERROR,
83+
"ereport error"
84+
)
85+
}
86+
87+
#[pg_test]
88+
fn test_ereport_domain_value() {
89+
let caught = pgrx::PgTryBuilder::new(|| -> Option<pgrx::pg_sys::panic::CaughtError> {
90+
pgrx::ereport_domain!(
91+
PgLogLevel::ERROR,
92+
"test_extension_domain",
93+
PgSqlErrorCode::ERRCODE_INTERNAL_ERROR,
94+
"ereport error"
95+
);
96+
None
97+
})
98+
.catch_when(PgSqlErrorCode::ERRCODE_INTERNAL_ERROR, |e| Some(e))
99+
.execute();
100+
101+
match caught {
102+
Some(pgrx::pg_sys::panic::CaughtError::ErrorReport(report))
103+
| Some(pgrx::pg_sys::panic::CaughtError::PostgresError(report)) => {
104+
assert_eq!(report.sql_error_code(), PgSqlErrorCode::ERRCODE_INTERNAL_ERROR);
105+
assert_eq!(report.message(), "ereport error");
106+
assert_eq!(report.domain(), Some("test_extension_domain"));
107+
}
108+
Some(other) => panic!("unexpected error kind: {other:?}"),
109+
None => panic!("expected error, but code returned normally"),
110+
}
111+
}
112+
77113
#[pg_test(error = "panic message")]
78114
fn test_panic() {
79115
panic!("panic message")

‎pgrx/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ pub use pg_sys::panic::pgrx_extern_c_guard;
124124
pub use pg_sys::pg_try::PgTryBuilder;
125125
pub use pg_sys::utils::name_data_to_str;
126126
pub use pg_sys::{
127-
FATAL, PANIC, check_for_interrupts, debug1, debug2, debug3, debug4, debug5, ereport, error,
128-
function_name, info, log, notice, warning,
127+
FATAL, PANIC, check_for_interrupts, debug1, debug2, debug3, debug4, debug5, ereport,
128+
ereport_domain, error, function_name, info, log, notice, warning,
129129
};
130130

131131
#[doc(hidden)]

‎pgrx/src/prelude.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,6 @@ pub use crate::spi::Spi;
6060
pub use crate::pg_sys::elog::PgLogLevel;
6161
pub use crate::pg_sys::errcodes::PgSqlErrorCode;
6262
pub use crate::pg_sys::{
63-
FATAL, PANIC, check_for_interrupts, debug1, debug2, debug3, debug4, debug5, ereport, error,
64-
function_name, info, log, notice, warning,
63+
FATAL, PANIC, check_for_interrupts, debug1, debug2, debug3, debug4, debug5, ereport,
64+
ereport_domain, error, function_name, info, log, notice, warning,
6565
};

0 commit comments

Comments
 (0)