On 7/27/21 6:40 PM, Tom Lane wrote: > While cleaning out dead branches in my git repo, I came across an > early draft of what eventually became commit ffa2e4670 ("In libpq, > always append new error messages to conn->errorMessage"). I realized > that it contained a good idea that had gotten lost on the way to that > commit. Namely, let's reduce all of the 60-or-so "out of memory" > reports in libpq to calls to a common subroutine, and then let's teach > the common subroutine a recovery strategy for the not-unlikely > possibility that it fails to append the "out of memory" string to > conn->errorMessage. That recovery strategy of course is to reset the > errorMessage buffer to empty, hopefully regaining some space. We lose > whatever we'd had in the buffer before, but we have a better chance of > the "out of memory" message making its way to the user. > > The first half of that just saves a few hundred bytes of repetitive > coding. However, I think that the addition of recovery logic is > important for robustness, because as things stand libpq may be > worse off than before for OOM handling. Before ffa2e4670, almost > all of these call sites did printfPQExpBuffer(..., "out of memory"). > That would automatically clear the message buffer to empty, and > thereby be sure to report the out-of-memory failure if at all > possible. Now we might fail to report the thing that the user > really needs to know to make sense of what happened. > > Therefore, I feel like this was an oversight in ffa2e4670, > and we ought to back-patch the attached into v14. > > cc'ing the RMT in case they wish to object. > >
I'm honored you've confused me with Alvaro :-) This seems sensible, and we certainly shouldn't be worse off than before, so let's do it. I'm fine with having two functions for call simplicity, but I don't feel strongly about it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com