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
signature.asc
Description: PGP signature
