On Sat, Jun 11, 2022 at 09:41:37AM -0500, Justin Pryzby wrote:
> Note that this gives:
>
> guc.c:7573:9: warning: ‘dst’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> with gcc version 9.2.1 20191008 (Ubuntu 9.2.1-9ubuntu2)
>
> I wonder whether you'd consider renaming pg_normalize_config_value() to
> pg_pretty_config_value() or similar.
I have looked at the patch, and I am not convinced that we need a
function that does a integer -> integer-with-unit conversion for the
purpose of this test. One thing is that it can be unstable with the
unit in the conversion where values are close to a given threshold
(aka for cases like 2048kB/2MB). On top of that, this overlaps with
the existing system function in charge of converting values with bytes
as size unit, while this stuff handles more unit types and all GUC
types. I think that there could be some room in doing the opposite
conversion, feeding the value from postgresql.conf.sample to something
and compare it directly with boot_val. That's solvable at SQL level,
still a system function may be more user-friendly.
Extending the tests to check after the values is something worth
doing, but I think that I would limit the checks to the parameters
that do not have units for now, until we figure out which interface
would be more adapted for doing the normalization of the parameter
values.
-#syslog_facility = 'LOCAL0'
+#syslog_facility = 'local0'
Those changes should not be necessary in postgresql.conf.sample. The
test should be in charge of applying the lower() conversion, in the
same way as guc.c does internally, and that's a mode supported by the
parameter parsing. Using an upper-case value in the sample file is
actually meaningful sometimes (for example, syslog can use upper-case
strings to refer to LOCAL0~7).
--
Michael
