On Fri, May 24, 2024 at 11:48 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> You give me too much credit..
Gee, usually I'm very good at avoiding that mistake. :-)
> We don't want to change the behavior to allow this to succeed -- it
> would allow leaving the server in a state that it fails to start (rather
> than helping to avoid doing so, as intended by this thread).
+1.
> Maybe there should be a comment explaning why PGC_FILE is used, and
> maybe there should be a TAP test for the behavior, but they're pretty
> unrelated to this thread. So, I've dropped the 001 patch.
+1 for that, too.
+ /* Note that filename was already canonicalized */
I see that this comment is copied from load_libraries(), but I don't
immediately see where the canonicalization actually happens. Do you
know, or can you find out? Because that's crucial here, else stat()
might not target the real filename. I wonder if it will anyway. Like,
couldn't the library be versioned, and might not dlopen() try a few
possibilities?
+ errdetail("The server will currently fail to start with this setting."),
+ errdetail("New sessions will currently fail to connect with the new
setting."));
I understand why these messages have the word "currently" in them, but
I bet the user won't. I'm not sure exactly what to recommend at the
moment (and I'm quite busy today due to the conference upcoming) but I
think we should try to find some way to rephrase these.
--
Robert Haas
EDB: http://www.enterprisedb.com