Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vxu79-00HFNj-0K for pgsql-hackers@arkaria.postgresql.org; Wed, 04 Mar 2026 21:50:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vxu75-00EOc5-2i for pgsql-hackers@arkaria.postgresql.org; Wed, 04 Mar 2026 21:50:40 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1vxu75-00EObx-0y for pgsql-hackers@lists.postgresql.org; Wed, 04 Mar 2026 21:50:40 +0000 Received: from mail-qk1-x741.google.com ([2607:f8b0:4864:20::741]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1vxu72-00000000R6J-2Waw for pgsql-hackers@lists.postgresql.org; Wed, 04 Mar 2026 21:50:38 +0000 Received: by mail-qk1-x741.google.com with SMTP id af79cd13be357-8cbb2329e7eso742729585a.0 for ; Wed, 04 Mar 2026 13:50:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dunslane-net.20230601.gappssmtp.com; s=20230601; t=1772661034; x=1773265834; darn=lists.postgresql.org; h=in-reply-to:autocrypt:content-language:from:references:cc:to :subject:user-agent:mime-version:date:message-id:from:to:cc:subject :date:message-id:reply-to; bh=QgR+CXlHx3HMrSPXXyzRydRAlL2bj4jpDOXBGjxRzGc=; b=mX8GraZ3FLJI8GXUQN7p3aOKyrdv4i9IN8iQEuXnjIAJ27gXH5WI5btQXbV8CvgV/8 ENaELZTTbdwgUMyz6qY69tEG+QWuFsc3Kpl/dxVrwFW8akmUgcwSS7MqyTHONLRsgG0e W3cFHP2ddXFGlhmJSGzj8kLrL/SwJr2TTqgu4M6tuVKmXb7Te2aEN3E3k1YJNlYoR8lf uV142HEzI4jkmJZACVxcfXkmjUlMsyHygmK22nHJv0RPbFx0Cn9wBTCm/gbNn5JVHhkn jV0bKp4ICpL9djV2EyhMdXySy74HggTiHMsiTbExvitxqh7JlAEKd+HRiJQ11Qji3qRh zKsg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772661034; x=1773265834; h=in-reply-to:autocrypt:content-language:from:references:cc:to :subject:user-agent:mime-version:date:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QgR+CXlHx3HMrSPXXyzRydRAlL2bj4jpDOXBGjxRzGc=; b=aN7spLl4h4T3up1HurotEh1X6Cp5DLX7MkJ5iLONbUkwnz7VjT70u1bAgJIIIJb1uQ 9w8uqTLSYQSghGgOEiI479RNFLTkfGkZDRc0dP7UyGQ3dKM/gpFhSv5daaVSFV2zGEho rEAkNM/Yki8TNIVk3RYkioAs3oQk6paTgXkmGAyN388JSTh4mRCX0of9yBAgkAaPgZVv 6FNTJxh8h/Iu3ZoHa0fhFQIz8sZn1e3S0BvL7W9mI2c5SjPXMyT4cjwBc0FLVvirFZuU Lh9qTzTd5EmAcsubJ+6wsCV4lei3N0b/5ekDvBCgvmCYBqZEJWmLFLdFKIPsYzmSFevX eauw== X-Forwarded-Encrypted: i=1; AJvYcCUBGLOpcrEuhZw/gIj8qOZXLfWXmCVRfPnMQNZEGUEz9dDn0+lkytMZt8A8a4aWrBmxnmrWuARViYXx2qML@lists.postgresql.org X-Gm-Message-State: AOJu0Ywa/LZLzTKwSjEm0VLGc2PumIQgZVq6k08aV02m4G8t6Jc/kN4Z nkbL5CFoN3FKoZ/9Xd6EnF0qGA2Ljl8rwvhUzJtVKmOXXUIxvoZLpH/tTnp4NbEtJVA= X-Gm-Gg: ATEYQzxv13aDCI7jSpAo7HU/izKJXIFKDUXZUUrsEAqRqxPo4gW5kTqKPg3stwhIdcW +izL5rfIfiSB4vaPJPBACQBxrARCt19cgelMzFQ0qkr+Bjk/yMOLXY0W/xKRBlGkH7LYMvZ9VD4 6bp068UYcGzhO0gjegxFOggfnH6/9wVeaPJn5WFtt+J445ax61FPnSTFZaCtn/SWCe16kATALy4 d/xw2rTZOD6vVQztfr4Nncz+ouMEcqdPxqOVGzCJUwthiN+8mCeArEgXUh+uKGwI22J3dufrYlq ZZJJf7qTydSc6oqMBTv/tEPc30a4IQfzgizVB6Cix6e5kdIw68CM4LS2uFDJNZcKupiX1Yuf4qg wsTmjsK0QO84nIsLRIxMe6V2iQ/7YX5bMzZiH2reavJYvgdoBiZ2+oaKq2QXVJVoI8FHdStUUmO PjIIruo3gAlPO4WnDtgwiuypt3em2NJQ== X-Received: by 2002:a05:620a:3949:b0:8ca:3d7c:e74a with SMTP id af79cd13be357-8cd5afb208cmr392060785a.56.1772661034114; Wed, 04 Mar 2026 13:50:34 -0800 (PST) Received: from ?IPV6:2605:a601:a6b0:500::1cb? ([2605:a601:a6b0:500::1cb]) by smtp.googlemail.com with ESMTPSA id 6a1803df08f44-899f4fb20a3sm103497786d6.31.2026.03.04.13.50.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Mar 2026 13:50:33 -0800 (PST) Content-Type: multipart/alternative; boundary="------------JvWy99edmXlSqtQV26VaPR0z" Message-ID: Date: Wed, 4 Mar 2026 16:50:30 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: pg_waldump: support decoding of WAL inside tarfile To: Amul Sul Cc: Robert Haas , Chao Li , Jakub Wartak , PostgreSQL Hackers References: From: Andrew Dunstan Content-Language: en-US Autocrypt: addr=andrew@dunslane.net; keydata= xsBNBE7KWFkBCAClridxur2AIc7eW2AR7izbfp3EnNefie2HbLF0izW5Ik5UjX2HBXBx4syI gY6b0ugohXrr274+baoAlvSbq6cAoQuEVrk5IZFzt20b1Xkx65FwGSEj526yiKLocqkJceSq Xr9xcA5SGY+FZv441chh5SU92v4q6z+6LPpoHOh97ptAVXZYNTtU0LevyvD5lja0TzbvJm6C eFXitJfnm1pLEr0DGJCR/iUOl/N62Kh4855zZC7NHIjQHPOvV5Stz/l5ilDhvGVk+xkXFPys SjZoUr1rXhYLpiyi5sR0X9FHXT0KnGuz1F5ERO7ZTLSSQ6fJwPj6gOk9K+vvoKvoeql5ABEB AAHNJEFuZHJldyBEdW5zdGFuIDxhbmRyZXdAZHVuc2xhbmUubmV0PsLAlwQTAQgAQQIbAwIX gAIZAQULCQgHAwUVCgkICwUWAgMBAAIeBRYhBOQ+WEYd/Hy/RGkVpZn6f8tZ/DuBBQJoGNGd BQkdEO8nAAoJEJn6f8tZ/DuBq74H/jkTR4Zi3stbw+xC7v2u3QozssK7MYPL2AsVfh7OealS h182fiWXpfvmmAB7WUHbhk9GC2RAOnHI/2d2jgKaMLAHsGYOT0YopTVIwRY43fCw/mK67yxc wmDcX+zyKfLaivNbf5A7QPLNwda98bEAMSJ8Sn652Uc6cA8t3uKGsVzbRBQOoYzjgvBCfSrE 9ql3PDNg0l4BfAqabd2f70ZUm9VAMEPrgv/v2xI7M2XiL4g5BVmqLCOwxLM8RMCotCuoweUr VO43DeBCIDwLxotMJKvGWDjBzQYlU1NPUAtNcz/gN9ITUe1VUGjyvGj4u1lxBOcQQUw7l1+T 5moZ4iZxXzvOwE0ETspYWQEIANGc4zQULOxhbqO2dyD51YhqCNRmm9oKWaqf+wmW4tpDe/VV cxAnNizd4LWCHfzpb5cHAtGkOPePMfzWVf6nvdF7d3eglbtf59+zG7O7llV0xSSoFiieQBsr GvqDInXYX/4mRRXMtyhM353/tixC9RWLs1oofyYmCPPXXY7h9R7en3B8BoVrRFcdzlIY/NFN hFGW/9dkEiGjgna2Rk6e15kln4ZvFBWUg23p93w/pqXcxY6+k/8TEk+C4R+M6w7o2PLGOjdZ +kPiUcw5H85zf/yZJwQXzisXaNduwWB6Vads9YC9dj6kPR1c4VGRqAaYL++LAEOqrlvm2Tvq QqZRtnEAEQEAAcLAfAQYAQgAJgIbDBYhBOQ+WEYd/Hy/RGkVpZn6f8tZ/DuBBQJoGNI2BQkd EODdAAoJEJn6f8tZ/DuBfw0IAKTsfD40teP/pp+bsLLMSxPXUYrrprTj7WFB5v61p6dkpSr/ qXmMlyahdxQFaPmfVgVirB1Vk/kHiWNnnGjfUV9nB2Zg9LI0Xb9/ts3LsUiRWXzG3tkMY6XL vsVOxW4XFRND9l2q+WW93aZ1DZl+fqWfYgMvsusFRhmGFOKTRfKPta2Pkv+AhA24N4+PrR5p bU4k2MO8PAGiK8eaYKGFG1bHKuAvoDoF7WXJ3FHxuWqLnKEt4dfOLm5pAe3zq1Lt6q8azT9i QWGpSAK5vQUWQHBHpiDjdPeqKZ6HiAXIIKfSmb+jrvXBqoP+D6/K7rUjG2aXiRtTIAXms9sm VRu7cmw= In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------JvWy99edmXlSqtQV26VaPR0z Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2026-03-04 We 7:52 AM, Amul Sul wrote: > On Wed, Mar 4, 2026 at 6:07 AM Andrew Dunstan wrote: >> >> On 2026-03-02 Mo 8:00 AM, Amul Sul wrote: >>> On Wed, Feb 18, 2026 at 12:28 PM Amul Sul wrote: >>>> On Tue, Feb 10, 2026 at 3:06 PM Amul Sul wrote: >>>>> On Wed, Feb 4, 2026 at 6:39 PM Amul Sul wrote: >>>>>> On Wed, Jan 28, 2026 at 2:41 AM Robert Haas wrote: >>>>>>> On Tue, Jan 27, 2026 at 7:07 AM Amul Sul wrote: >>>>>>>> In the attached version, I am using the WAL segment name as the hash >>>>>>>> key, which is much more straightforward. I have rewritten >>>>>>>> read_archive_wal_page(), and it looks much cleaner than before. The >>>>>>>> logic to discard irrelevant WAL files is still within >>>>>>>> get_archive_wal_entry. I added an explanation for setting cur_wal to >>>>>>>> NULL, which is now handled in the separate function I mentioned >>>>>>>> previously. >>>>>>>> >>>>>>>> Kindly have a look at the attached version; let me know if you are >>>>>>>> still not happy with the current approach for filtering/discarding >>>>>>>> irrelevant WAL segments. It isn't much different from the previous >>>>>>>> version, but I have tried to keep it in a separate routine for better >>>>>>>> code readability, with comments to make it easier to understand. I >>>>>>>> also added a comment for ArchivedWALFile. >>>>>>> I feel like the division of labor between get_archive_wal_entry() and >>>>>>> read_archive_wal_page() is odd. I noticed this in the last version, >>>>>>> too, and it still seems to be the case. get_archive_wal_entry() first >>>>>>> calls ArchivedWAL_lookup(). If that finds an entry, it just returns. >>>>>>> If it doesn't, it loops until an entry for the requested file shows up >>>>>>> and then returns it. Then control returns to read_archive_wal_page() >>>>>>> which loops some more until we have all the data we need for the >>>>>>> requested file. But it seems odd to me to have two separate loops >>>>>>> here. I think that the first loop is going to call read_archive_file() >>>>>>> until we find the beginning of the file that we care about and then >>>>>>> the second one is going to call read_archive_file() some more until we >>>>>>> have read enough of it to satisfy the request. It feels odd to me to >>>>>>> do it that way, as if we told somebody to first wait until 9 o'clock >>>>>>> and then wait another 30 minutes, instead of just telling them to wait >>>>>>> until 9:30. I realize it's not quite the same thing, because apart >>>>>>> from calling read_archive_file(), the two loops do different things, >>>>>>> but I still think it looks odd. >>>>>>> >>>>>>> + /* >>>>>>> + * Ignore if the timeline is different or the current segment is not >>>>>>> + * the desired one. >>>>>>> + */ >>>>>>> + XLogFromFileName(entry->fname, &curSegTimeline, &curSegNo, WalSegSz); >>>>>>> + if (privateInfo->timeline != curSegTimeline || >>>>>>> + privateInfo->startSegNo > curSegNo || >>>>>>> + privateInfo->endSegNo < curSegNo || >>>>>>> + segno > curSegNo) >>>>>>> + { >>>>>>> + free_archive_wal_entry(entry->fname, privateInfo); >>>>>>> + continue; >>>>>>> + } >>>>>>> >>>>>>> The comment doesn't match the code. If it did, the test would be >>>>>>> (privateInfo->timeline != curSegTimeline || segno != curSegno). But >>>>>>> instead the segno test is > rather than !=, and the checks against >>>>>>> startSegNo and endSegNo aren't explained at all. I think I understand >>>>>>> why the segno test uses > rather than !=, but it's the point of the >>>>>>> comment to explain things like that, rather than leaving the reader to >>>>>>> guess. And I don't know why we also need to test startSegNo and >>>>>>> endSegNo. >>>>>>> >>>>>>> I also wonder what the point is of doing XLogFromFileName() on the >>>>>>> fname provided by the caller and then again on entry->fname. Couldn't >>>>>>> you just compare the strings? >>>>>>> >>>>>>> Again, the division of labor is really odd here. It's the job of >>>>>>> astreamer_waldump_content() to skip things that aren't WAL files at >>>>>>> all, but it's the job of get_archive_wal_entry() to skip things that >>>>>>> are WAL files but not the one we want. I disagree with putting those >>>>>>> checks in completely separate parts of the code. >>>>>>> >>>>>> Keeping the timeline and segment start-end range checks inside the >>>>>> archive streamer creates a circular dependency that cannot be resolved >>>>>> without a 'dirty hack'. We must read the first available WAL file page >>>>>> to determine the wal_segment_size before it can calculate the target >>>>>> segment range. Moving the checks inside the streamer would make it >>>>>> impossible to process that initial file, as the necessary filtering >>>>>> parameters -- would still be unknown which would need to be skipped >>>>>> for the first read somehow. What if later we realized that the first >>>>>> WAL file which was allowed to be streamed by skipping that check is >>>>>> irrelevant and doesn't fall under the start-end segment range? >>>>>> >>>>> Please have a look at the attached version, specifically patch 0005. >>>>> In astreamer_waldump_content(), I have moved the WAL file filtration >>>>> check from get_archive_wal_entry(). This check will be skipped during >>>>> the initial read in init_archive_reader(), which instead performs it >>>>> explicitly once it determines the WAL segment size and the start/end >>>>> segments. >>>>> >>>>> To access the WAL segment size inside astreamer_waldump_content(), I >>>>> have moved the WAL segment size variable into the XLogDumpPrivate >>>>> structure in the separate 0004 patch. >>>> Attached is an updated version including the aforesaid changes. It >>>> includes a new refactoring patch (0001) that moves the logic for >>>> identifying tar archives and their compression types from >>>> pg_basebackup and pg_verifybackup into a separate-reusable function, >>>> per a suggestion from Euler [1]. Additionally, I have added a test >>>> for the contrecord decoding to the main patch (now 0006). >>>> >>>> 1]http://postgr.es/m/3c8e7b02-2152-495a-a0b6-e37cf9286a70@app.fastmail.com >>>> >>> Rebased against the latest master, fixed typos in code comments, and >>> replaced palloc0 with palloc0_object. >>> >> Hi Amul. >> >> >> I think this looks in pretty good shape. >> > Thank you very much for looking at the patch. > >> Attached are patches for a few things I think could be fixed. They are >> mostly self-explanatory. The TAP test fix is the only sane way I could >> come up with stopping the skip code you had from reporting a wildly >> inaccurate number of tests skipped. The sane way to do this from a >> Test::More perspective is a subtest, but unfortunately meson does not >> like subtest output, which is why we don't use it elsewhere, so the only >> way I could come up with was to split this out into a separate test. Of >> course, we might just say we don't care about the misreport, in which >> case we could just live with things as they are. >> > I agree that the reported skip number was incorrect, and I have > corrected it in the attached patch. I haven't applied your patch for > the TAP test improvements yet because I wanted to double-check it > first with you; the patch as it stood created duplicate tests already > present in 001_basic.pl. To avoid this duplication, I have added a > loop that performs tests for both plain and tar WAL directory inputs, > similar to the approach used in pg_verifybackup for different > compression type tests (e.g., 008_untar.pl, 010_client_untar.pl). I > don't have any objection to doing so if you feel the duplication is > acceptable, but I feel that using a loop for the tests in 001_basic.pl > is a bit tidier. Let me know your thoughts. I will take a look. > > I have applied all your other patches but skipped the changes to > pg_verifybackup.c from cf5955-fixes.patch.no-cfbot, as they seem > unrelated or perhaps I have misunderstood them. That's what I get for using a poorly written tool. cheers andrew -- Andrew Dunstan EDB:https://www.enterprisedb.com --------------JvWy99edmXlSqtQV26VaPR0z Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit


On 2026-03-04 We 7:52 AM, Amul Sul wrote:
On Wed, Mar 4, 2026 at 6:07 AM Andrew Dunstan <andrew@dunslane.net> wrote:


On 2026-03-02 Mo 8:00 AM, Amul Sul wrote:

On Wed, Feb 18, 2026 at 12:28 PM Amul Sul <sulamul@gmail.com> wrote:

On Tue, Feb 10, 2026 at 3:06 PM Amul Sul <sulamul@gmail.com> wrote:

On Wed, Feb 4, 2026 at 6:39 PM Amul Sul <sulamul@gmail.com> wrote:

On Wed, Jan 28, 2026 at 2:41 AM Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Jan 27, 2026 at 7:07 AM Amul Sul <sulamul@gmail.com> wrote:

In the attached version, I am using the WAL segment name as the hash
key, which is much more straightforward. I have rewritten
read_archive_wal_page(), and it looks much cleaner than before. The
logic to discard irrelevant WAL files is still within
get_archive_wal_entry. I added an explanation for setting cur_wal to
NULL, which is now handled in the separate function I mentioned
previously.

Kindly have a look at the attached version; let me know if you are
still not happy with the current approach for filtering/discarding
irrelevant WAL segments. It isn't much different from the previous
version, but I have tried to keep it in a separate routine for better
code readability, with comments to make it easier to understand. I
also added a comment for ArchivedWALFile.

I feel like the division of labor between get_archive_wal_entry() and
read_archive_wal_page() is odd. I noticed this in the last version,
too, and it still seems to be the case. get_archive_wal_entry() first
calls ArchivedWAL_lookup(). If that finds an entry, it just returns.
If it doesn't, it loops until an entry for the requested file shows up
and then returns it. Then control returns to read_archive_wal_page()
which loops some more until we have all the data we need for the
requested file. But it seems odd to me to have two separate loops
here. I think that the first loop is going to call read_archive_file()
until we find the beginning of the file that we care about and then
the second one is going to call read_archive_file() some more until we
have read enough of it to satisfy the request. It feels odd to me to
do it that way, as if we told somebody to first wait until 9 o'clock
and then wait another 30 minutes, instead of just telling them to wait
until 9:30. I realize it's not quite the same thing, because apart
from calling read_archive_file(), the two loops do different things,
but I still think it looks odd.

+ /*
+ * Ignore if the timeline is different or the current segment is not
+ * the desired one.
+ */
+ XLogFromFileName(entry->fname, &curSegTimeline, &curSegNo, WalSegSz);
+ if (privateInfo->timeline != curSegTimeline ||
+ privateInfo->startSegNo > curSegNo ||
+ privateInfo->endSegNo < curSegNo ||
+ segno > curSegNo)
+ {
+ free_archive_wal_entry(entry->fname, privateInfo);
+ continue;
+ }

The comment doesn't match the code. If it did, the test would be
(privateInfo->timeline != curSegTimeline || segno != curSegno). But
instead the segno test is > rather than !=, and the checks against
startSegNo and endSegNo aren't explained at all. I think I understand
why the segno test uses > rather than !=, but it's the point of the
comment to explain things like that, rather than leaving the reader to
guess. And I don't know why we also need to test startSegNo and
endSegNo.

I also wonder what the point is of doing XLogFromFileName() on the
fname provided by the caller and then again on entry->fname. Couldn't
you just compare the strings?

Again, the division of labor is really odd here. It's the job of
astreamer_waldump_content() to skip things that aren't WAL files at
all, but it's the job of get_archive_wal_entry() to skip things that
are WAL files but not the one we want. I disagree with putting those
checks in completely separate parts of the code.


Keeping the timeline and segment start-end range checks inside the
archive streamer creates a circular dependency that cannot be resolved
without a 'dirty hack'. We must read the first available WAL file page
to determine the wal_segment_size before it can calculate the target
segment range. Moving the checks inside the streamer would make it
impossible to process that initial file, as the necessary filtering
parameters --  would still be unknown which would need to be skipped
for the first read somehow. What if later we realized that the first
WAL file which was allowed to be streamed by skipping that check is
irrelevant and doesn't fall under the start-end segment range?


Please have a look at the attached version, specifically patch 0005.
In astreamer_waldump_content(), I have moved the WAL file filtration
check from get_archive_wal_entry(). This check will be skipped during
the initial read in init_archive_reader(), which instead performs it
explicitly once it determines the WAL segment size and the start/end
segments.

To access the WAL segment size inside astreamer_waldump_content(), I
have moved the WAL segment size variable into the XLogDumpPrivate
structure in the separate 0004 patch.

Attached is an updated version including the aforesaid changes. It
includes a new refactoring patch (0001) that moves the logic for
identifying tar archives and their compression types from
pg_basebackup and pg_verifybackup into a separate-reusable function,
per a suggestion from Euler [1].  Additionally, I have added a test
for the contrecord decoding to the main patch (now 0006).

1] http://postgr.es/m/3c8e7b02-2152-495a-a0b6-e37cf9286a70@app.fastmail.com


Rebased against the latest master, fixed typos in code comments, and
replaced palloc0 with palloc0_object.


Hi Amul.


I think this looks in pretty good shape.


Thank you very much for looking at the patch.


Attached are patches for a few things I think could be fixed. They are
mostly self-explanatory. The TAP test fix is the only sane way I could
come up with stopping the skip code you had from reporting a wildly
inaccurate number of tests skipped. The sane way to do this from a
Test::More perspective is a subtest, but unfortunately meson does not
like subtest output, which is why we don't use it elsewhere, so the only
way I could come up with was to split this out into a separate test. Of
course, we might just say we don't care about the misreport, in which
case we could just live with things as they are.


I agree that the reported skip number was incorrect, and I have
corrected it in the attached patch. I haven't applied your patch for
the TAP test improvements yet because I wanted to double-check it
first with you; the patch as it stood created duplicate tests already
present in 001_basic.pl. To avoid this duplication, I have added a
loop that performs tests for both plain and tar WAL directory inputs,
similar to the approach used in pg_verifybackup for different
compression type tests (e.g., 008_untar.pl, 010_client_untar.pl). I
don't have any objection to doing so if you feel the duplication is
acceptable, but I feel that using a loop for the tests in 001_basic.pl
is a bit tidier. Let me know your thoughts.


I will take a look.


I have applied all your other patches but skipped the changes to
pg_verifybackup.c from cf5955-fixes.patch.no-cfbot, as they seem
unrelated or perhaps I have misunderstood them.


<brown-paper-bag> That's what I get for using a poorly written tool.


cheers


andrew 

--
Andrew Dunstan
EDB: https://www.enterprisedb.com
--------------JvWy99edmXlSqtQV26VaPR0z--