Re: GUC for data checksums

On 15.09.2013 17:05, Andres Freund wrote:
> On 2013-09-15 03:34:53 +0200, Bernd Helmle wrote:
>>
>>
>> --On 15. September 2013 00:25:34 +0200 Andres Freund
>> <andres(at)2ndquadrant(dot)com> wrote:
>>
>>> Looks like a good idea to me. The implementation looks sane as well,
>>> except that I am not sure if we really need to introduce that faux
>>> variable. If the variable cannot be set and we have a SHOW hook, do we
>>> need it?
>>
>> It's along the line with the other informational variables like block_size
>> et al. Do you want to have a function instead or what's your intention?
>
> Well, you've added a "data_checksums" variable that won't ever get used,
> right? You can't set the variable and the show hook doesn't actually use
> it.
> The reason you presumably did so is that there is no plain variable that
> contains information about data checksums, we first need to read the
> control file to know whether it's enabled and GUCs are initialized way
> earlier than that.
>
> A quick look unfortunately shows that there's no support for GUCs
> without an actual underlying variable, so unless somebody adds that,
> there doesn't seem to be much choice.
>
> I think a comment documenting that the data_checksums variable is not
> actually used would be appropriate.

Surprisingly we don't have any other gucs that would be set at initdb
time, and not changed after that. But we used to have two, lc_collate
and lc_ctype, until we changed them to be per-database settings. We used
to do this in ReadControlFile:

> /* Make the fixed locale settings visible as GUC variables, too */
> SetConfigOption("lc_collate", ControlFile->lc_collate,
> PGC_INTERNAL, PGC_S_OVERRIDE);
> SetConfigOption("lc_ctype", ControlFile->lc_ctype,
> PGC_INTERNAL, PGC_S_OVERRIDE);

I did the same for data_checksums, and committed. Thanks for the patch.

- Heikki