Обсуждение: 003_check_guc.pl crashes if some extensions were loaded.

Hello!

Found that src/test/modules/test_misc/t/003_check_guc.pl will crash if an extension
that adds own GUCs was loaded into memory.
So it is now impossible to run a check-world with loaded extension libraries.

Reproduction:
cd src/test/modules/test_misc
export EXTRA_INSTALL="contrib/pg_stat_statements"
export TEMP_CONFIG=$(pwd)/pg_stat_statements_temp.conf
echo -e "shared_preload_libraries = 'pg_stat_statements'" > $TEMP_CONFIG
echo "compute_query_id = 'regress'" >> $TEMP_CONFIG
make check PROVE_TESTS='t/003_check_guc.pl'

# +++ tap check in src/test/modules/test_misc +++
t/003_check_guc.pl .. 1/?
#   Failed test 'no parameters missing from postgresql.conf.sample'
#   at t/003_check_guc.pl line 81.
#          got: '5'
#     expected: '0'
# Looks like you failed 1 test of 3.

Maybe exclude such GUCs from this test?
For instance, like that:

--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -19,7 +19,7 @@ my $all_params = $node->safe_psql(
         "SELECT name
       FROM pg_settings
     WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
-       name <> 'config_file'
+       name <> 'config_file' AND name NOT LIKE '%.%'
       ORDER BY 1");

With the best wishes,

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Thu, Nov 02, 2023 at 12:28:05AM +0300, Anton A. Melnikov wrote:
> Found that src/test/modules/test_misc/t/003_check_guc.pl will crash if an extension
> that adds own GUCs was loaded into memory.
> So it is now impossible to run a check-world with loaded extension libraries.

Right.  That's annoying, so let's fix it.

> --- a/src/test/modules/test_misc/t/003_check_guc.pl
> +++ b/src/test/modules/test_misc/t/003_check_guc.pl
> @@ -19,7 +19,7 @@ my $all_params = $node->safe_psql(
>         "SELECT name
>       FROM pg_settings
>     WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
> -       name <> 'config_file'
> +       name <> 'config_file' AND name NOT LIKE '%.%'
>       ORDER BY 1");

Wouldn't it be better to add a qual as of "category <> 'Customized
Options'"?  That's something arbitrarily assigned for all custom GUCs
and we are sure that none of them will exist in
postgresql.conf.sample.  There's also no guarantee that out-of-core
custom GUCs will include a dot in their name (even if I know that
maintainers close to the community adopt this convention and are
rather careful about that).
--
Michael

Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Nov 02, 2023 at 12:28:05AM +0300, Anton A. Melnikov wrote:
>> "SELECT name
>> FROM pg_settings
>> WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
>> -       name <> 'config_file'
>> +       name <> 'config_file' AND name NOT LIKE '%.%'
>> ORDER BY 1");

> Wouldn't it be better to add a qual as of "category <> 'Customized
> Options'"?

+1, seems like a cleaner answer.

> That's something arbitrarily assigned for all custom GUCs
> and we are sure that none of them will exist in
> postgresql.conf.sample.  There's also no guarantee that out-of-core
> custom GUCs will include a dot in their name (even if I know that
> maintainers close to the community adopt this convention and are
> rather careful about that).

Actually we do force that, see valid_custom_variable_name().
But I think your idea is better.

            regards, tom lane

On Wed, Nov 01, 2023 at 07:29:51PM -0400, Tom Lane wrote:
> Actually we do force that, see valid_custom_variable_name().
> But I think your idea is better.

Ah, indeed, thanks.  I didn't recall this was the case.
--
Michael

On 02.11.2023 01:53, Michael Paquier wrote:> On Thu, Nov 02, 2023 at 12:28:05AM +0300, Anton A. Melnikov wrote:
>> Found that src/test/modules/test_misc/t/003_check_guc.pl will crash if an extension
>> that adds own GUCs was loaded into memory.
>> So it is now impossible to run a check-world with loaded extension libraries.
> 
> Right.  That's annoying, so let's fix it.

Thanks!

On 02.11.2023 02:29, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> Wouldn't it be better to add a qual as of "category <> 'Customized
>> Options'"?
> 
> +1, seems like a cleaner answer.

Also agreed. That is a better variant!


-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company