agora inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v17 08/23] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
6+ messages / 2 participants
[nested] [flat]

* [PATCH v17 08/23] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
@ 2022-09-26 21:05 Andres Freund <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Andres Freund @ 2022-09-26 21:05 UTC (permalink / raw)

There's no need for two different set of flags, as we'll only ever run on a
one architecture at a time... Simplifies meson PGXS compatibility.
---
 src/port/Makefile      | 16 ++++++++--------
 config/c-compiler.m4   |  8 ++++----
 configure              | 19 +++++++++----------
 configure.ac           | 10 +++++-----
 src/Makefile.global.in |  3 +--
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/src/port/Makefile b/src/port/Makefile
index b3754d8940a..711f59e32bd 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -88,15 +88,15 @@ libpgport.a: $(OBJS)
 thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
 
-# all versions of pg_crc32c_sse42.o need CFLAGS_SSE42
-pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_SSE42)
+# all versions of pg_crc32c_sse42.o need CFLAGS_CRC
+pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
-# all versions of pg_crc32c_armv8.o need CFLAGS_ARMV8_CRC32C
-pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
+# all versions of pg_crc32c_armv8.o need CFLAGS_CRC
+pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
 #
 # Shared library versions of object files
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 000b075312e..eb8cc8ce170 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -597,7 +597,7 @@ fi])# PGAC_HAVE_GCC__ATOMIC_INT64_CAS
 # the other ones are, on x86-64 platforms)
 #
 # An optional compiler flag can be passed as argument (e.g. -msse4.2). If the
-# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_SSE42.
+# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_SSE42_CRC32_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_sse42_crc32_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=$1], [Ac_cachevar],
@@ -613,7 +613,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <nmmintrin.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_SSE42="$1"
+  CFLAGS_CRC="$1"
   pgac_sse42_crc32_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
@@ -629,7 +629,7 @@ undefine([Ac_cachevar])dnl
 #
 # An optional compiler flag can be passed as argument (e.g.
 # -march=armv8-a+crc). If the intrinsics are supported, sets
-# pgac_armv8_crc32c_intrinsics, and CFLAGS_ARMV8_CRC32C.
+# pgac_armv8_crc32c_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_ARMV8_CRC32C_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_armv8_crc32c_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=$1], [Ac_cachevar],
@@ -647,7 +647,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <arm_acle.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="$1"
+  CFLAGS_CRC="$1"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
diff --git a/configure b/configure
index 1caca21b625..80b28cb9310 100755
--- a/configure
+++ b/configure
@@ -645,8 +645,7 @@ MSGMERGE
 MSGFMT_FLAGS
 MSGFMT
 PG_CRC32C_OBJS
-CFLAGS_ARMV8_CRC32C
-CFLAGS_SSE42
+CFLAGS_CRC
 LIBOBJS
 ZSTD
 LZ4
@@ -17815,7 +17814,7 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=" >&5
 $as_echo_n "checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=... " >&6; }
 if ${pgac_cv_sse42_crc32_intrinsics_+:} false; then :
@@ -17850,7 +17849,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics_" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics_" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics_" = x"yes"; then
-  CFLAGS_SSE42=""
+  CFLAGS_CRC=""
   pgac_sse42_crc32_intrinsics=yes
 fi
 
@@ -17889,13 +17888,12 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics__msse4_2" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics__msse4_2" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics__msse4_2" = x"yes"; then
-  CFLAGS_SSE42="-msse4.2"
+  CFLAGS_CRC="-msse4.2"
   pgac_sse42_crc32_intrinsics=yes
 fi
 
 fi
 
-
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -17922,7 +17920,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=" >&5
 $as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... " >&6; }
 if ${pgac_cv_armv8_crc32c_intrinsics_+:} false; then :
@@ -17959,7 +17957,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics_" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics_" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics_" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C=""
+  CFLAGS_CRC=""
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
@@ -18000,13 +17998,14 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="-march=armv8-a+crc"
+  CFLAGS_CRC="-march=armv8-a+crc"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
 fi
 
 
+
 # Select CRC-32C implementation.
 #
 # If we are targeting a processor that has Intel SSE 4.2 instructions, we can
@@ -18034,7 +18033,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/configure.ac b/configure.ac
index 10fa55dd154..f837a602d01 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2086,12 +2086,11 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 PGAC_SSE42_CRC32_INTRINSICS([])
 if test x"$pgac_sse42_crc32_intrinsics" != x"yes"; then
   PGAC_SSE42_CRC32_INTRINSICS([-msse4.2])
 fi
-AC_SUBST(CFLAGS_SSE42)
 
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
@@ -2105,12 +2104,13 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 PGAC_ARMV8_CRC32C_INTRINSICS([])
 if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
   PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc])
 fi
-AC_SUBST(CFLAGS_ARMV8_CRC32C)
+
+AC_SUBST(CFLAGS_CRC)
 
 # Select CRC-32C implementation.
 #
@@ -2139,7 +2139,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d8ea2da583c..c87b6d97ca9 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -262,8 +262,7 @@ CFLAGS_SL_MODULE = @CFLAGS_SL_MODULE@
 CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@
 CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@
 CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@
-CFLAGS_SSE42 = @CFLAGS_SSE42@
-CFLAGS_ARMV8_CRC32C = @CFLAGS_ARMV8_CRC32C@
+CFLAGS_CRC = @CFLAGS_CRC@
 PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
 CXXFLAGS = @CXXFLAGS@
 
-- 
2.37.3.542.gdd3f6c4cae


--losgcde4yzp6hbli
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v17-0009-autoconf-Rely-on-ar-supporting-index-creation.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH v3 1/4] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
@ 2022-10-05 19:24 Andres Freund <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Andres Freund @ 2022-10-05 19:24 UTC (permalink / raw)

Until now we emitted the cflags to build the CRC objects into architecture
specific variables. That doesn't make a whole lot of sense to me - we're never
going to target x86 and arm at the same time, so they don't need to be
separate variables.

It might be better to instead continue to have CFLAGS_SSE42 /
CFLAGS_ARMV8_CRC32C be computed by PGAC_ARMV8_CRC32C_INTRINSICS /
PGAC_SSE42_CRC32_INTRINSICS and then set CFLAGS_CRC based on those. But it
seems unlikely that we'd need other sets of CRC specific flags for those two
architectures at the same time.

Discussion: https://postgr.es/m/[email protected]
---
 src/port/Makefile      | 16 ++++++++--------
 config/c-compiler.m4   |  8 ++++----
 configure              | 19 +++++++++----------
 configure.ac           | 10 +++++-----
 src/Makefile.global.in |  3 +--
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/src/port/Makefile b/src/port/Makefile
index b3754d8940a..711f59e32bd 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -88,15 +88,15 @@ libpgport.a: $(OBJS)
 thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
 
-# all versions of pg_crc32c_sse42.o need CFLAGS_SSE42
-pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_SSE42)
+# all versions of pg_crc32c_sse42.o need CFLAGS_CRC
+pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
-# all versions of pg_crc32c_armv8.o need CFLAGS_ARMV8_CRC32C
-pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
+# all versions of pg_crc32c_armv8.o need CFLAGS_CRC
+pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
 #
 # Shared library versions of object files
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 000b075312e..eb8cc8ce170 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -597,7 +597,7 @@ fi])# PGAC_HAVE_GCC__ATOMIC_INT64_CAS
 # the other ones are, on x86-64 platforms)
 #
 # An optional compiler flag can be passed as argument (e.g. -msse4.2). If the
-# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_SSE42.
+# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_SSE42_CRC32_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_sse42_crc32_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=$1], [Ac_cachevar],
@@ -613,7 +613,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <nmmintrin.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_SSE42="$1"
+  CFLAGS_CRC="$1"
   pgac_sse42_crc32_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
@@ -629,7 +629,7 @@ undefine([Ac_cachevar])dnl
 #
 # An optional compiler flag can be passed as argument (e.g.
 # -march=armv8-a+crc). If the intrinsics are supported, sets
-# pgac_armv8_crc32c_intrinsics, and CFLAGS_ARMV8_CRC32C.
+# pgac_armv8_crc32c_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_ARMV8_CRC32C_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_armv8_crc32c_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=$1], [Ac_cachevar],
@@ -647,7 +647,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <arm_acle.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="$1"
+  CFLAGS_CRC="$1"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
diff --git a/configure b/configure
index e04ee9fb416..e65ac574fb5 100755
--- a/configure
+++ b/configure
@@ -645,8 +645,7 @@ MSGMERGE
 MSGFMT_FLAGS
 MSGFMT
 PG_CRC32C_OBJS
-CFLAGS_ARMV8_CRC32C
-CFLAGS_SSE42
+CFLAGS_CRC
 LIBOBJS
 ZSTD
 LZ4
@@ -17813,7 +17812,7 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=" >&5
 $as_echo_n "checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=... " >&6; }
 if ${pgac_cv_sse42_crc32_intrinsics_+:} false; then :
@@ -17848,7 +17847,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics_" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics_" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics_" = x"yes"; then
-  CFLAGS_SSE42=""
+  CFLAGS_CRC=""
   pgac_sse42_crc32_intrinsics=yes
 fi
 
@@ -17887,13 +17886,12 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics__msse4_2" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics__msse4_2" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics__msse4_2" = x"yes"; then
-  CFLAGS_SSE42="-msse4.2"
+  CFLAGS_CRC="-msse4.2"
   pgac_sse42_crc32_intrinsics=yes
 fi
 
 fi
 
-
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -17920,7 +17918,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=" >&5
 $as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... " >&6; }
 if ${pgac_cv_armv8_crc32c_intrinsics_+:} false; then :
@@ -17957,7 +17955,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics_" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics_" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics_" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C=""
+  CFLAGS_CRC=""
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
@@ -17998,13 +17996,14 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="-march=armv8-a+crc"
+  CFLAGS_CRC="-march=armv8-a+crc"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
 fi
 
 
+
 # Select CRC-32C implementation.
 #
 # If we are targeting a processor that has Intel SSE 4.2 instructions, we can
@@ -18032,7 +18031,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/configure.ac b/configure.ac
index f146c8301ae..219e85441d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2087,12 +2087,11 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 PGAC_SSE42_CRC32_INTRINSICS([])
 if test x"$pgac_sse42_crc32_intrinsics" != x"yes"; then
   PGAC_SSE42_CRC32_INTRINSICS([-msse4.2])
 fi
-AC_SUBST(CFLAGS_SSE42)
 
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
@@ -2106,12 +2105,13 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 PGAC_ARMV8_CRC32C_INTRINSICS([])
 if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
   PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc])
 fi
-AC_SUBST(CFLAGS_ARMV8_CRC32C)
+
+AC_SUBST(CFLAGS_CRC)
 
 # Select CRC-32C implementation.
 #
@@ -2140,7 +2140,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 99889167e18..a5e2f36d217 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -262,8 +262,7 @@ CFLAGS_SL_MODULE = @CFLAGS_SL_MODULE@
 CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@
 CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@
 CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@
-CFLAGS_SSE42 = @CFLAGS_SSE42@
-CFLAGS_ARMV8_CRC32C = @CFLAGS_ARMV8_CRC32C@
+CFLAGS_CRC = @CFLAGS_CRC@
 PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
 CXXFLAGS = @CXXFLAGS@
 
-- 
2.38.0


--72v5daafgeyrdmny
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0002-autoconf-Don-t-AC_SUBST-LD-in-configure.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH v4 1/4] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
@ 2022-10-05 19:24 Andres Freund <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Andres Freund @ 2022-10-05 19:24 UTC (permalink / raw)

Until now we emitted the cflags to build the CRC objects into architecture
specific variables. That doesn't make a whole lot of sense to me - we're never
going to target x86 and arm at the same time, so they don't need to be
separate variables.

It might be better to instead continue to have CFLAGS_SSE42 /
CFLAGS_ARMV8_CRC32C be computed by PGAC_ARMV8_CRC32C_INTRINSICS /
PGAC_SSE42_CRC32_INTRINSICS and then set CFLAGS_CRC based on those. But it
seems unlikely that we'd need other sets of CRC specific flags for those two
architectures at the same time.

This simplifies the upcoming meson PGXS compatibility.

Discussion: https://postgr.es/m/[email protected]
---
 configure.ac           | 10 +++++-----
 config/c-compiler.m4   |  8 ++++----
 src/port/Makefile      | 16 ++++++++--------
 configure              | 19 +++++++++----------
 src/Makefile.global.in |  3 +--
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/configure.ac b/configure.ac
index f76b7ee31fc..6e7c8e09411 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2091,12 +2091,11 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 PGAC_SSE42_CRC32_INTRINSICS([])
 if test x"$pgac_sse42_crc32_intrinsics" != x"yes"; then
   PGAC_SSE42_CRC32_INTRINSICS([-msse4.2])
 fi
-AC_SUBST(CFLAGS_SSE42)
 
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
@@ -2110,12 +2109,13 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 PGAC_ARMV8_CRC32C_INTRINSICS([])
 if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
   PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc])
 fi
-AC_SUBST(CFLAGS_ARMV8_CRC32C)
+
+AC_SUBST(CFLAGS_CRC)
 
 # Select CRC-32C implementation.
 #
@@ -2144,7 +2144,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 000b075312e..eb8cc8ce170 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -597,7 +597,7 @@ fi])# PGAC_HAVE_GCC__ATOMIC_INT64_CAS
 # the other ones are, on x86-64 platforms)
 #
 # An optional compiler flag can be passed as argument (e.g. -msse4.2). If the
-# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_SSE42.
+# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_SSE42_CRC32_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_sse42_crc32_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=$1], [Ac_cachevar],
@@ -613,7 +613,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <nmmintrin.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_SSE42="$1"
+  CFLAGS_CRC="$1"
   pgac_sse42_crc32_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
@@ -629,7 +629,7 @@ undefine([Ac_cachevar])dnl
 #
 # An optional compiler flag can be passed as argument (e.g.
 # -march=armv8-a+crc). If the intrinsics are supported, sets
-# pgac_armv8_crc32c_intrinsics, and CFLAGS_ARMV8_CRC32C.
+# pgac_armv8_crc32c_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_ARMV8_CRC32C_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_armv8_crc32c_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=$1], [Ac_cachevar],
@@ -647,7 +647,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <arm_acle.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="$1"
+  CFLAGS_CRC="$1"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
diff --git a/src/port/Makefile b/src/port/Makefile
index b3754d8940a..711f59e32bd 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -88,15 +88,15 @@ libpgport.a: $(OBJS)
 thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
 
-# all versions of pg_crc32c_sse42.o need CFLAGS_SSE42
-pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_SSE42)
+# all versions of pg_crc32c_sse42.o need CFLAGS_CRC
+pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
-# all versions of pg_crc32c_armv8.o need CFLAGS_ARMV8_CRC32C
-pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
+# all versions of pg_crc32c_armv8.o need CFLAGS_CRC
+pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
 #
 # Shared library versions of object files
diff --git a/configure b/configure
index 3966368b8d9..f62fbc5d0f5 100755
--- a/configure
+++ b/configure
@@ -645,8 +645,7 @@ MSGMERGE
 MSGFMT_FLAGS
 MSGFMT
 PG_CRC32C_OBJS
-CFLAGS_ARMV8_CRC32C
-CFLAGS_SSE42
+CFLAGS_CRC
 LIBOBJS
 OPENSSL
 ZSTD
@@ -17957,7 +17956,7 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=" >&5
 $as_echo_n "checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=... " >&6; }
 if ${pgac_cv_sse42_crc32_intrinsics_+:} false; then :
@@ -17992,7 +17991,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics_" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics_" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics_" = x"yes"; then
-  CFLAGS_SSE42=""
+  CFLAGS_CRC=""
   pgac_sse42_crc32_intrinsics=yes
 fi
 
@@ -18031,13 +18030,12 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics__msse4_2" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics__msse4_2" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics__msse4_2" = x"yes"; then
-  CFLAGS_SSE42="-msse4.2"
+  CFLAGS_CRC="-msse4.2"
   pgac_sse42_crc32_intrinsics=yes
 fi
 
 fi
 
-
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -18064,7 +18062,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=" >&5
 $as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... " >&6; }
 if ${pgac_cv_armv8_crc32c_intrinsics_+:} false; then :
@@ -18101,7 +18099,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics_" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics_" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics_" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C=""
+  CFLAGS_CRC=""
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
@@ -18142,13 +18140,14 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="-march=armv8-a+crc"
+  CFLAGS_CRC="-march=armv8-a+crc"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
 fi
 
 
+
 # Select CRC-32C implementation.
 #
 # If we are targeting a processor that has Intel SSE 4.2 instructions, we can
@@ -18176,7 +18175,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 84e72e99278..346b73260fc 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -262,8 +262,7 @@ CFLAGS_SL_MODULE = @CFLAGS_SL_MODULE@
 CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@
 CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@
 CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@
-CFLAGS_SSE42 = @CFLAGS_SSE42@
-CFLAGS_ARMV8_CRC32C = @CFLAGS_ARMV8_CRC32C@
+CFLAGS_CRC = @CFLAGS_CRC@
 PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
 CXXFLAGS = @CXXFLAGS@
 
-- 
2.38.0


--qh3eagtoyzwfapdm
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0002-autoconf-Don-t-AC_SUBST-LD-in-configure.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH v2 1/5] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C
@ 2022-10-05 19:24 Andres Freund <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Andres Freund @ 2022-10-05 19:24 UTC (permalink / raw)

Until now we emit the cflags to build the CRC objects into architecture
specific variables. That doesn't make a whole lot of sense to me - we're
never going to target x86 and arm at the same time, so they don't need to be
separate variables.

It might be better to instead continue to have CFLAGS_SSE42 /
CFLAGS_ARMV8_CRC32C be computed by PGAC_ARMV8_CRC32C_INTRINSICS /
PGAC_SSE42_CRC32_INTRINSICS and then set CFLAGS_CRC based on those. But it
seems unlikely that we'd need other sets of flags for those two architectures
at the same time.
---
 src/port/Makefile      | 16 ++++++++--------
 config/c-compiler.m4   |  8 ++++----
 configure              | 19 +++++++++----------
 configure.ac           | 10 +++++-----
 src/Makefile.global.in |  3 +--
 5 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/src/port/Makefile b/src/port/Makefile
index b3754d8940a..711f59e32bd 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -88,15 +88,15 @@ libpgport.a: $(OBJS)
 thread.o: CFLAGS+=$(PTHREAD_CFLAGS)
 thread_shlib.o: CFLAGS+=$(PTHREAD_CFLAGS)
 
-# all versions of pg_crc32c_sse42.o need CFLAGS_SSE42
-pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_SSE42)
-pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_SSE42)
+# all versions of pg_crc32c_sse42.o need CFLAGS_CRC
+pg_crc32c_sse42.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_sse42_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
-# all versions of pg_crc32c_armv8.o need CFLAGS_ARMV8_CRC32C
-pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
-pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_ARMV8_CRC32C)
+# all versions of pg_crc32c_armv8.o need CFLAGS_CRC
+pg_crc32c_armv8.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_shlib.o: CFLAGS+=$(CFLAGS_CRC)
+pg_crc32c_armv8_srv.o: CFLAGS+=$(CFLAGS_CRC)
 
 #
 # Shared library versions of object files
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 000b075312e..eb8cc8ce170 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -597,7 +597,7 @@ fi])# PGAC_HAVE_GCC__ATOMIC_INT64_CAS
 # the other ones are, on x86-64 platforms)
 #
 # An optional compiler flag can be passed as argument (e.g. -msse4.2). If the
-# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_SSE42.
+# intrinsics are supported, sets pgac_sse42_crc32_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_SSE42_CRC32_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_sse42_crc32_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=$1], [Ac_cachevar],
@@ -613,7 +613,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <nmmintrin.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_SSE42="$1"
+  CFLAGS_CRC="$1"
   pgac_sse42_crc32_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
@@ -629,7 +629,7 @@ undefine([Ac_cachevar])dnl
 #
 # An optional compiler flag can be passed as argument (e.g.
 # -march=armv8-a+crc). If the intrinsics are supported, sets
-# pgac_armv8_crc32c_intrinsics, and CFLAGS_ARMV8_CRC32C.
+# pgac_armv8_crc32c_intrinsics, and CFLAGS_CRC.
 AC_DEFUN([PGAC_ARMV8_CRC32C_INTRINSICS],
 [define([Ac_cachevar], [AS_TR_SH([pgac_cv_armv8_crc32c_intrinsics_$1])])dnl
 AC_CACHE_CHECK([for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=$1], [Ac_cachevar],
@@ -647,7 +647,7 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <arm_acle.h>],
   [Ac_cachevar=no])
 CFLAGS="$pgac_save_CFLAGS"])
 if test x"$Ac_cachevar" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="$1"
+  CFLAGS_CRC="$1"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 undefine([Ac_cachevar])dnl
diff --git a/configure b/configure
index 1caca21b625..80b28cb9310 100755
--- a/configure
+++ b/configure
@@ -645,8 +645,7 @@ MSGMERGE
 MSGFMT_FLAGS
 MSGFMT
 PG_CRC32C_OBJS
-CFLAGS_ARMV8_CRC32C
-CFLAGS_SSE42
+CFLAGS_CRC
 LIBOBJS
 ZSTD
 LZ4
@@ -17815,7 +17814,7 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=" >&5
 $as_echo_n "checking for _mm_crc32_u8 and _mm_crc32_u32 with CFLAGS=... " >&6; }
 if ${pgac_cv_sse42_crc32_intrinsics_+:} false; then :
@@ -17850,7 +17849,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics_" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics_" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics_" = x"yes"; then
-  CFLAGS_SSE42=""
+  CFLAGS_CRC=""
   pgac_sse42_crc32_intrinsics=yes
 fi
 
@@ -17889,13 +17888,12 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_sse42_crc32_intrinsics__msse4_2" >&5
 $as_echo "$pgac_cv_sse42_crc32_intrinsics__msse4_2" >&6; }
 if test x"$pgac_cv_sse42_crc32_intrinsics__msse4_2" = x"yes"; then
-  CFLAGS_SSE42="-msse4.2"
+  CFLAGS_CRC="-msse4.2"
   pgac_sse42_crc32_intrinsics=yes
 fi
 
 fi
 
-
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
@@ -17922,7 +17920,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=" >&5
 $as_echo_n "checking for __crc32cb, __crc32ch, __crc32cw, and __crc32cd with CFLAGS=... " >&6; }
 if ${pgac_cv_armv8_crc32c_intrinsics_+:} false; then :
@@ -17959,7 +17957,7 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics_" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics_" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics_" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C=""
+  CFLAGS_CRC=""
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
@@ -18000,13 +17998,14 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&5
 $as_echo "$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" >&6; }
 if test x"$pgac_cv_armv8_crc32c_intrinsics__march_armv8_apcrc" = x"yes"; then
-  CFLAGS_ARMV8_CRC32C="-march=armv8-a+crc"
+  CFLAGS_CRC="-march=armv8-a+crc"
   pgac_armv8_crc32c_intrinsics=yes
 fi
 
 fi
 
 
+
 # Select CRC-32C implementation.
 #
 # If we are targeting a processor that has Intel SSE 4.2 instructions, we can
@@ -18034,7 +18033,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/configure.ac b/configure.ac
index 10fa55dd154..f837a602d01 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2086,12 +2086,11 @@ fi
 #
 # First check if the _mm_crc32_u8 and _mm_crc32_u64 intrinsics can be used
 # with the default compiler flags. If not, check if adding the -msse4.2
-# flag helps. CFLAGS_SSE42 is set to -msse4.2 if that's required.
+# flag helps. CFLAGS_CRC is set to -msse4.2 if that's required.
 PGAC_SSE42_CRC32_INTRINSICS([])
 if test x"$pgac_sse42_crc32_intrinsics" != x"yes"; then
   PGAC_SSE42_CRC32_INTRINSICS([-msse4.2])
 fi
-AC_SUBST(CFLAGS_SSE42)
 
 # Are we targeting a processor that supports SSE 4.2? gcc, clang and icc all
 # define __SSE4_2__ in that case.
@@ -2105,12 +2104,13 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [
 #
 # First check if __crc32c* intrinsics can be used with the default compiler
 # flags. If not, check if adding -march=armv8-a+crc flag helps.
-# CFLAGS_ARMV8_CRC32C is set if the extra flag is required.
+# CFLAGS_CRC is set if the extra flag is required.
 PGAC_ARMV8_CRC32C_INTRINSICS([])
 if test x"$pgac_armv8_crc32c_intrinsics" != x"yes"; then
   PGAC_ARMV8_CRC32C_INTRINSICS([-march=armv8-a+crc])
 fi
-AC_SUBST(CFLAGS_ARMV8_CRC32C)
+
+AC_SUBST(CFLAGS_CRC)
 
 # Select CRC-32C implementation.
 #
@@ -2139,7 +2139,7 @@ if test x"$USE_SLICING_BY_8_CRC32C" = x"" && test x"$USE_SSE42_CRC32C" = x"" &&
       USE_SSE42_CRC32C_WITH_RUNTIME_CHECK=1
     else
       # Use ARM CRC Extension if available.
-      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_ARMV8_CRC32C" = x""; then
+      if test x"$pgac_armv8_crc32c_intrinsics" = x"yes" && test x"$CFLAGS_CRC" = x""; then
         USE_ARMV8_CRC32C=1
       else
         # ARM CRC Extension, with runtime check?
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index d8ea2da583c..c87b6d97ca9 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -262,8 +262,7 @@ CFLAGS_SL_MODULE = @CFLAGS_SL_MODULE@
 CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@
 CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@
 CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@
-CFLAGS_SSE42 = @CFLAGS_SSE42@
-CFLAGS_ARMV8_CRC32C = @CFLAGS_ARMV8_CRC32C@
+CFLAGS_CRC = @CFLAGS_CRC@
 PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
 CXXFLAGS = @CXXFLAGS@
 
-- 
2.37.3.542.gdd3f6c4cae


--muxyv6qxhuvraoxq
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0002-autoconf-Rely-on-ar-supporting-index-creation.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root
@ 2025-07-08 08:53 Mikhail Kharitonov <[email protected]>
  2025-07-08 09:41 ` Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root Mikhail Kharitonov <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Mikhail Kharitonov @ 2025-07-08 08:53 UTC (permalink / raw)
  To: [email protected]

Hi all,

I’m sending v2 of the patch. This is a clean rebase onto current master
(commit a27893df45e) and a squash of the fix together with the TAP
test into a single patch file.

I would appreciate your thoughts and comments on the current problem.

Thank you!

--
Best regards,
Mikhail Kharitonov

On Thu, May 29, 2025 at 9:30 AM Mikhail Kharitonov
<[email protected]> wrote:
>
> Hi,
>
> Thank you for the feedback.
>
> I would like to clarify that the current behavior does not break replication
> between PostgreSQL instances. The logical replication stream is still accepted
> by the subscriber, and the data is applied correctly. However, the protocol
> semantics are violated, which may cause issues for external systems that rely
> on interpreting this stream.
>
> When using publish_via_partition_root = true and setting REPLICA IDENTITY FULL
> only on the parent table (but not on all partitions), logical replication
> generates messages with the tag 'O' (old tuple) for updates and deletes even
> for partitions that do not have full identity configured.
>
> In those cases, only key columns are sent, and the rest of the tuple is omitted.
>  This contradicts the meaning of tag 'O', which, according
>  to the documentation [1], indicates that the full old tuple is included.
>
> This behavior is safe for the standard PostgreSQL subscriber, which does not
> rely on the tag when applying changes. However, third-party tools that consume
> the logical replication stream and follow the protocol strictly can be misled.
> For example, one of our clients uses a custom CDC mechanism that extracts
> changes and sends them to Oracle. Their handler interprets the 'O' tag as a
> signal that the full old row is available. When it is not - the data is
> processed incorrectly.
>
> The attached patch changes the behavior so that the 'O' or 'K' tag is chosen
> based on the REPLICA IDENTITY setting of the actual partition where the row
> ends up not only the parent.
>     - If the partition has REPLICA IDENTITY FULL, the full tuple is
> sent and tagged 'O'.
>     - Otherwise, only the key columns are sent, and the tag 'K' is used.
>
> This aligns the behavior with the protocol documentation.
> I have also included a TAP test: 036_partition_replica_identity.pl,
> located in src/test/subscription/t/
>
> It demonstrates two cases:
>     - An update/delete on a partition with REPLICA IDENTITY FULL correctly
>     emits an 'O' tag with the full old row.
>     - An update/delete on a partition without REPLICA IDENTITY FULL currently
>     also emits an 'O' tag, but only with key fields - this is the problem.
>
> After applying the patch, the second case correctly uses the 'K' tag.
>
> This patch is a minimal change it does not alter protocol structure
> or introduce new behavior. It only ensures the implementation matches
> the documentation. In the future, we might consider a broader redesign
> of logical replication for partitioned tables (see [2]), but this is
> a narrow fix that solves a real inconsistency.
>
> Looking forward to your comments.
>
> Best regards,
> Mikhail Kharitonov
>
> [1] https://www.postgresql.org/docs/current/protocol-logicalrep-message-formats.html
> [2] https://www.postgresql.org/message-id/[email protected]
>
> On Mon, May 12, 2025 at 5:25 PM Maxim Orlov <[email protected]> wrote:
> >
> > Hi!
> >
> > This is probably not the most familiar part of Postgres to me, but does it break anything? Or is it just inconsistency in the replication protocol?
> >
> > A test for the described scenario would be a great addition. And, if it is feasible, provide an example of what would be broken with the way partitioned tables are replicated now.
> >
> > There is a chance that the replication protocol for partitioned tables needs to be rewritten, and I sincerely hope that I am wrong about this. It seems Alvaro Herrera tried this here [0].
> >
> >
> > [0] https://www.postgresql.org/message-id/[email protected]
> >
> >
> > --
> > Best regards,
> > Maxim Orlov.





^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root
  2025-07-08 08:53 Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root Mikhail Kharitonov <[email protected]>
@ 2025-07-08 09:41 ` Mikhail Kharitonov <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Mikhail Kharitonov @ 2025-07-08 09:41 UTC (permalink / raw)
  To: [email protected]

On Tue, Jul 8, 2025 at 11:53 AM Mikhail Kharitonov
<[email protected]> wrote:
>
> Hi all,
>
> I’m sending v2 of the patch. This is a clean rebase onto current master
> (commit a27893df45e) and a squash of the fix together with the TAP
> test into a single patch file.
>
> I would appreciate your thoughts and comments on the current problem.
>
> Thank you!
>
> --
> Best regards,
> Mikhail Kharitonov
>

Sorry, I forgot the attachment in my previous message.
Please find the v2 patch attached.


Attachments:

  [application/octet-stream] 0002-replica_identity.patch (11.5K, ../../CAKkoVausL8emdjEiDD_5p918HaZhB2+2RycV4eD0FOsmx5X5Dg@mail.gmail.com/2-0002-replica_identity.patch)
  download | inline diff:
From 2ea131eb052c0e4c1c78f011d156e795b5640a58 Mon Sep 17 00:00:00 2001
From: Mikhail Kharitonov <[email protected]>
Date: Tue, 8 Jul 2025 09:27:56 +0300
Subject: [PATCH v2] Logical replication: fix replica identity flag for
 partitioned tables

With publish_via_partition_root = true, UPDATE and DELETE rows that land in a
partition without REPLICA IDENTITY FULL were sent with tuple flag 'O' (old
tuple) even though only primary-key columns were included.  This violates the
logical-replication protocol and can confuse external CDC tools that rely
on the flag.

The flag is now chosen according to the replica identity of the partition
that actually stores the row: partitions with REPLICA IDENTITY FULL still use
'O', all others use 'K'.

A TAP test (src/test/subscription/t/036_partition_replica_identity.pl)
covers both cases.

Author: Mikhail Kharitonov <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAKkoVatDVuH%3DAcnecrZSOQ0_Md6RfW4ExvCKJzJJ4JTrX1wbxQ%40mail.gmail.com
---
 src/backend/replication/logical/proto.c       |  32 +++--
 src/backend/replication/pgoutput/pgoutput.c   |   4 +-
 src/include/replication/logicalproto.h        |   6 +-
 .../t/036_partition_replica_identity.pl       | 135 ++++++++++++++++++
 4 files changed, 158 insertions(+), 19 deletions(-)
 create mode 100755 src/test/subscription/t/036_partition_replica_identity.pl

diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index 1a352b542dc..935a39faf20 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -447,36 +447,37 @@ logicalrep_read_insert(StringInfo in, LogicalRepTupleData *newtup)
  * Write UPDATE to the output stream.
  */
 void
-logicalrep_write_update(StringInfo out, TransactionId xid, Relation rel,
+logicalrep_write_update(StringInfo out, TransactionId xid, 
+						Relation real_relation, Relation send_as,
 						TupleTableSlot *oldslot, TupleTableSlot *newslot,
 						bool binary, Bitmapset *columns,
 						PublishGencolsType include_gencols_type)
 {
 	pq_sendbyte(out, LOGICAL_REP_MSG_UPDATE);
 
-	Assert(rel->rd_rel->relreplident == REPLICA_IDENTITY_DEFAULT ||
-		   rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
-		   rel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);
+	Assert(real_relation->rd_rel->relreplident == REPLICA_IDENTITY_DEFAULT ||
+		   real_relation->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
+		   real_relation->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);
 
 	/* transaction ID (if not valid, we're not streaming) */
 	if (TransactionIdIsValid(xid))
 		pq_sendint32(out, xid);
 
 	/* use Oid as relation identifier */
-	pq_sendint32(out, RelationGetRelid(rel));
+	pq_sendint32(out, RelationGetRelid(send_as));
 
 	if (oldslot != NULL)
 	{
-		if (rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL)
+		if (real_relation->rd_rel->relreplident == REPLICA_IDENTITY_FULL)
 			pq_sendbyte(out, 'O');	/* old tuple follows */
 		else
 			pq_sendbyte(out, 'K');	/* old key follows */
-		logicalrep_write_tuple(out, rel, oldslot, binary, columns,
+		logicalrep_write_tuple(out, send_as, oldslot, binary, columns,
 							   include_gencols_type);
 	}
 
 	pq_sendbyte(out, 'N');		/* new tuple follows */
-	logicalrep_write_tuple(out, rel, newslot, binary, columns,
+	logicalrep_write_tuple(out, send_as, newslot, binary, columns,
 						   include_gencols_type);
 }
 
@@ -525,14 +526,15 @@ logicalrep_read_update(StringInfo in, bool *has_oldtuple,
  * Write DELETE to the output stream.
  */
 void
-logicalrep_write_delete(StringInfo out, TransactionId xid, Relation rel,
+logicalrep_write_delete(StringInfo out, TransactionId xid,
+						Relation real_relation, Relation send_as,
 						TupleTableSlot *oldslot, bool binary,
 						Bitmapset *columns,
 						PublishGencolsType include_gencols_type)
 {
-	Assert(rel->rd_rel->relreplident == REPLICA_IDENTITY_DEFAULT ||
-		   rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
-		   rel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);
+	Assert(real_relation->rd_rel->relreplident == REPLICA_IDENTITY_DEFAULT ||
+		   real_relation->rd_rel->relreplident == REPLICA_IDENTITY_FULL ||
+		   real_relation->rd_rel->relreplident == REPLICA_IDENTITY_INDEX);
 
 	pq_sendbyte(out, LOGICAL_REP_MSG_DELETE);
 
@@ -541,14 +543,14 @@ logicalrep_write_delete(StringInfo out, TransactionId xid, Relation rel,
 		pq_sendint32(out, xid);
 
 	/* use Oid as relation identifier */
-	pq_sendint32(out, RelationGetRelid(rel));
+	pq_sendint32(out, RelationGetRelid(send_as));
 
-	if (rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL)
+	if (real_relation->rd_rel->relreplident == REPLICA_IDENTITY_FULL)
 		pq_sendbyte(out, 'O');	/* old tuple follows */
 	else
 		pq_sendbyte(out, 'K');	/* old key follows */
 
-	logicalrep_write_tuple(out, rel, oldslot, binary, columns,
+	logicalrep_write_tuple(out, send_as, oldslot, binary, columns,
 						   include_gencols_type);
 }
 
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 082b4d9d327..261be60b8f4 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1587,12 +1587,12 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
 									relentry->include_gencols_type);
 			break;
 		case REORDER_BUFFER_CHANGE_UPDATE:
-			logicalrep_write_update(ctx->out, xid, targetrel, old_slot,
+			logicalrep_write_update(ctx->out, xid, relation, targetrel, old_slot,
 									new_slot, data->binary, relentry->columns,
 									relentry->include_gencols_type);
 			break;
 		case REORDER_BUFFER_CHANGE_DELETE:
-			logicalrep_write_delete(ctx->out, xid, targetrel, old_slot,
+			logicalrep_write_delete(ctx->out, xid, relation, targetrel, old_slot,
 									data->binary, relentry->columns,
 									relentry->include_gencols_type);
 			break;
diff --git a/src/include/replication/logicalproto.h b/src/include/replication/logicalproto.h
index b261c60d3fa..8940c31b4f1 100644
--- a/src/include/replication/logicalproto.h
+++ b/src/include/replication/logicalproto.h
@@ -228,7 +228,8 @@ extern void logicalrep_write_insert(StringInfo out, TransactionId xid,
 									PublishGencolsType include_gencols_type);
 extern LogicalRepRelId logicalrep_read_insert(StringInfo in, LogicalRepTupleData *newtup);
 extern void logicalrep_write_update(StringInfo out, TransactionId xid,
-									Relation rel, TupleTableSlot *oldslot,
+									Relation real_relation, Relation send_as,
+									TupleTableSlot *oldslot,
 									TupleTableSlot *newslot, bool binary,
 									Bitmapset *columns,
 									PublishGencolsType include_gencols_type);
@@ -236,7 +237,8 @@ extern LogicalRepRelId logicalrep_read_update(StringInfo in,
 											  bool *has_oldtuple, LogicalRepTupleData *oldtup,
 											  LogicalRepTupleData *newtup);
 extern void logicalrep_write_delete(StringInfo out, TransactionId xid,
-									Relation rel, TupleTableSlot *oldslot,
+									Relation real_relation, Relation send_as,
+									TupleTableSlot *oldslot,
 									bool binary, Bitmapset *columns,
 									PublishGencolsType include_gencols_type);
 extern LogicalRepRelId logicalrep_read_delete(StringInfo in,
diff --git a/src/test/subscription/t/036_partition_replica_identity.pl b/src/test/subscription/t/036_partition_replica_identity.pl
new file mode 100755
index 00000000000..f69bca5ebe2
--- /dev/null
+++ b/src/test/subscription/t/036_partition_replica_identity.pl
@@ -0,0 +1,135 @@
+# Test logical replication with publish_via_partition_root,
+# where the parent has REPLICA IDENTITY FULL, but one partition does not.
+#
+# Expected behavior:
+# - For partitions with REPLICA IDENTITY FULL, old tuple must be marked as 'O' and contain full row.
+# - For partitions with REPLICA IDENTITY DEFAULT, old tuple should be marked as 'K' and contain only key columns.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+sub log_state
+{
+  my ($node, $label) = @_;
+
+  my $rows = $node->safe_psql('postgres', q{
+    SELECT tableoid::regclass AS part,
+           id,
+           to_char(ts,'YYYY-MM-DD') AS ts,
+           load
+    FROM   part_table
+    ORDER  BY 1,2;
+  });
+  diag "----- $label: rows -----\n$rows\n";
+}
+
+my $pub = PostgreSQL::Test::Cluster->new('publisher');
+$pub->init(allows_streaming => 'logical');
+$pub->start;
+
+my $sub = PostgreSQL::Test::Cluster->new('subscriber');
+$sub->init;
+$sub->start;
+
+$pub->safe_psql('postgres', q{
+create table part_table(
+  id  int generated always as identity,
+  ts  timestamp,
+  load text,
+  constraint part_table_pk primary key(id, ts)
+) partition by range(ts);
+
+create table part_table_sect_1 partition of part_table
+  for values from ('2000-01-01') to ('2024-01-01');
+create table part_table_sect_2 partition of part_table
+  for values from ('2024-01-01') to (maxvalue);
+
+alter table part_table        replica identity full;
+alter table part_table_sect_1 replica identity full;
+
+create publication pub_part_table
+  for table part_table
+  with (publish_via_partition_root = true);
+});
+
+$pub->safe_psql('postgres',
+  q{select pg_create_logical_replication_slot('slot_test', 'pgoutput');});
+
+$sub->safe_psql('postgres', q{
+create table part_table(
+  id  int,
+  ts  timestamp,
+  load text,
+  constraint part_table_pk primary key(id, ts)
+) partition by range(ts);
+
+create table part_table_sect_1 partition of part_table
+  for values from ('2000-01-01') to ('2024-01-01');
+create table part_table_sect_2 partition of part_table
+  for values from ('2024-01-01') to (maxvalue);
+});
+
+my $connstr = $pub->connstr . ' dbname=postgres';
+$sub->safe_psql('postgres', qq{
+create subscription sub_part
+  connection '$connstr application_name=sub_part'
+  publication pub_part_table;});
+
+$sub->wait_for_subscription_sync($pub, 'sub_part');
+
+$pub->safe_psql('postgres', q{
+insert into part_table values (default, '2020-01-01 00:00', 'first');
+insert into part_table values (default, '2025-01-01 00:00', 'second');
+});
+$sub->wait_for_subscription_sync($pub, 'sub_part');
+diag("\n");
+log_state($pub, 'publisher after insert');
+log_state($sub, 'subscriber after insert');
+
+$pub->safe_psql('postgres',
+  q{update part_table set ts = ts + interval '1 day';});
+$sub->wait_for_subscription_sync($pub, 'sub_part');
+
+log_state($pub, 'publisher after update');
+log_state($sub, 'subscriber after update');
+
+$pub->safe_psql('postgres', q{delete from part_table;});
+$sub->wait_for_subscription_sync($pub, 'sub_part');
+
+log_state($pub, 'publisher after delete');
+log_state($sub, 'subscriber after delete');
+
+my $wal = $pub->safe_psql('postgres', q{
+select string_agg(encode(data, 'escape'),'')
+  from pg_logical_slot_get_binary_changes(
+         'slot_test', null, null,
+         'proto_version','1',
+         'publication_names','pub_part_table');
+});
+
+diag("---- WAL stream ----\n$wal\n");
+
+# 1: first partition has REPLICA IDENTITY FULL - full old tuple
+# (see - https://www.postgresql.org/docs/current/protocol-logicalrep-message-formats.html#PROTOCOL-LOGICALREP-MESSAGE-FORMATS-UPDATE)
+like(
+  $wal,
+  qr/U.*O.*first.*first/s,
+  'partition WITH REPLICA IDENTITY FULL contains full old tuple'
+);
+
+# 2: second partition has REPLICA IDENTITY DEFAULT - only keys expected.
+if ($wal =~ /U.*K.*second/s)
+{
+    pass("Tag K correctly used for partition with REPLICA IDENTITY DEFAULT");
+}
+elsif ($wal =~ /(U.*O.*second)/s)
+{
+    my $blk = $1;
+    my $count = () = $blk =~ /second/g;
+    is($count, 2, "Tag O used but this partition with REPLICA IDENTITY DEFAULT");
+}
+
+done_testing();
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2025-07-08 09:41 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 21:05 [PATCH v17 08/23] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C Andres Freund <[email protected]>
2022-10-05 19:24 [PATCH v3 1/4] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C Andres Freund <[email protected]>
2022-10-05 19:24 [PATCH v4 1/4] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C Andres Freund <[email protected]>
2022-10-05 19:24 [PATCH v2 1/5] autoconf: Unify CFLAGS_SSE42 and CFLAGS_ARMV8_CRC32C Andres Freund <[email protected]>
2025-07-08 08:53 Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root Mikhail Kharitonov <[email protected]>
2025-07-08 09:41 ` Re: [PATCH] Fix replica identity mismatch for partitioned tables with publish_via_partition_root Mikhail Kharitonov <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox