On Mon, Aug 10, 2026 at 5:13 PM Daniel Gustafsson <[email protected]> wrote:
> Thanks, that was indeed missed in the original commit. I reworked the patch a
> tiny bit and added it to the test suite as per the attached v2. Once the
> REL_19_STABLE branch is open I'll push this along with a catversion bump.
+1
I have a few minor review comments:
+ <row>
+ <entry><structfield>data_checksum_state</structfield></entry>
pg_control_checkpoint() uses "data_checksum_state", while pg_controldata
calls the same field "Latest checkpoint's data_checksum_version".
Isn't it be better to use the same name in both places?
Personally I prefer "data_checksum_state" because the value is no longer
just a checksum format version. It can represent off, on, inprogress-off,
and inprogress-on, so "state" seems more accurate than "version".
+ <entry><type>integer</type></entry>
+ </row>
Isn't it be more useful for this column to return the textual checksum
state rather than the raw integer value? The value represents the same
states exposed by the data_checksums GUC: on, off, inprogress-on, and
inprogress-off. Returning 0 through 3 exposes the internal enum values
and seems less user-friendly.
If we go this way, I think pg_controldata should also report the textual
checksum state.
+# Ensure the new state is registered properly in pg_control_checkpoint()
+$result =
+ $node->safe_psql('postgres', 'SELECT data_checksum_state FROM
pg_control_checkpoint();');
+is($result, '1', 'ensure pg_control_checkpoint reports correct state');
<snip>
+# And ensure the disabled state is shown in pg_control_checkpoint()
+$result =
+ $node->safe_psql('postgres', 'SELECT data_checksum_state FROM
pg_control_checkpoint();');
+is($result, '0', 'ensure pg_control_checkpoint reports correct state');
The two new tests use the same test name
"ensure pg_control_checkpoint reports correct state".
It would be easier to diagnose failures if the names distinguished
the two cases, for example:
ensure pg_control_checkpoint reports on state
ensure pg_control_checkpoint reports off state
In commit log message:
Backpatch to v19 where onlince checksums were introduced.
Typo: "onlince" should be "online".
Regards,
--
Fujii Masao
