Re: Relation bulk write facility

Hi,

On 2024-02-28 00:24:01 +0400, Heikki Linnakangas wrote:
> Here's a patch to fully remove AIX support.

Thomas mentioned to me that cfbot failed with this applied:
https://cirrus-ci.com/task/6348635416297472
https://api.cirrus-ci.com/v1/artifact/task/6348635416297472/log/tmp_install/log/initdb-template.log

initdb: error while loading shared libraries: libpq.so.5: cannot open shared object file: No such file or directory


While I couldn't reproduce the failure, I did notice that locally with the
patch applied, system libpq ended up getting used. Which isn't pre-installed
in the CI environment, explaining the failure.

The problem is due to this hunk:
> @@ -401,10 +376,6 @@ install-lib-static: $(stlib) installdirs-lib
>  
>  install-lib-shared: $(shlib) installdirs-lib
>  ifdef soname
> -# we don't install $(shlib) on AIX
> -# (see http://archives.postgresql.org/message-id/52EF20B2E3209443BC37736D00C3C1380A6E79FE@EXADV1.host.magwien.gv.at)
> -ifneq ($(PORTNAME), aix)
> -	$(INSTALL_SHLIB) $< '$(DESTDIR)$(libdir)/$(shlib)'
>  ifneq ($(PORTNAME), cygwin)
>  ifneq ($(PORTNAME), win32)
>  ifneq ($(shlib), $(shlib_major))

So the versioned name didn't end up getting installed anymore, leading to
broken symlinks in the install directory.



> diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> index 86cc01a640b..fc6b00224f6 100644
> --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
> @@ -400,9 +400,6 @@ is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
>  SKIP:
>  {
>  	my $tar = $ENV{TAR};
> -	# don't check for a working tar here, to accommodate various odd
> -	# cases such as AIX. If tar doesn't work the init_from_backup below
> -	# will fail.
>  	skip "no tar program available", 1
>  	  if (!defined $tar || $tar eq '');

Maybe better to not remove the whole comment, just the reference to AIX?


> diff --git a/src/test/regress/sql/sanity_check.sql b/src/test/regress/sql/sanity_check.sql
> index 7f338d191c6..2e9d5ebef3f 100644
> --- a/src/test/regress/sql/sanity_check.sql
> +++ b/src/test/regress/sql/sanity_check.sql
> @@ -21,12 +21,15 @@ SELECT relname, relkind
>         AND relfilenode <> 0;
>  
>  --
> --- When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
> +-- When MAXIMUM_ALIGNOF==8 but ALIGNOF_DOUBLE==4, the C ABI may impose 8-byte alignment
>  -- some of the C types that correspond to TYPALIGN_DOUBLE SQL types.  To ensure
>  -- catalog C struct layout matches catalog tuple layout, arrange for the tuple
>  -- offset of each fixed-width, attalign='d' catalog column to be divisible by 8
>  -- unconditionally.  Keep such columns before the first NameData column of the
>  -- catalog, since packagers can override NAMEDATALEN to an odd number.
> +-- (XXX: I'm not sure if any of the supported platforms have MAXIMUM_ALIGNOF==8 and
> +-- ALIGNOF_DOUBLE==4.  Perhaps we should just require that
> +-- ALIGNOF_DOUBLE==MAXIMUM_ALIGNOF)
>  --
>  WITH check_columns AS (
>   SELECT relname, attname,

I agree, this should be an error, and we should then remove the test.


Greetings,

Andres Freund