2026年8月10日(月) 20:06 Fujii Masao <[email protected]>:
>
> 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".

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


Reply via email to