Skip to content

fix: don't allocate for static strings in ereport!#2282

Open
gruuya wants to merge 1 commit intopgcentralfoundation:developfrom
splitgraph:fix-ereport-static-str-allocation
Open

fix: don't allocate for static strings in ereport!#2282
gruuya wants to merge 1 commit intopgcentralfoundation:developfrom
splitgraph:fix-ereport-static-str-allocation

Conversation

@gruuya
Copy link
Copy Markdown
Contributor

@gruuya gruuya commented Apr 24, 2026

This is a spin-off from #2269; whereas in that PR we resolve the issue of eager allocation in high level logging macros (debug1, log, warning, etc.), there was one omission concerning only ereport!.

Namely, ereport! now uses IntoMessage::into_message to get the message from the input arg. When invoked indirectly via high level logging macros this is always std::fmt::Arguments, which in case of static strings results in a std::borrow::Cow::Borrowed message.

However, when ereport! is called directly with a static string this applies instead

impl IntoMessage for &str {
#[inline]
fn into_message(self) -> std::borrow::Cow<'static, str> {
std::borrow::Cow::Owned(self.to_owned())
}
}

In other words, in this case we always allocate, even for static strings. The fix is simple: replace impl IntoMessage for &str with impl IntoMessage for &'static str (since Rust doesn't allow both to co-exist), which can then return std::borrow::Cow::Borrowed for ereport!(..., "my string literal").

The only "downside" is that stuff like

      let msg = format!("table {} not found", table_name);
      ereport!(..., msg.as_str());

will error out during compilation. Arguably though this is for the better, since it forces the user to pass the string down, or call clone()/to_string()/to_owned(), making the allocation explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant