On Wed, Oct 26, 2022 at 10:13:29AM +0530, Bharath Rupireddy wrote:
> +1 for improving the test coverage. Is there a strong reason to
> validate individual output columns rather than select count(*) > 0
> from pg_control_XXXX(); sort of tests? If the intention is to validate
> the pg_controlfile contents, we have pg_controldata to look at and
> pg_control_XXXX() functions doing crc checks.
And it could be possible that the control file finishes by writing
some incorrect data due to a bug in the backend. Adding a count(*) or
similar to get the number of fields of the function is basically the
same as checking its execution, still I'd like to think that having a
minimum set of checks would be kind of nice on top of that. Among all
the ones I wrote in the patch upthread, the following ones would be in
my minimalistic list:
- timeline_id > 0
- timeline_id >= prev_timeline_id
- checkpoint_lsn >= redo_lsn
- data_page_checksum_version >= 0
- Perhaps the various fields of pg_control_init() using their
lower-bound values.
- Perhaps pg_control_version and/or catalog_version_no > NN
> If this isn't enough, we
> can have the pg_control_validate() function to do all the necessary
> checks and simplify the tests, no?
There is no function like that. Perhaps that you mean to introduce
something like that at the C level, but that does not seem necessary
to me as long as a SQL is able to do the job for the most meaningful
parts.
--
Michael
