On Tue, Feb 08, 2022 at 01:06:29PM +0900, Michael Paquier wrote:
> Makes sense. check_guc also checks after this pattern.
Okay, I have done all the adjustments you mentioned, added a couple of
comments and applied the patch. If the buildfarm is happy, I'll
go retire check_guc.
--
Michael
On Mon, Feb 07, 2022 at 09:07:28PM -0600, Justin Pryzby wrote:
> I think this is the regex I wrote to handle either "name = value" or "name
> value", which was needed between f47ed79cc..4d7c3e344. See skip_equals.
Yes, I took it from there, noticing that it was working just fine for
this
On Tue, Feb 08, 2022 at 10:44:07AM +0900, Michael Paquier wrote:
> What do you think about the updated version attached? I have applied
> the addition of config_data() separately.
Looks fine
> + # Check if this line matches a GUC parameter.
> + if ($line =~ m/^#?([_[:alpha:]]+) (= .*|[^
On Sun, Feb 06, 2022 at 09:04:14PM -0600, Justin Pryzby wrote:
> Your test is checking that stuff in sample.conf is actually a GUC and not
> marked NOT_IN_SAMPLE. But those are both unlikely mistakes to make.
Yeah, you are right. Still, I don't see any reason to not include both.
> I'd first
Thanks for working on it.
Your test is checking that stuff in sample.conf is actually a GUC and not
marked NOT_IN_SAMPLE. But those are both unlikely mistakes to make.
The important/interesting test is the opposite: that all GUCs are present in
the sample file. It's a lot easier for someone to
On Sun, Feb 06, 2022 at 02:09:45PM +0900, Michael Paquier wrote:
> Actually, I am thinking that we should implement it before retiring
> completely check_guc, but not in the fashion you are suggesting. I
> would be tempted to add something in the TAP tests as of
> src/test/misc/, where we
On Mon, Jan 31, 2022 at 04:56:45PM -0600, Justin Pryzby wrote:
> I'm not clear on what things are required/prohibited to allow/expect
> "installcheck" to pass. It's possible that postgresql.conf doesn't even exist
> in the data dir, right ?
There are no written instructions AFAIK, but I have as
On Mon, Jan 31, 2022 at 02:17:41PM +0900, Michael Paquier wrote:
> With all those doc fixes, applied after an extra round of review. So
> this makes us rather covered with the checks on the flags.
Thanks
> Now, what do we do with the rest of check_guc that involve a direct
> lookup at what's on
On Sat, Jan 29, 2022 at 06:18:50PM -0600, Justin Pryzby wrote:
> "The most meaningful ones are included" doesn't seem to add anything.
> Maybe it'd be useful to say "(Only the most useful flags are exposed)"
Yes, I have used something like that.
> I think the description is wrong, or just copied
On Sat, Jan 29, 2022 at 03:38:53PM +0900, Michael Paquier wrote:
> +-- Three exceptions as of transaction_*
> +SELECT name FROM pg_settings_flags
> + WHERE NOT no_show_all AND no_reset_all
> + ORDER BY 1;
> + name
> +
> + transaction_deferrable
> +
t
independently of the actual feature proposed, for clarity).
--
Michael
From a87afcfb5757367aeef795505e9361d5a03facc7 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Sat, 29 Jan 2022 15:32:10 +0900
Subject: [PATCH v2] Expose GUC flags in SQL function, replacing check_guc
Note-to-self: CATVERSION bum
= "$i" ] ; then
- hidden=1
-fi
- done
- if [ "$hidden" -eq 0 ] ; then
-grep -i '#'$i' ' postgresql.conf.sample > /dev/null
-if [ $? -ne 0 ] ; then
- echo "$i seems to be missing from postgresql.conf.sample";
-fi
- fi
+ grep "#$
On Tue, Jan 25, 2022 at 09:44:26PM -0600, Justin Pryzby wrote:
> It seems like an arbitrary and short-sighted policy to expose a handful of
> flags in the view for the purpose of retiring ./check_guc, but not expose
> other
> flags, because we thought we knew that no user could ever want them.
>
On Wed, Jan 26, 2022 at 09:54:43AM +0900, Michael Paquier wrote:
> On Tue, Jan 25, 2022 at 12:07:51PM -0600, Justin Pryzby wrote:
> > On Tue, Jan 25, 2022 at 11:47:14AM +0100, Peter Eisentraut wrote:
> >> Does this stuff have any value for users? I'm worried we are exposing a
> >> bunch of stuff
On Tue, Jan 25, 2022 at 12:07:51PM -0600, Justin Pryzby wrote:
> On Tue, Jan 25, 2022 at 11:47:14AM +0100, Peter Eisentraut wrote:
>> Does this stuff have any value for users? I'm worried we are exposing a
>> bunch of stuff that is really just for internal purposes. Like, what value
>> does
On Tue, Jan 25, 2022 at 11:47:14AM +0100, Peter Eisentraut wrote:
> On 25.01.22 02:07, Justin Pryzby wrote:
> > +CREATE TABLE pg_settings_flags AS SELECT name, category,
> > + 'NO_SHOW_ALL' =ANY(flags) AS no_show_all,
> > + 'NO_RESET_ALL' =ANY(flags) AS no_reset_all,
> > + 'NOT_IN_SAMPLE'
On 25.01.22 02:07, Justin Pryzby wrote:
+CREATE TABLE pg_settings_flags AS SELECT name, category,
+ 'NO_SHOW_ALL' =ANY(flags) AS no_show_all,
+ 'NO_RESET_ALL' =ANY(flags) AS no_reset_all,
+ 'NOT_IN_SAMPLE' =ANY(flags) AS not_in_sample,
+ 'EXPLAIN' =ANY(flags) AS
On Mon, Jan 24, 2022 at 07:07:29PM -0600, Justin Pryzby wrote:
> I think you'll find that this is how it's done elsewhere in postgres.
> In the frontend, see appendPQExpBufferChar and appendPGArray and 3e6e86abc.
> On the backend, see: git grep -F "'{'" |grep -w appendStringInfoChar
Yeah, I was
t; statement
- for hidethis in $INTENTIONALLY_NOT_INCLUDED ; do
-if [ "$hidethis" = "$i" ] ; then
- hidden=1
-fi
- done
- if [ "$hidden" -eq 0 ] ; then
- grep -i '#'$i' ' postgresql.conf.sample > /dev/null
-if [ $? -ne 0 ] ; then
- echo "$i
On Thu, Jan 06, 2022 at 02:19:08PM +0900, Michael Paquier wrote:
> On Wed, Jan 05, 2022 at 05:55:17PM -0600, Justin Pryzby wrote:
> > pg_settings is currently defined with "SELECT *". Is it fine to enumerate a
> > list of columns instead ?
>
> I'd like to think that this is a better practice
At Tue, 4 Jan 2022 21:06:48 -0600, Justin Pryzby wrote
in
> On Wed, Jan 05, 2022 at 11:47:57AM +0900, Kyotaro Horiguchi wrote:
> > At Tue, 28 Dec 2021 20:32:40 -0600, Justin Pryzby
> > wrote in
> In that case, *all* the flags should be exposed. There's currently 20, which
> means it may not
On Wed, Jan 05, 2022 at 05:55:17PM -0600, Justin Pryzby wrote:
> pg_settings is currently defined with "SELECT *". Is it fine to enumerate a
> list of columns instead ?
I'd like to think that this is a better practice when it comes
documenting the columns, but I don't see an actual need for this
ot; -eq 0 ] ; then
-grep -i '#'$i' ' postgresql.conf.sample > /dev/null
-if [ $? -ne 0 ] ; then
- echo "$i seems to be missing from postgresql.conf.sample";
-fi
- fi
+ grep "#$i " postgresql.conf.sample >/dev/null ||
+echo "$i seems to be miss
On Tue, Jan 04, 2022 at 09:06:48PM -0600, Justin Pryzby wrote:
> I think pg_get_guc_flags() may be best, but I'm interested to hear other
> opinions.
My opinion on this matter is rather close to what you have here with
handling things through one extra attribute. But I don't see the
point of
ote:
> > > One option is to expose the GUC flags in pg_settings, so this can all be
> > > done
> > > in SQL regression tests.
> > >
> > > Maybe the flags should be text strings, so it's a nicer user-facing
> > > interface.
&
At Tue, 28 Dec 2021 20:32:40 -0600, Justin Pryzby wrote
in
> On Thu, Dec 09, 2021 at 09:53:23AM -0600, Justin Pryzby wrote:
> > On Thu, Dec 09, 2021 at 05:17:54PM +0900, Michael Paquier wrote:
> > One option is to expose the GUC flags in pg_settings, so this can all be
>
bility of grep. And also, it would make sense to
> > return a non-zero exit code if an incompatibility is found for the
> > automation part.
>
> One option is to expose the GUC flags in pg_settings, so this can all be done
> in SQL regression tests.
>
> Maybe the fl
bility of grep. And also, it would make sense to
> > return a non-zero exit code if an incompatibility is found for the
> > automation part.
>
> One option is to expose the GUC flags in pg_settings, so this can all be done
> in SQL regression tests.
>
> Maybe the fl
or the
> automation part.
One option is to expose the GUC flags in pg_settings, so this can all be done
in SQL regression tests.
Maybe the flags should be text strings, so it's a nicer user-facing interface.
But then the field would be pretty wide, even though we're only adding it for
regression t
On Wed, Dec 08, 2021 at 01:23:51PM +0100, Peter Eisentraut wrote:
> I wasn't really aware of this script either. But I think it's a good idea
> to have it. But only if it's run automatically as part of a test suite run.
Okay. If we do that, I am wondering whether it would be better to
rewrite
On 08.12.21 07:27, Michael Paquier wrote:
I saw that Tom updated it within the last 12 months, which I took to mean that
it was still being maintained. But I'm okay with removing it.
Yes, I saw that as of bf8a662. With 42 incorrect reports, I still see
more evidence with removing it. Before
On Mon, Dec 06, 2021 at 07:36:55AM -0600, Justin Pryzby wrote:
> The script checks that guc.c and sample config are consistent.
>
> I think your undertanding of INTENTIONALLY_NOT_INCLUDED is not right.
> That's a list of stuff it "avoids reporting" as an suspected error, not an
> additional list
On Mon, Dec 06, 2021 at 03:58:39PM +0900, Michael Paquier wrote:
> On Sun, Dec 05, 2021 at 11:38:05PM -0600, Justin Pryzby wrote:
> > Thanks. One more item. The check_guc script currently outputs 68 false
> > positives - even though it includes a list of 20 exceptions. This is not
> > useful.
>
On Sun, Dec 05, 2021 at 11:38:05PM -0600, Justin Pryzby wrote:
> Thanks. One more item. The check_guc script currently outputs 68 false
> positives - even though it includes a list of 20 exceptions. This is not
> useful.
Indeed. Hmm. This script does a couple of things:
1) Check the format
On Fri, Dec 03, 2021 at 10:06:47AM +0900, Michael Paquier wrote:
> On Wed, Dec 01, 2021 at 11:17:34PM -0600, Justin Pryzby wrote:
> > I find it easier to read "wait before authentication ..." than "wait ...
> > before
> > authentication".
>
> I have a hard time seeing a strong difference here.
On Wed, Dec 01, 2021 at 11:17:34PM -0600, Justin Pryzby wrote:
> I find it easier to read "wait before authentication ..." than "wait ...
> before
> authentication".
I have a hard time seeing a strong difference here. At the end, I
have used what you suggested, adjusted the rest based on your
On Thu, Dec 02, 2021 at 02:11:38PM +0900, Michael Paquier wrote:
> On Wed, Dec 01, 2021 at 09:34:39PM -0600, Justin Pryzby wrote:
> >> @@ -2762,7 +2763,8 @@ static struct config_int ConfigureNamesInt[] =
> >>{"pre_auth_delay", PGC_SIGHUP, DEVELOPER_OPTIONS,
> >> -
On Wed, Dec 01, 2021 at 09:34:39PM -0600, Justin Pryzby wrote:
>> @@ -2762,7 +2763,8 @@ static struct config_int ConfigureNamesInt[] =
>> {"pre_auth_delay", PGC_SIGHUP, DEVELOPER_OPTIONS,
>> -gettext_noop("Waits N seconds on connection startup
>> before
> @@ -2142,7 +2142,8 @@ static struct config_int ConfigureNamesInt[] =
> {"post_auth_delay", PGC_BACKEND, DEVELOPER_OPTIONS,
> - gettext_noop("Waits N seconds on connection startup
> after authentication."),
> + gettext_noop("Sets the amount
On Wed, Dec 01, 2021 at 01:59:05AM -0600, Justin Pryzby wrote:
> On Tue, Nov 30, 2021 at 03:36:45PM +0900, Michael Paquier wrote:
>> -gettext_noop("Waits N seconds on connection startup
>> before authentication."),
>> +gettext_noop("Sets the amount of
On Tue, Nov 30, 2021 at 03:36:45PM +0900, Michael Paquier wrote:
> - gettext_noop("Forces a switch to the next WAL file if a
> "
> - "new file has not been started
> within N seconds."),
> + gettext_noop("Sets
On Mon, Nov 29, 2021 at 05:04:01PM +0900, Michael Paquier wrote:
> 0001, to adjust the units, and 0003, to make the GUC descriptions less
> unit-dependent, are good ideas.
Actually, after sleeping on it and doing some digging in
codesearch.debian.org, changing the units of max_identifier_length,
On Sun, Nov 28, 2021 at 09:08:33PM -0600, Justin Pryzby wrote:
> This isn't flagged with GUC_EXPLAIN:
> enable_incremental_sort
Yeah, that's inconsistent.
> This isn't marked GUC_NOT_IN_SAMPLE, like all other DEVELOPER_OPTIONS:
> trace_recovery_messages
Indeed.
> I'm not sure
.
https://www.postgresql.org/message-id/flat/16997-ff16127f6e0d1...@postgresql.org
--
Justin
>From b52bbf317126dbd75a2f8e98ceec4f3beb66f572 Mon Sep 17 00:00:00 2001
From: Justin Pryzby
Date: Sun, 28 Nov 2021 12:08:48 -0600
Subject: [PATCH 01/10] guc.c: fix GUC flags
---
src/backend/utils/m
44 matches
Mail list logo
