On Mon, Nov 11, 2024 at 12:09:15PM +0200, Heikki Linnakangas wrote:
> I had a quick glance at this, and I agree with Robert's comment earlier that
> this requires a lot of context to understand.
All this feedback was 4 weeks ago, and is unanswered, so I've marked
the CF entry as returned with feed
I had a quick glance at this, and I agree with Robert's comment earlier
that this requires a lot of context to understand.
Does this change the behavior of what is accepted or not? What are those
cases? Can you give some examples, please?
When do you get the new warnings? Examples please.
Do
On Fri, May 24, 2024 at 01:15:13PM -0400, Robert Haas wrote:
> + /* 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 th
On Fri, May 24, 2024 at 11:48 AM Justin Pryzby 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 h
On Fri, May 24, 2024 at 09:26:54AM -0400, Robert Haas wrote:
> On Thu, Jul 6, 2023 at 4:15 PM Justin Pryzby wrote:
> But then I realized, reading another email, that Justin already knew
> that the behavior would change, or at least I'm 90% certain that he
> knows that.
You give me too much credi
On Thu, Jul 6, 2023 at 4:15 PM Justin Pryzby wrote:
> I'm still hoping.
Hi,
I got asked to take a look at this thread.
First, I want to explain why I think this thread hasn't gotten as much
feedback as Justin was hoping. It is always possible for any thread to
have that problem just because peo
On Fri, Jul 22, 2022 at 03:26:47PM -0400, Tom Lane wrote:
> Hmph. I wonder if we shouldn't change that, because it's a lie.
> The value isn't actually coming from the config file, at least
> not yet.
On Thu, Jul 06, 2023 at 03:15:20PM -0500, Justin Pryzby wrote:
> On Sat, Oct 29, 2022 at 10:40 AM
On Thu, Dec 28, 2023 at 12:27 PM Shubham Khanna
wrote:
>
> I was reviewing the Patch and came across a minor issue that the Patch
> does not apply on the current Head. Please provide the updated version
> of the patch.
For your information, the commitfest manager has the ability to send
private m
On Thu, Dec 28, 2023 at 10:54 AM Justin Pryzby wrote:
>
> On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
> > > > > On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > > > > > It caused no issue when I changed:
> > > > > >
> > > > > > /* Check that it'
On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
> > > > On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > > > > It caused no issue when I changed:
> > > > >
> > > > > /* Check that it's acceptable for the
> > > > > indicated parameter */
> > > > >
On Mon, Oct 31, 2022 at 08:31:20AM -0500, Justin Pryzby wrote:
> On Sun, Oct 30, 2022 at 04:12:33PM -0700, Maciek Sakrejda wrote:
> > On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
> > > On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > > > It caused no issue when I changed
Justin Pryzby writes:
> On Sun, Oct 30, 2022 at 04:12:33PM -0700, Maciek Sakrejda wrote:
>> Also, for what it's worth, I think requiring the libraries to be in
>> place before running ALTER SYSTEM does not really seem that onerous. I
>> can't really think of use cases it precludes.
> Right now, i
On Sun, Oct 30, 2022 at 04:12:33PM -0700, Maciek Sakrejda wrote:
> On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
> >
> > On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > > It caused no issue when I changed:
> > >
> > > /* Check that it's acceptable
On Sat, Oct 29, 2022 at 10:40 AM Justin Pryzby wrote:
>
> On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> > It caused no issue when I changed:
> >
> > /* Check that it's acceptable for the indicated
> > parameter */
> > if (!parse_a
On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> It caused no issue when I changed:
>
> /* Check that it's acceptable for the indicated
> parameter */
> if (!parse_and_validate_value(record, name, value,
> -
On Fri, Sep 02, 2022 at 05:24:58PM -0500, Justin Pryzby wrote:
> I'm not sure where to go from here.
Not sure either, but the thread has no activity for a bit more than 1
month, so marked as RwF for now.
--
Michael
signature.asc
Description: PGP signature
On Fri, Jul 22, 2022 at 03:26:47PM -0400, Tom Lane wrote:
> Justin Pryzby writes:
> > On Fri, Jul 22, 2022 at 03:00:23PM -0400, Tom Lane wrote:
> >> Shouldn't you be doing this when the source is PGC_S_TEST, instead?
>
> > That makes sense, but it doesn't work for ALTER SYSTEM, which uses
> > PG
Justin Pryzby writes:
> On Fri, Jul 22, 2022 at 03:00:23PM -0400, Tom Lane wrote:
>> Shouldn't you be doing this when the source is PGC_S_TEST, instead?
> That makes sense, but it doesn't work for ALTER SYSTEM, which uses PGC_S_FILE.
Hmph. I wonder if we shouldn't change that, because it's a li
On Fri, Jul 22, 2022 at 03:00:23PM -0400, Tom Lane wrote:
> Justin Pryzby writes:
> > On Fri, Jul 22, 2022 at 01:53:21PM -0400, Tom Lane wrote:
> >> This indicates that the warning is being issued in the wrong place.
> >> It's okay if it comes out during ALTER SYSTEM. It's not okay if it
> >> com
Justin Pryzby writes:
> On Fri, Jul 22, 2022 at 01:53:21PM -0400, Tom Lane wrote:
>> This indicates that the warning is being issued in the wrong place.
>> It's okay if it comes out during ALTER SYSTEM. It's not okay if it
>> comes out during server start; then it's just an annoyance.
> The prev
On Fri, Jul 22, 2022 at 01:53:21PM -0400, Tom Lane wrote:
> > 2022-07-22 10:37:50.217 PDT [1131187] LOG: database system is shut down
> > 2022-07-22 10:37:50.306 PDT [1134058] WARNING: could not access file
> > "$libdir/plugins/lol"
> > 2022-07-22 10:37:50.306 PDT [1134058] DETAIL: The server w
Maciek Sakrejda writes:
>> I've started to think that we should really WARN whenever a (set of) GUC is
>> set
>> in a manner that the server will fail to start - not just for shared
>> libraries.
> +0.5. I think it's a reasonable change, but I've never broken my
> server with anything other tha
Thanks for picking this back up, Justin.
>I've started to think that we should really WARN whenever a (set of) GUC is set
>in a manner that the server will fail to start - not just for shared libraries.
+0.5. I think it's a reasonable change, but I've never broken my
server with anything other th
Finally returning to this .. rebased and updated per feedback.
I'm not sure of a good place to put test cases for this..
>From 13e8c8b96d1a5313fd3edde515a5278cf8c6b23e Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Mon, 13 Dec 2021 08:42:38 -0600
Subject: [PATCH v5 1/3] warn when setting GUC
I've started to think that we should really WARN whenever a (set of) GUC is set
in a manner that the server will fail to start - not just for shared libraries.
In particular, I think the server should also warn if it's going to fail to
start like this:
2022-06-15 22:48:34.279 CDT postmaster[20782
On Wed, Mar 23, 2022 at 03:02:23PM -0400, Tom Lane wrote:
> I agree with Maciek's concerns about these HINTs being emitted
> inappropriately, but I also have a stylistic gripe: they're only
> halfway hints. Given that we fix things so they only print when they
> should, the complaint about the ser
Maciek Sakrejda writes:
> In v4, the message looks fine to me for shared_preload_libraries
> (except there is a doubled "is"). However, I also get the message for
> a simple SET with local_preload_libraries:
> postgres=# set local_preload_libraries=xyz;
> WARNING: could not access file "xyz"
> H
On Wed, Feb 9, 2022 at 5:58 PM Justin Pryzby wrote:
> FYI, it has said "while..." and hasn't said "guc" since the 2nd revision of
> the
> patch.
The v3-0001 attached above had "while... for GUC..."--sorry I wasn't clear.
In v4, the message looks fine to me for shared_preload_libraries
(except t
On Fri, Jan 28, 2022 at 09:42:17AM -0500, Robert Haas wrote:
> -1 from me on using "guc" in any user-facing error message. And even
> guc -> setting isn't a big improvement. If we're going to structure
> the reporting this way there, we should try to use a meaningful phrase
> there, probably beginn
On Wed, Feb 2, 2022 at 7:39 AM David G. Johnston
wrote:
> I would at least consider having the UX go something like:
>
> postgres=# ALTER SYSTEM SET shared_preload_libraries = not_such_library;
> ERROR: that library is not present>.
> HINT: to bypass the error please add FORCE before SET
> postg
On Tue, Feb 1, 2022 at 11:06 PM Maciek Sakrejda
wrote:
> I tried running ALTER SYSTEM and got the warnings as expected:
>
> postgres=# alter system set shared_preload_libraries =
> no_such_library,not_this_one_either;
> WARNING: could not access file "$libdir/plugins/no_such_library"
> WARNING:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
I tried the latest version of the patch, and it works as discussed. T
Thanks for loooking
On Fri, Jan 28, 2022 at 11:36:20PM +, Cary Huang wrote:
> This is fine as this is what these patches are aiming to provide. However,
> when I try to restart the server, it fails to start because abc.so and xyz.so
> do not exist. Setting the parameters "local_preload_libra
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Hello
I tested the patches on master branch on Ubuntu 18.04 and
On Tue, Dec 28, 2021 at 12:45 PM Justin Pryzby wrote:
> 0002 adds context when failing to start.
>
> 2021-12-27 17:01:12.996 CST postmaster[1403] WARNING: could not load
> library: $libdir/plugins/asdf: cannot open shared object file: No such file
> or directory
> 2021-12-27 17:
On Sun, Jan 09, 2022 at 11:58:18AM -0800, Maciek Sakrejda wrote:
> On Sat, Jan 8, 2022 at 2:07 PM Justin Pryzby wrote:
> > Unfortunately, the output for dlopen() is not portable, which (I think)
> > means
> > most of what I wrote can't be made to work.. Since it doesn't work to call
> > dlopen()
On Sat, Jan 8, 2022 at 2:07 PM Justin Pryzby wrote:
> Unfortunately, the output for dlopen() is not portable, which (I think) means
> most of what I wrote can't be made to work.. Since it doesn't work to call
> dlopen() when using SET, I tried using just stat(). But that also fails on
> windows,
On Sat, Jan 08, 2022 at 01:29:24PM -0800, Maciek Sakrejda wrote:
> Thanks for working on this! I tried it out and it worked for me. I
> reviewed the patch and didn't see any problems, but I'm not much of a
> C programmer.
Thanks for looking at it. I was just hacking on it myself.
Unfortunately,
On Thu, Dec 30, 2021 at 12:21 AM Bharath Rupireddy
wrote:
> Overall the idea looks good to me. A warning on ALTER SYSTEM SET seems
> reasonable than nothing. ERROR isn't the way to go as it limits the
> users of setting the extensions in shared_preload_libraries first,
> installing them later. Is
Thanks for working on this! I tried it out and it worked for me. I
reviewed the patch and didn't see any problems, but I'm not much of a
C programmer.
On Tue, Dec 28, 2021 at 9:45 AM Justin Pryzby wrote:
> 0002 adds context when failing to start.
>
> 2021-12-27 17:01:12.996 CST postmaster
On Tue, Dec 28, 2021 at 11:15 PM Justin Pryzby wrote:
>
> forking
>
> On Mon, Dec 13, 2021 at 09:01:57AM -0500, Robert Haas wrote:
> > On Thu, Dec 9, 2021 at 2:32 AM Maciek Sakrejda wrote:
> > > > Considering the vanishingly small number of actual complaints we've
> > > > seen about this, that s
forking
On Mon, Dec 13, 2021 at 09:01:57AM -0500, Robert Haas wrote:
> On Thu, Dec 9, 2021 at 2:32 AM Maciek Sakrejda wrote:
> > > Considering the vanishingly small number of actual complaints we've
> > > seen about this, that sounds ridiculously over-engineered.
> > > A documentation example sh
42 matches
Mail list logo
