MIME-Version: 1.0
References: <20250911054220.3784-1-root@ip-172-31-36-228.ec2.internal>
 <CANWCAZYZQw-nzTXbx3Bk332VtY9_D7ksDsuMZ0A-iDZ53yG7Ng@mail.gmail.com>
 <CAK64mnfeWLBRbMfnOsag0vGTDnT84KJzpuei40nG0OHyw4SESw@mail.gmail.com>
 <CANWCAZa1b2rcvoK657SmcKwh2P2cgASQ1D-0JPj5d3LbfaAVgA@mail.gmail.com>
 <CAK64mneN20+sW5WhV+r7hMVo4Rd0z11B6=3L039rWMt1wK3nPg@mail.gmail.com>
 <CANWCAZZuS3sNgLRo8Z4AM=uY4zTmz=dH5D4Z9xV6K0CEuJ8Hdw@mail.gmail.com>
 <CAK64mnejn9AZMYz03e7HX8Uui35PihUuOy=b+iBG=YtRKx0Log@mail.gmail.com>
 <CANWCAZZ_0AQMk1HgHXHX+JaeBfy_4kzwHgTdqMptDA7zM+nm+Q@mail.gmail.com>
 <CAK64mnc6jbehHv5AHc84tVFRJg4zeMiFuvPX9xZkRpq0210MFA@mail.gmail.com>
In-Reply-To: 
 <CAK64mnc6jbehHv5AHc84tVFRJg4zeMiFuvPX9xZkRpq0210MFA@mail.gmail.com>
From: John Naylor <johncnaylorls@gmail.com>
Date: Fri, 14 Nov 2025 18:34:37 +0700
Message-ID: 
 <CANWCAZY940P3wGOQAZWMLQL4MQGGyOu7WBjBEcn_gqcrr+NvAw@mail.gmail.com>
Subject: Re: Proposal for enabling auto-vectorization for checksum
 calculations
To: Andrew Kim <tenistarkim@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org,
	Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CANWCAZY940P3wGOQAZWMLQL4MQGGyOu7WBjBEcn_gqcrr%2BNvAw%40mail.gmail.com>
Precedence: bulk

On Thu, Nov 6, 2025 at 6:50=E2=80=AFAM Andrew Kim <tenistarkim@gmail.com> w=
rote:
> The v9 patch series is attached.

Thanks! BTW, I've set the commitfest entry to "Needs Review".

I spent some time with the v9-0001 refactoring patch, and I have one
observation that's worth sharing now:

--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -29,8 +29,7 @@
 #include "getopt_long.h"
 #include "pg_getopt.h"
 #include "storage/bufpage.h"
-#include "storage/checksum.h"
-#include "storage/checksum_impl.h"
+#include "port/checksum.h"

Outside the backend it is no longer required to include the
implementation header -- it just links and works correctly. That seems
like a good thing. In fact...

On Tue, Oct 21, 2025 I wrote:
> Looking at commit f04216341dd1, we have at least one example of an
> external program, pg_filedump. If we can keep this working with
> minimal fuss, it should be fine everywhere.

> https://github.com/df7cb/pg_filedump/blob/master/pg_filedump.c#L29
>
> ```
> /* checksum_impl.h uses Assert, which doesn't work outside the server */
> #undef Assert
> #define Assert(X)
>
> #include "storage/checksum.h"
> #include "storage/checksum_impl.h"
> ```

I tried building pg_filedump against the server with just the 0001
patch and it also builds fine leaving out the implementation:

diff --git a/pg_filedump.c b/pg_filedump.c
index 606a85b..0268381 100644
--- a/pg_filedump.c
+++ b/pg_filedump.c
@@ -26,12 +26,7 @@

 #include <utils/pg_crc.h>

-/*     checksum_impl.h uses Assert, which doesn't work outside the server =
*/
-#undef Assert
-#define Assert(X)
-
-#include "storage/checksum.h"
-#include "storage/checksum_impl.h"
+#include "port/checksum.h"
 #include "decode.h"
 #include <inttypes.h>

I verified that it does in fact break when built against our master
branch without the impl.h header.

Further, if I replace the above CRC #include to point instead to our
hardware-specific API in port/pg_crc32c.h, it builds fine after
adjusting the typedef that it expects, and interprets the control file
normally:

<pg_control Contents> *********************************************

                          CRC: Correct
           pg_control Version: 1800
              Catalog Version: 202511051
...

I'll think more about this.

--
John Naylor
Amazon Web Services