pg_control_checkpoint(): add "data_checksum_version" (Pg19)?

On 10 Aug 2026, at 06:14, Ian Lawrence Barwick <barwick@gmail.com> wrote:

Commit f19c0eccae adds the data_checksum_version field to the pg_controldata
output, but there is no matching entry in "pg_control_checkpoint()". I can't
see any reason why this is omitted (I don't see any mention of it in the
corresponding -hackers thread [*]) so assume it's not intentional. A similar
update was made in 99e949f84 for logical decoding status.

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.

--
Daniel Gustafsson

On Mon, Aug 10, 2026 at 5:13 PM Daniel Gustafsson <daniel@yesql.se> 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

2026年8月10日(月) 20:06 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Aug 10, 2026 at 5:13 PM Daniel Gustafsson <daniel@yesql.se> 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".

Makes sense (somehow I have "data_checksum represents a version" ingrained
in my memory and hadn't caught up with the change yet).

+       <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.

Also makes sense.

If we go this way, I think pg_controldata should also report the textual
checksum state.

The function "get_checksum_state_string()" which performs the conversion is in
src/backend/access/rmgrdesc/xlogdesc.c, which AFAIR can't be linked from
frontend utilities, so we'd either have to move that somewhere else (maybe
src/common/controldata_utils.c?) or duplicate it.

Regards

Ian Barwick

On Mon, Aug 10, 2026 at 9:39 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

The function "get_checksum_state_string()" which performs the conversion is in
src/backend/access/rmgrdesc/xlogdesc.c, which AFAIR can't be linked from
frontend utilities, so we'd either have to move that somewhere else (maybe
src/common/controldata_utils.c?) or duplicate it.

Agreed. I think moving it to common is better than duplicating the mapping.

src/common/controldata_utils.c seems like a reasonable place, since
this state is stored in the control/checkpoint data and both pg_controldata
and pg_control_checkpoint() already use controldata_utils.

Regards,

--
Fujii Masao

2026年8月10日(月) 22:30 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Aug 10, 2026 at 9:39 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

The function "get_checksum_state_string()" which performs the conversion is in
src/backend/access/rmgrdesc/xlogdesc.c, which AFAIR can't be linked from
frontend utilities, so we'd either have to move that somewhere else (maybe
src/common/controldata_utils.c?) or duplicate it.

Agreed. I think moving it to common is better than duplicating the mapping.

src/common/controldata_utils.c seems like a reasonable place, since
this state is stored in the control/checkpoint data and both pg_controldata
and pg_control_checkpoint() already use controldata_utils.

TBH it feels a little odd there because it's not really specific to
the controldata
output functions, but it has to go somewhere, I guess.

Updated patch with that and changes per your other suggestions attached.

Regards

Ian Barwick

On 10 Aug 2026, at 16:21, Ian Lawrence Barwick <barwick@gmail.com> wrote:
2026年8月10日(月) 22:30 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Aug 10, 2026 at 9:39 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

The function "get_checksum_state_string()" which performs the conversion is in
src/backend/access/rmgrdesc/xlogdesc.c, which AFAIR can't be linked from
frontend utilities, so we'd either have to move that somewhere else (maybe
src/common/controldata_utils.c?) or duplicate it.

I originally opted against displaying the mapped name to keep
pg_control_checkpoint a way to extract the raw data, but thinking about it
might be more useful to show the mapped value since the mapping is private.

Agreed. I think moving it to common is better than duplicating the mapping.

src/common/controldata_utils.c seems like a reasonable place, since
this state is stored in the control/checkpoint data and both pg_controldata
and pg_control_checkpoint() already use controldata_utils.

TBH it feels a little odd there because it's not really specific to
the controldata output functions, but it has to go somewhere, I guess.

Yeah, I'm not convinced it's the right place but that being said I also don't
have a better idea yet. If I can't think of a better place by the time the
tree is open I say we go with this placement.

Thanks for the updated patch!

--
Daniel Gustafsson

On Aug 11, 2026, at 02:07, Daniel Gustafsson <daniel@yesql.se> wrote:

On 10 Aug 2026, at 16:21, Ian Lawrence Barwick <barwick@gmail.com> wrote:
2026年8月10日(月) 22:30 Fujii Masao <masao.fujii@gmail.com>:

On Mon, Aug 10, 2026 at 9:39 PM Ian Lawrence Barwick <barwick@gmail.com> wrote:

The function "get_checksum_state_string()" which performs the conversion is in
src/backend/access/rmgrdesc/xlogdesc.c, which AFAIR can't be linked from
frontend utilities, so we'd either have to move that somewhere else (maybe
src/common/controldata_utils.c?) or duplicate it.

I originally opted against displaying the mapped name to keep
pg_control_checkpoint a way to extract the raw data, but thinking about it
might be more useful to show the mapped value since the mapping is private.

Agreed. I think moving it to common is better than duplicating the mapping.

src/common/controldata_utils.c seems like a reasonable place, since
this state is stored in the control/checkpoint data and both pg_controldata
and pg_control_checkpoint() already use controldata_utils.

TBH it feels a little odd there because it's not really specific to
the controldata output functions, but it has to go somewhere, I guess.

Yeah, I'm not convinced it's the right place but that being said I also don't
have a better idea yet. If I can't think of a better place by the time the
tree is open I say we go with this placement.

I don’t see a better place either.

Overall, v3 looks good to me. I only have one small comment. Since get_checksum_state_string() has been moved to a common location and can be called by both frontend and backend code, would it be better to give it a more specific name, such as get_data_checksum_state_string()?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Tue, Aug 11, 2026 at 3:07 AM Daniel Gustafsson <daniel@yesql.se> wrote:

I originally opted against displaying the mapped name to keep
pg_control_checkpoint a way to extract the raw data, but thinking about it
might be more useful to show the mapped value since the mapping is private.

I suggested making pg_controldata report the latest checkpoint's checksum state
as text rather than as a version. But, I found that pg_controldata reports
similar information "Data page checksum version", and pg_control_init()
exposes data_page_checksum_version as an integer version.

This makes me wonder whether we should drop my suggestion and keep the latest
checkpoint's checksum field as a version as well, or instead move toward
reporting textual checksum states consistently in all these places.
Or is there another approach that would make more sense?

Regards,

--
Fujii Masao

On 11 Aug 2026, at 16:38, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Aug 11, 2026 at 3:07 AM Daniel Gustafsson <daniel@yesql.se> wrote:

I originally opted against displaying the mapped name to keep
pg_control_checkpoint a way to extract the raw data, but thinking about it
might be more useful to show the mapped value since the mapping is private.

I suggested making pg_controldata report the latest checkpoint's checksum state
as text rather than as a version. But, I found that pg_controldata reports
similar information "Data page checksum version", and pg_control_init()
exposes data_page_checksum_version as an integer version.

This makes me wonder whether we should drop my suggestion and keep the latest
checkpoint's checksum field as a version as well, or instead move toward
reporting textual checksum states consistently in all these places.
Or is there another approach that would make more sense?

I'm leaning towards using an integer representation in v19, which is the safe
option, and rethink it for v20 when we can consider it without any rush. A
compromise could be do document the integer <=> state name mapping in the
checksums section in the docs.

--
Daniel Gustafsson

On Tue, Aug 11, 2026 at 11:56 PM Daniel Gustafsson <daniel@yesql.se> wrote:

I'm leaning towards using an integer representation in v19, which is the safe
option, and rethink it for v20 when we can consider it without any rush. A
compromise could be do document the integer <=> state name mapping in the
checksums section in the docs.

+1 to keep using "version" for now and clarify the mapping in the docs.

Regards,

--
Fujii Masao

[RMT hat]

FTR I've added this one as an open item for v19.

--
nathan

On 12 Aug 2026, at 02:29, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Aug 11, 2026 at 11:56 PM Daniel Gustafsson <daniel@yesql.se> wrote:

I'm leaning towards using an integer representation in v19, which is the safe
option, and rethink it for v20 when we can consider it without any rush. A
compromise could be do document the integer <=> state name mapping in the
checksums section in the docs.

+1 to keep using "version" for now and clarify the mapping in the docs.

I took a stab at fixing this today and in doing so I realized another issue
which isn't directly related to online checksums but also affect the offline
checksums available in all supported versions. pg_control_init is documented
to return the state at cluster initialization, but the checksum version in the
controlfile is overwritten when changed, and the original value is lost. The
attached 0001 fixes this by adding a new field to the controlfile. I think
this should be backpatched to 19, but it could also be argued against that at
this point in the cycle since it's a pre-existing regression (especially since
it updates the controlfile format).

The 0002 use data_page_checksum_version consistently, adds a mapping table to
the documentation and fixes the tests.

--
Daniel Gustafsson

Hi,

On Thu, Aug 13, 2026 at 12:17:16PM +0200, Daniel Gustafsson wrote:

On 12 Aug 2026, at 02:29, Fujii Masao <masao.fujii@gmail.com> wrote:

On Tue, Aug 11, 2026 at 11:56 PM Daniel Gustafsson <daniel@yesql.se> wrote:

I'm leaning towards using an integer representation in v19, which is the safe
option, and rethink it for v20 when we can consider it without any rush. A
compromise could be do document the integer <=> state name mapping in the
checksums section in the docs.

+1 to keep using "version" for now and clarify the mapping in the docs.

I took a stab at fixing this today and in doing so I realized another issue
which isn't directly related to online checksums but also affect the offline
checksums available in all supported versions. pg_control_init is documented
to return the state at cluster initialization, but the checksum version in the
controlfile is overwritten when changed, and the original value is lost. The
attached 0001 fixes this by adding a new field to the controlfile. I think
this should be backpatched to 19, but it could also be argued against that at
this point in the cycle since it's a pre-existing regression (especially since
it updates the controlfile format).

The 0002 use data_page_checksum_version consistently, adds a mapping table to
the documentation and fixes the tests.

Thanks for the updates!

I've a few comments:

=== 1

+# Regardless of the new state, pg_control_init() should still report checksums
+# as off.

Could we also test the opposite case and ensure pg_control_init() still reports 1?
Also, as 0001 fixes the offline case too, worth checking pg_checksums leaves the value
reported by pg_control_init() unchanged?

And should this test be part of 0001 instead of 0002?

=== 2

+   version using an integer representation due to how they were originally
+   implementated.

s/implementated/implemented/

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 14 Aug 2026, at 07:47, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Thanks for the updates!

Thanks for review!

+# Regardless of the new state, pg_control_init() should still report checksums
+# as off.

Could we also test the opposite case and ensure pg_control_init() still reports 1?
Also, as 0001 fixes the offline case too, worth checking pg_checksums leaves the value
reported by pg_control_init() unchanged?

Sure, I can add that as well.

And should this test be part of 0001 instead of 0002?

Agreed, it should.

+   version using an integer representation due to how they were originally
+   implementated.

s/implementated/implemented/

Thanks, will fix.

--
Daniel Gustafsson

On 14 Aug 2026, at 16:34, Daniel Gustafsson <daniel@yesql.se> wrote:

Thanks, will fix.

Attached is a rebased patchset for this, which I want to go ahead with. I've
added a third, to the rest of the patches, unrelated patch which cleans up a
few unqualified queries in the test suites. It's pretty uninteresting as it
goes, so I bundled it here to get it in (since I found these when poking at
patchset).

--
Daniel Gustafsson

Hi,

On Mon, Aug 17, 2026 at 04:38:30PM +0200, Daniel Gustafsson wrote:

On 14 Aug 2026, at 16:34, Daniel Gustafsson <daniel@yesql.se> wrote:

Thanks, will fix.

Attached is a rebased patchset for this, which I want to go ahead with.

Thanks! Just 2 comments:

=== 1

+# Make sure pg_control_init reports the initial disabled state
+$result = $node->safe_psql('postgres',
+       'SELECT data_page_checksum_version FROM pg_control_init();');
+is($result, '0', 'ensure pg_control_init reports disabled state');

This is done before the checksum state changes, when both the initial and current
states are off. So it does not verify that pg_control_init() preserves the initial
value after a state change.

Worth to move it (or add one) here?

"
# Finish test suite by enabling checksums and make sure all data can be read
# back and no processes are left over
enable_data_checksums($node, wait => 'on');
"

=== 2

Also worth checking in 004_offline.pl after,

"
# Enable checksums offline using pg_checksums
$node->stop;
$node->checksum_enable_offline;
$node->start;
"

that pg_control_init() still reports the initial disabled state? That would also
verify for the pg_checksums case.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 18 Aug 2026, at 06:55, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Thanks for reviewing!

+# Make sure pg_control_init reports the initial disabled state
+$result = $node->safe_psql('postgres',
+       'SELECT data_page_checksum_version FROM pg_control_init();');
+is($result, '0', 'ensure pg_control_init reports disabled state');

This is done before the checksum state changes, when both the initial and current
states are off. So it does not verify that pg_control_init() preserves the initial
value after a state change.

Correct, the intention was to test the initial state.

Worth to move it (or add one) here?

"
# Finish test suite by enabling checksums and make sure all data can be read
# back and no processes are left over
enable_data_checksums($node, wait => 'on');

Added a new test here to make it's still reported per the initdb state.

Also worth checking in 004_offline.pl after,

"
# Enable checksums offline using pg_checksums
$node->stop;
$node->checksum_enable_offline;
$node->start;
"

that pg_control_init() still reports the initial disabled state? That would also
verify for the pg_checksums case.

Added.

I also added another test for pg_control_checkpoint in 004_offline to test the
state before and after a CHECKPOINT from offline checksum enabling.

--
Daniel Gustafsson

Hi,

On Tue, Aug 18, 2026 at 10:41:51AM +0200, Daniel Gustafsson wrote:

On 18 Aug 2026, at 06:55, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Added a new test here to make it's still reported per the initdb state.

Also worth checking in 004_offline.pl after,

"
# Enable checksums offline using pg_checksums
$node->stop;
$node->checksum_enable_offline;
$node->start;
"

that pg_control_init() still reports the initial disabled state? That would also
verify for the pg_checksums case.

Added.

Thanks for the updated version!

I also added another test for pg_control_checkpoint in 004_offline to test the
state before and after a CHECKPOINT from offline checksum enabling.

Yeah, makes sense to me.

I don't have more comments, LGTM.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com