Message-ID: <fabc53d3-aee9-4474-921a-cac66dc2e6db@iki.fi>
Date: Wed, 28 Feb 2024 15:25:19 +0400
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Relation bulk write facility
Content-Language: en-US
To: Andres Freund <andres@anarazel.de>, Thomas Munro
 <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas
 <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Peter Smith <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
 Melanie Plageman <melanieplageman@gmail.com>
References: <20240224212936.f3zcbs24f7oaapm4@alap3.anarazel.de>
 <0C469A7A-548C-4B0C-9495-730FE80046E1@iki.fi>
 <CA+hUKGLAr9Rki58cNCdv_ivR5=Uyps5v-e-bEhHUK3gJTT7wtQ@mail.gmail.com>
 <CA+hUKGLuG2s4-+4DH+JDJh0CiSGU8SbGfK1ZXYWkMQS7iHdGxw@mail.gmail.com>
 <31db8d41-2ac9-4b5e-b7a8-712d6d7d7aa2@iki.fi>
 <20240225194322.a5@rfd.leadboat.com> <1535845.1708890691@sss.pgh.pa.us>
 <CA+Tgmoaoi6BioWTndnkRmMVxQ4QL=xF7Z_PC_8zwpreNshfnJQ@mail.gmail.com>
 <ZdwRK-bqyRt4khc6@paquier.xyz> <3a0d22fa-7613-4881-aa19-ddd31fb92cc0@iki.fi>
 <20240228092642.i23w4qzdsuy6da6y@alap3.anarazel.de>
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <20240228092642.i23w4qzdsuy6da6y@alap3.anarazel.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/fabc53d3-aee9-4474-921a-cac66dc2e6db%40iki.fi>
Precedence: bulk

Committed, after fixing the various little things you pointed out:

On 28/02/2024 00:30, Thomas Munro wrote:
> --- a/doc/src/sgml/installation.sgml
> +++ b/doc/src/sgml/installation.sgml
> @@ -3401,7 +3401,7 @@ export MANPATH
>     <para>
>      <productname>PostgreSQL</productname> can be expected to work on current
>      versions of these operating systems: Linux, Windows,
> -   FreeBSD, OpenBSD, NetBSD, DragonFlyBSD, macOS, AIX, Solaris, and illumos.
> +   FreeBSD, OpenBSD, NetBSD, DragonFlyBSD, macOS, Solaris, and illumos.
> 
> There is also a little roll-of-honour of operating systems we used to
> support, just a couple of paragraphs down, where AIX should appear.

Added.

On 28/02/2024 05:52, Noah Misch wrote:
> Regardless of how someone were to step up to maintain it, we'd be telling them
> such contributions have negative value and must stop.  We're expelling AIX due
> to low demand, compiler bugs, its ABI, and its shlib symbol export needs.

Reworded.

>> Apperently the "pg_attribute_aligned(a)" attribute doesn't work on AIX
>> (linker?) for values larger than PG_IO_ALIGN_SIZE.
> 
> No; see https://postgr.es/m/20240225194322.a5%40rfd.leadboat.com

Ok, reworded.

On 28/02/2024 11:26, Andres Freund wrote:
> On 2024-02-28 00:24:01 +0400, Heikki Linnakangas wrote:
> 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.

Fixed, thanks!

>> 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?

Ok, done

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

Done.

-- 
Heikki Linnakangas
Neon (https://neon.tech)