From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Justin Pryzby <pryzby(at)telsasoft(dot)com>, Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fix GUC_NO_SHOW_ALL test scenario in 003_check_guc.pl |
Date: | 2023-01-30 05:06:10 |
Message-ID: | Y9dQQkqrxkUFxpsv@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Jan 29, 2023 at 01:05:07PM -0500, Tom Lane wrote:
> I kind of think this is a lot of unnecessary work. The case that is
> problematic is a GUC that's marked GUC_NO_SHOW_ALL but not marked
> GUC_NOT_IN_SAMPLE. There aren't any of those, and I don't think there
> are likely to be any in future, because it doesn't make a lot of sense.
> Why don't we just make a policy against doing that, and enforce it
> with an assertion somewhere in GUC initialization?
[..thinks..]
Looking at guc.sql, I think that these is a second mistake: the test
checks for (no_show_all AND !no_reset_all) but this won't work
because NO_SHOW_ALL GUCs cannot be scanned via SQL. There are two
parameters that include this combination of flags: default_with_oids
and ssl_renegotiation_limit, so the check would not do what it
should. I think that this part should be removed.
For the second part to prevent GUCs to be marked as NO_SHOW_ALL &&
!NOT_IN_SAMPLE, check_GUC_init() looks like the right place to me,
because this routine has been designed exactly for this purpose.
So, what do you think about the attached?
--
Michael
Attachment | Content-Type | Size |
---|---|---|
guc-checks.patch | text/x-diff | 3.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | wangw.fnst@fujitsu.com | 2023-01-30 05:06:48 | RE: Logical replication timeout problem |
Previous Message | Amit Kapila | 2023-01-30 04:57:14 | Re: Assertion failure in SnapBuildInitialSnapshot() |