Re: [PATCH] Expose port->authn_id to extensions and triggers

2022-08-14 Thread Michael Paquier
On Fri, Aug 12, 2022 at 03:34:04PM +0200, Drouvot, Bertrand wrote:
> 3)
> 
> +SerializeClientConnectionInfo(Size maxsize, char *start_address)
> +{
> +   /*
> +    * First byte is an indication of whether or not authn_id has been
> set to
> +    * non-NULL, to differentiate that case from the empty string.
> +    */
> 
> is authn_id being an empty string possible?

I don't recall that this can be the case yet, but we cannot discard
it.  One thing was itching me about the serialization and
deserialization logic though: could it be more readable if we used an
intermediate structure to store the length of the serialized strings?
We use this approach in other areas, like for the snapshot data in
snapmgr.c.  This would handle the case of an empty and NULL string, by
storing -1 as length for NULL and >= 0 for the string length if there
is something set, while making the addition of more fields a
no-brainer.
--
Michael


signature.asc
Description: PGP signature


Re: Tab completion for "ALTER TYPE typename SET" and rearranged "Alter TYPE typename RENAME"

2022-08-14 Thread Michael Paquier
On Sun, Aug 14, 2022 at 08:25:01AM +0530, vignesh C wrote:
> Attached patch has the changes for the same. Thoughts?
>
> a) Add tab completion for "ALTER TYPE typename SET" was missing.

Why not.  I can also note that CREATE TYPE lists all the properties
that can be set to a new type.  We could bother adding these for ALTER
TYPE, perhaps?

> b) Tab completion for "ALTER TYPE  RENAME VALUE" was not along with tab
> completion of "ALTER TYPE" commands, it was present after "ALTER GROUP
> ", rearranged "ALTER TYPE  RENAME VALUE", so that it is along with
> "ALTER TYPE" commands.

Yeah, no objections to keep that grouped.
--
Michael


signature.asc
Description: PGP signature


Re: fix stale help message

2022-08-14 Thread Michael Paquier
On Wed, Aug 10, 2022 at 11:32:18PM +0800, Junwang Zhao wrote:
> when parsing command-line options, the -f option support disabling
> 8 scan and join methods, o, b and t disable index-only scans,
> bitmap index scans, and TID scans respectively, add them to the
> help message.
>
> @@ -351,7 +351,7 @@ help(const char *progname)
> printf(_("  -?, --help show this help, then exit\n"));
> 
> printf(_("\nDeveloper options:\n"));
> -   printf(_("  -f s|i|n|m|h   forbid use of some plan
> types\n"));
> +   printf(_("  -f s|i|o|b|t|n|m|h forbid use of some plan
> types\n"));
> printf(_("  -n do not reinitialize shared
> memory after abnormal exit\n"));
> printf(_("  -O allow system table structure
> changes\n"));
> printf(_("  -P disable system indexes\n"));

set_plan_disabling_options() is telling that you have all of them, as
much as the docs.  I don't mind fixing that as you suggest, FWIW.
--
Michael


signature.asc
Description: PGP signature


latest patches not updated in repos

2022-08-14 Thread Graaff, Selwyn
Hi,

I am not sure whom I need to contact but the patches were not pushed to the 
repos as per release notes.
https://www.postgresql.org/docs/release/
PostgreSQL: Release Notes
11th August 2022: PostgreSQL 14.5, 13.8, 12.12, 11.17, 10.22, and 15 Beta 3 
Released! Quick Links. Documentation; Manuals. Archive; Release Notes; Books; 
Tutorials ...
www.postgresql.org
The latest still shows 1 version back.
[cid:3d2d83b5-db21-489c-bf73-d9ed43743689]

regards
Selwyn

 

This email is subject to a disclaimer.

Visit the FNB website and view the email disclaimer and privacy notice by 
clicking the "About FNB + Legal" and "Legal Matters" links.
If you are unable to access our website, please contact us to send you a copy 
of the email disclaimer or privacy notice.


Re: fix stale help message

2022-08-14 Thread Junwang Zhao
Hi Michael,

Thanks for your reply :)

I think the goal of `help message` is to tell users(like DBA) how
to use postgres, so it's better to provide a complete view.

I'm not sure by saying `set_plan_disabling_options` do you mean
the postgres source code? If that's the case, I think most of the
installations don't have source code but just binaries, people will
reference the help message by running `postgres --help`.

BTW, I noticed in [0] the doc gives the full options, so I think we
should keep the source code in consistent with the doc.

[0] https://www.postgresql.org/docs/current/app-postgres.html

On Sun, Aug 14, 2022 at 6:17 PM Michael Paquier  wrote:
>
> On Wed, Aug 10, 2022 at 11:32:18PM +0800, Junwang Zhao wrote:
> > when parsing command-line options, the -f option support disabling
> > 8 scan and join methods, o, b and t disable index-only scans,
> > bitmap index scans, and TID scans respectively, add them to the
> > help message.
> >
> > @@ -351,7 +351,7 @@ help(const char *progname)
> > printf(_("  -?, --help show this help, then exit\n"));
> >
> > printf(_("\nDeveloper options:\n"));
> > -   printf(_("  -f s|i|n|m|h   forbid use of some plan
> > types\n"));
> > +   printf(_("  -f s|i|o|b|t|n|m|h forbid use of some plan
> > types\n"));
> > printf(_("  -n do not reinitialize shared
> > memory after abnormal exit\n"));
> > printf(_("  -O allow system table structure
> > changes\n"));
> > printf(_("  -P disable system indexes\n"));
>
> set_plan_disabling_options() is telling that you have all of them, as
> much as the docs.  I don't mind fixing that as you suggest, FWIW.
> --
> Michael



-- 
Regards
Junwang Zhao




Re: latest patches not updated in repos

2022-08-14 Thread Tom Lane
"Graaff, Selwyn"  writes:
> I am not sure whom I need to contact but the patches were not pushed to the 
> repos as per release notes.
> https://www.postgresql.org/docs/release/
> PostgreSQL: Release Notes
> 11th August 2022: PostgreSQL 14.5, 13.8, 12.12, 11.17, 10.22, and 15 Beta 3 
> Released! Quick Links. Documentation; Manuals. Archive; Release Notes; Books; 
> Tutorials ...

I see them there.  Clear your browser cache, perhaps?

regards, tom lane




Re: latest patches not updated in repos

2022-08-14 Thread Devrim Gündüz
Hi,

On Sun, 2022-08-14 at 10:10 -0400, Tom Lane wrote:
> "Graaff, Selwyn"  writes:
> > I am not sure whom I need to contact but the patches were not
> > pushed to the repos as per release notes.
> > https://www.postgresql.org/docs/release/
> > PostgreSQL: Release Notes
> > 11th August 2022: PostgreSQL 14.5, 13.8, 12.12, 11.17, 10.22, and
> > 15 Beta 3 Released! Quick Links. Documentation; Manuals. Archive;
> > Release Notes; Books; Tutorials ...
> 
> I see them there.  Clear your browser cache, perhaps?

Actually I just pushed them. Only 13.8 updates were missing, sorry
about that.

Regards,

-- 
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR


signature.asc
Description: This is a digitally signed message part


Re: Tab completion for "ALTER TYPE typename SET" and rearranged "Alter TYPE typename RENAME"

2022-08-14 Thread vignesh C
On Sun, Aug 14, 2022 at 3:41 PM Michael Paquier  wrote:
>
> On Sun, Aug 14, 2022 at 08:25:01AM +0530, vignesh C wrote:
> > Attached patch has the changes for the same. Thoughts?
> >
> > a) Add tab completion for "ALTER TYPE typename SET" was missing.
>
> Why not.  I can also note that CREATE TYPE lists all the properties
> that can be set to a new type.  We could bother adding these for ALTER
> TYPE, perhaps?

Modified the patch to list all the properties in case of "ALTER TYPE
typename SET (". I have included the properties in alphabetical order
as I notice that the ordering is in alphabetical order in few cases
ex: "ALTER SUBSCRIPTION  SET (". The attached v2 patch has the
changes for the same. Thoughts?

Regards,
Vignesh


v2-0001-Tab-completion-for-ALTER-TYPE-typename-SET-and-re.patch
Description: Binary data


bogus assert in logicalmsg_desc

2022-08-14 Thread Tomas Vondra
Hi,

while experimenting with logical messages, I ran into this assert in
logicalmsg_desc:

Assert(prefix[xlrec->prefix_size] != '\0');

This seems to be incorrect, because LogLogicalMessage does this:

xlrec.prefix_size = strlen(prefix) + 1;

So prefix_size includes the null byte, so the assert points out at the
first payload byte. And of course, the check should be "==" because we
expect the byte to be \0, not the other way around.

It's pretty simple to make this crash by writing a logical message where
the first payload byte is \0, e.g. like this:

select pg_logical_emit_message(true, 'a'::text, '\x00'::bytea);

and then running pg_waldump on the WAL segment.

Attached is a patch addressing this. This was added in 14, so we should
backpatch to that version.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Companydiff --git a/src/backend/access/rmgrdesc/logicalmsgdesc.c b/src/backend/access/rmgrdesc/logicalmsgdesc.c
index 099e11a84e7..08e03aa30d1 100644
--- a/src/backend/access/rmgrdesc/logicalmsgdesc.c
+++ b/src/backend/access/rmgrdesc/logicalmsgdesc.c
@@ -28,7 +28,7 @@ logicalmsg_desc(StringInfo buf, XLogReaderState *record)
 		char	   *message = xlrec->message + xlrec->prefix_size;
 		char	   *sep = "";
 
-		Assert(prefix[xlrec->prefix_size] != '\0');
+		Assert(prefix[xlrec->prefix_size - 1] == '\0');
 
 		appendStringInfo(buf, "%s, prefix \"%s\"; payload (%zu bytes): ",
 		 xlrec->transactional ? "transactional" : "non-transactional",


Re: Include the dependent extension information in describe command.

2022-08-14 Thread vignesh C
On Sun, Aug 14, 2022 at 11:07 AM Tom Lane  wrote:
>
> vignesh C  writes:
> > Currently we do not include the dependent extension information for
> > index and materialized view in the describe command. I felt it would
> > be useful to include this information as part of the describe command
> > like:
> > \d+ idx_depends
> >   Index "public.idx_depends"
> >  Column |  Type   | Key? | Definition | Storage | Stats target
> > +-+--++-+--
> >  a  | integer | yes  | a  | plain   |
> > btree, for table "public.tbl_idx_depends"
> > Depends:
> > "plpgsql"
>
> > Attached a patch for the same. Thoughts?
>
> This seems pretty much useless noise to me.  Can you point to
> any previous requests for such a feature?  If we did do it,
> why would we do it in such a narrow fashion (ie, only dependencies
> of two specific kinds of objects on one other specific kind of
> object)?  Why did you do it in this direction rather than
> the other one, ie show dependencies when examining the extension?

While implementing logical replication of "index which depends on
extension", I found that this information was not available in any of
the \d describe commands. I felt having this information in the \d
describe command will be useful in validating the "depends on
extension" easily. Now that you pointed out, I agree that it will be
better to show the dependencies from the extension instead of handling
it in multiple places. I will change it to handle it from extension
and post an updated version soon for this.

Regards,
Vignesh




Re: bogus assert in logicalmsg_desc

2022-08-14 Thread Masahiko Sawada
On Mon, Aug 15, 2022 at 1:17 AM Tomas Vondra
 wrote:
>
> Hi,
>
> while experimenting with logical messages, I ran into this assert in
> logicalmsg_desc:
>
> Assert(prefix[xlrec->prefix_size] != '\0');
>
> This seems to be incorrect, because LogLogicalMessage does this:
>
> xlrec.prefix_size = strlen(prefix) + 1;
>
> So prefix_size includes the null byte, so the assert points out at the
> first payload byte. And of course, the check should be "==" because we
> expect the byte to be \0, not the other way around.
>
> It's pretty simple to make this crash by writing a logical message where
> the first payload byte is \0, e.g. like this:
>
> select pg_logical_emit_message(true, 'a'::text, '\x00'::bytea);
>
> and then running pg_waldump on the WAL segment.
>
> Attached is a patch addressing this. This was added in 14, so we should
> backpatch to that version.

+1

The patch looks good to me.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/




Re: bogus assert in logicalmsg_desc

2022-08-14 Thread Richard Guo
On Mon, Aug 15, 2022 at 12:17 AM Tomas Vondra 
wrote:

> So prefix_size includes the null byte, so the assert points out at the
> first payload byte. And of course, the check should be "==" because we
> expect the byte to be \0, not the other way around.


Yes, indeed. There is even a comment emphasizing the trailing null byte
in LogLogicalMessage.

/* trailing zero is critical; see logicalmsg_desc */



>
> Attached is a patch addressing this. This was added in 14, so we should
> backpatch to that version.
>

+1 for the patch.

Thanks
Richard


Re: Cleaning up historical portability baggage

2022-08-14 Thread Thomas Munro
On Sun, Aug 14, 2022 at 10:36 AM Andres Freund  wrote:
> On 2022-08-14 10:03:19 +1200, Thomas Munro wrote:
> > I hadn't paid attention to our existing abstract Unix socket support
> > before and now I'm curious: do we have a confirmed sighting of that
> > working on Windows?
>
> I vaguely remember successfully trying it in the past. But I just tried it
> unsuccessfully in a VM and there's a bunch of other places saying it's not
> working...
> https://github.com/microsoft/WSL/issues/4240

I think we'd better remove our claim that it works then.  Patch attached.

We could also reject it, I guess, but it doesn't immediately seem
harmful so I'm on the fence.  On the Windows version that Cirrus is
running, we happily start up with:

2022-08-13 20:44:35.174 GMT [4760][postmaster] LOG:  listening on Unix
socket "@c:/cirrus/.s.PGSQL.61696"

... and then client processes apparently can't see it, which is
confusing but, I guess, defensible if we're only claiming it works on
Linux.  We don't go out of our way to avoid this feature on a per-OS
basis in general, though at least on a typical Unix system it fails
fast.  For example, my FreeBSD system here barfs:

2022-08-15 13:26:13.483 NZST [29956] LOG:  could not bind Unix address
"@/tmp/.s.PGSQL.5432": No such file or directory

... because the kernel just sees an empty string and can't locate the
parent directory.
From a850eb8b46bfdcd768a04b335f7de360a4bc3c69 Mon Sep 17 00:00:00 2001
From: Thomas Munro 
Date: Mon, 15 Aug 2022 10:43:13 +1200
Subject: [PATCH] Doc: Abstract AF_UNIX sockets don't work on Windows after
 all.

An early release of AF_UNIX in Windows might have supported Linux-style
"abstract" Unix sockets with a system-wide namespace, but they do not
seem to work in current Windows versions and there is no mention of any
of this in the Winsock documentation.  Remove the claim that it works
from our documentation.

Back-patch to 14, where commit c9f0624b landed.

Discussion: https://postgr.es/m/20220813223646.oh2dkjrkj7jn7dpe%40awork3.anarazel.de
---
 doc/src/sgml/config.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a5cd4e44c7..1fbd5fc0d4 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -759,7 +759,7 @@ include_dir 'conf.d'

 A value that starts with @ specifies that a
 Unix-domain socket in the abstract namespace should be created
-(currently supported on Linux and Windows).  In that case, this value
+(currently supported on Linux).  In that case, this value
 does not specify a directory but a prefix from which
 the actual socket name is computed in the same manner as for the
 file-system namespace.  While the abstract socket name prefix can be
-- 
2.35.1



Re: Improve description of XLOG_RUNNING_XACTS

2022-08-14 Thread Masahiko Sawada
Sorry for the late reply.

On Thu, Jul 28, 2022 at 4:29 PM Kyotaro Horiguchi
 wrote:
>
> At Thu, 28 Jul 2022 15:53:33 +0900, Masahiko Sawada  
> wrote in
> > >
> > Do you mean that both could be true at the same time? If I read
> > GetRunningTransactionData() correctly, that doesn't happen.
>
> So, I wrote "since it is debugging output", and "fine if we asuume the
> record is sound". Is it any trouble with assuming the both *can*
> happen at once?  If something's broken, it will be reflected in the
> output.

Fair point. We may not need to interpret the contents.

On Thu, Jul 28, 2022 at 3:24 PM Kyotaro Horiguchi
 wrote:
>
> Another point is if the xid/subxid lists get long, I see it annoying
> that the "overflowed" messages goes far away to the end of the long
> line. Couldn't we rearrange the item order of the line as the follows?
>
> nextXid %u latestCompletedXid %u oldestRunningXid %u;[ subxid overflowed;][ 
> %d xacts: %u %u ...;][ subxacts: %u %u ..]
>

I'm concerned that we have two information of subxact apart. Given
that showing both individual subxacts and "overflow" is a bug, I think
we can output like:

if (xlrec->subxcnt > 0)
{
appendStringInfo(buf, "; %d subxacts:", xlrec->subxcnt);
for (i = 0; i < xlrec->subxcnt; i++)
appendStringInfo(buf, " %u", xlrec->xids[xlrec->xcnt + i]);
}

if (xlrec->subxid_overflow)
appendStringInfoString(buf, "; subxid overflowed");

Or we can output the "subxid overwlowed" first.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/




Re: fix stale help message

2022-08-14 Thread Michael Paquier
On Sun, Aug 14, 2022 at 09:03:15PM +0800, Junwang Zhao wrote:
> I'm not sure by saying `set_plan_disabling_options` do you mean
> the postgres source code? If that's the case, I think most of the
> installations don't have source code but just binaries, people will
> reference the help message by running `postgres --help`.

I am just telling that your patch does the right thing, based on the
state of the code and the contents of the docs.  Applied down to 10.
--
Michael


signature.asc
Description: PGP signature


Re: Tab completion for "ALTER TYPE typename SET" and rearranged "Alter TYPE typename RENAME"

2022-08-14 Thread Michael Paquier
On Sun, Aug 14, 2022 at 07:56:00PM +0530, vignesh C wrote:
> Modified the patch to list all the properties in case of "ALTER TYPE
> typename SET (". I have included the properties in alphabetical order
> as I notice that the ordering is in alphabetical order in few cases
> ex: "ALTER SUBSCRIPTION  SET (". The attached v2 patch has the
> changes for the same. Thoughts?

Seems fine here, so applied after tweaking a bit the comments, mostly
for consistency with the area.
--
Michael


signature.asc
Description: PGP signature


Re: Fast COPY FROM based on batch insert

2022-08-14 Thread Andrey Lepikhov

On 8/9/22 16:44, Etsuro Fujita wrote:

-1 foo
1 bar
\.

ERROR:  new row for relation "t1" violates check constraint "t1_f1positive"
DETAIL:  Failing row contains (-1, foo).
CONTEXT:  remote SQL command: INSERT INTO public.t1(f1, f2) VALUES
($1, $2), ($3, $4)
COPY ft1, line 3

In single-insert mode the error context information is correct, but in
batch-insert mode it isn’t (i.e., the line number isn’t correct).

The error occurs on the remote side, so I'm not sure if there is a
simple fix.  What I came up with is to just suppress error context
information other than the relation name, like the attached.  What do
you think about that?
I've spent many efforts to this problem too. Your solution have a 
rationale and looks fine.
I only think, we should add a bit of info into an error report to 
simplify comprehension why don't point specific line here. For example:

'COPY %s (buffered)'
or
'COPY FOREIGN TABLE %s'

or, if instead of relname_only field to save a MultiInsertBuffer 
pointer, we might add min/max linenos into the report:

'COPY %s, line between %llu and %llu'

--
Regards
Andrey Lepikhov
Postgres Professional




Header checker scripts should fail on failure

2022-08-14 Thread Thomas Munro
Hi,

I thought commit 81b9f23c9c8 had my back, but nope, we still need to
make CI turn red if "headerscheck" and "cpluspluscheck" don't like our
patches (crake in the build farm should be a secondary defence...).
See attached.
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
index eb06ee0111..2d3b785342 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -52,6 +52,8 @@ tmp=`mktemp -d /tmp/$me.XX`
 
 trap 'rm -rf $tmp' 0 1 2 3 15
 
+exit_status=0
+
 # Scan all of src/ and contrib/ for header files.
 for f in `cd "$srcdir" && find src contrib -name '*.h' -print`
 do
@@ -167,9 +169,13 @@ do
 	esac
 
 	# Run the test.
-	${CXX:-g++} -I $builddir -I $srcdir \
+	if ! ${CXX:-g++} -I $builddir -I $srcdir \
 		-I $builddir/src/include -I $srcdir/src/include \
 		-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
 		$EXTRAINCLUDES $EXTRAFLAGS $CXXFLAGS -c $tmp/test.cpp
-
+	then
+		exit_status=1
+	fi
 done
+
+exit $exit_status
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 3f8640a03d..b8419e46a4 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -48,6 +48,8 @@ tmp=`mktemp -d /tmp/$me.XX`
 
 trap 'rm -rf $tmp' 0 1 2 3 15
 
+exit_status=0
+
 # Scan all of src/ and contrib/ for header files.
 for f in `cd "$srcdir" && find src contrib -name '*.h' -print`
 do
@@ -150,9 +152,13 @@ do
 	esac
 
 	# Run the test.
-	${CC:-gcc} $CPPFLAGS $CFLAGS -I $builddir -I $srcdir \
+	if ! ${CC:-gcc} $CPPFLAGS $CFLAGS -I $builddir -I $srcdir \
 		-I $builddir/src/include -I $srcdir/src/include \
 		-I $builddir/src/interfaces/libpq -I $srcdir/src/interfaces/libpq \
 		$EXTRAINCLUDES $EXTRAFLAGS -c $tmp/test.c -o $tmp/test.o
-
+	then
+		exit_status=1
+	fi
 done
+
+exit $exit_status


Re: [PoC] Improve dead tuple storage for lazy vacuum

2022-08-14 Thread Masahiko Sawada
On Fri, Jul 22, 2022 at 10:43 AM Masahiko Sawada  wrote:
>
> On Tue, Jul 19, 2022 at 1:30 PM John Naylor
>  wrote:
> >
> >
> >
> > On Tue, Jul 19, 2022 at 9:11 AM Masahiko Sawada  
> > wrote:
> >
> > > I’d like to keep the first version simple. We can improve it and add
> > > more optimizations later. Using radix tree for vacuum TID storage
> > > would still be a big win comparing to using a flat array, even without
> > > all these optimizations. In terms of single-value leaves method, I'm
> > > also concerned about an extra pointer traversal and extra memory
> > > allocation. It's most flexible but multi-value leaves method is also
> > > flexible enough for many use cases. Using the single-value method
> > > seems to be too much as the first step for me.
> > >
> > > Overall, using 64-bit keys and 64-bit values would be a reasonable
> > > choice for me as the first step . It can cover wider use cases
> > > including vacuum TID use cases. And possibly it can cover use cases by
> > > combining a hash table or using tree of tree, for example.
> >
> > These two aspects would also bring it closer to Andres' prototype, which 1) 
> > makes review easier and 2) easier to preserve optimization work already 
> > done, so +1 from me.
>
> Thanks.
>
> I've updated the patch. It now implements 64-bit keys, 64-bit values,
> and the multi-value leaves method. I've tried to remove duplicated
> codes but we might find a better way to do that.
>

With the recent changes related to simd, I'm going to split the patch
into at least two parts: introduce other simd optimized functions used
by the radix tree and the radix tree implementation. Particularly we
need two functions for radix tree: a function like pg_lfind32 but for
8 bits integers and return the index, and a function that returns the
index of the first element that is >= key.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/




Re: Cleaning up historical portability baggage

2022-08-14 Thread Thomas Munro
On Sun, Aug 14, 2022 at 10:03 AM Thomas Munro  wrote:
> All green on CI...  Next stop, build farm.

All good so far (except for an admonishment from crake, for which my
penance was to fix headerscheck, see separate thread...).  I did
figure out one thing that I mentioned I was confused by before: the
reason Windows didn't like my direct calls to gai_strerror() is
because another header of ours clobbered one of Windows' own macros.
This new batch includes a fix for that.

  Remove configure probe for IPv6.
  Remove dead ifaddrs.c fallback code.
  Remove configure probe for net/if.h.
  Fix macro problem with gai_strerror on Windows.
  Remove configure probe for netinet/tcp.h.
  mstcpip.h is not missing on MinGW.

The interesting one is a continuation of my "all computers have X"
series.  This episode: IPv6.
From cd6ba7f0b275a5c2840ee93849a54b94e40a6450 Mon Sep 17 00:00:00 2001
From: Thomas Munro 
Date: Mon, 15 Aug 2022 14:47:58 +1200
Subject: [PATCH 1/6] Remove configure probe for IPv6.

SUSv3 requires  to define struct sockaddr_in6, and all
targeted Unix systems have it.  Windows has it in ws2ipdef.h.  Remove
the configure probe, the macro and a small amount of dead code.  Also
remove a mention of IPv6-less builds from the documentation, since there
aren't any.
---
 configure   | 10 --
 configure.ac|  6 --
 doc/src/sgml/client-auth.sgml   |  2 --
 src/backend/libpq/auth.c| 21 -
 src/backend/libpq/hba.c |  5 -
 src/backend/libpq/ifaddr.c  | 29 +
 src/backend/libpq/pqcomm.c  |  2 --
 src/backend/utils/adt/network.c | 10 --
 src/backend/utils/adt/pgstatfuncs.c | 11 ++-
 src/bin/initdb/initdb.c | 10 --
 src/include/pg_config.h.in  |  3 ---
 src/interfaces/libpq/fe-connect.c   |  2 --
 src/tools/ifaddrs/test_ifaddrs.c|  2 --
 src/tools/msvc/Solution.pm  |  1 -
 14 files changed, 3 insertions(+), 111 deletions(-)

diff --git a/configure b/configure
index 176e0f9b00..8178f290a9 100755
--- a/configure
+++ b/configure
@@ -16279,16 +16279,6 @@ cat >>confdefs.h <<_ACEOF
 _ACEOF
 
 
-ac_fn_c_check_type "$LINENO" "struct sockaddr_in6" "ac_cv_type_struct_sockaddr_in6" "$ac_includes_default
-#include 
-"
-if test "x$ac_cv_type_struct_sockaddr_in6" = xyes; then :
-
-$as_echo "#define HAVE_IPV6 1" >>confdefs.h
-
-fi
-
-
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for PS_STRINGS" >&5
 $as_echo_n "checking for PS_STRINGS... " >&6; }
 if ${pgac_cv_var_PS_STRINGS+:} false; then :
diff --git a/configure.ac b/configure.ac
index eed7019c4a..ee45d856c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1804,12 +1804,6 @@ AC_CHECK_DECLS([pwritev], [], [AC_LIBOBJ(pwritev)], [#include ])
 # This is probably only present on macOS, but may as well check always
 AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include ])
 
-AC_CHECK_TYPE([struct sockaddr_in6],
-[AC_DEFINE(HAVE_IPV6, 1, [Define to 1 if you have support for IPv6.])],
-[],
-[$ac_includes_default
-#include ])
-
 AC_CACHE_CHECK([for PS_STRINGS], [pgac_cv_var_PS_STRINGS],
 [AC_LINK_IFELSE([AC_LANG_PROGRAM(
 [#include 
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 433759928b..c6f1b70fd3 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -305,8 +305,6 @@ hostnogssenc  database  user
 
   
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 290eb17325..7760d714a0 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -3014,13 +3014,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	int			packetlength;
 	pgsocket	sock;
 
-#ifdef HAVE_IPV6
 	struct sockaddr_in6 localaddr;
 	struct sockaddr_in6 remoteaddr;
-#else
-	struct sockaddr_in localaddr;
-	struct sockaddr_in remoteaddr;
-#endif
 	struct addrinfo hint;
 	struct addrinfo *serveraddrs;
 	int			port;
@@ -3130,18 +3125,12 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 	}
 
 	memset(&localaddr, 0, sizeof(localaddr));
-#ifdef HAVE_IPV6
 	localaddr.sin6_family = serveraddrs[0].ai_family;
 	localaddr.sin6_addr = in6addr_any;
 	if (localaddr.sin6_family == AF_INET6)
 		addrsize = sizeof(struct sockaddr_in6);
 	else
 		addrsize = sizeof(struct sockaddr_in);
-#else
-	localaddr.sin_family = serveraddrs[0].ai_family;
-	localaddr.sin_addr.s_addr = INADDR_ANY;
-	addrsize = sizeof(struct sockaddr_in);
-#endif
 
 	if (bind(sock, (struct sockaddr *) &localaddr, addrsize))
 	{
@@ -3244,21 +3233,11 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
 			return STATUS_ERROR;
 		}
 
-#ifdef HAVE_IPV6
 		if (remoteaddr.sin6_port != pg_hton16(port))
-#else
-		if (remoteaddr.sin_port != pg_hton16(port))
-#endif
 		{
-#ifdef HAVE_IPV6
 			ereport(LOG,
 	(errmsg("RADIUS response from %s was sent from incorrect port: %d

Re: Assertion failure in WaitForWALToBecomeAvailable state machine

2022-08-14 Thread Bharath Rupireddy
On Thu, Aug 11, 2022 at 10:06 PM Bharath Rupireddy
 wrote:
>
> Today I encountered the assertion failure [2] twice while working on
> another patch [1]. The pattern seems to be that the walreceiver got
> killed or crashed and set it's state to WALRCV_STOPPING or
> WALRCV_STOPPED by the team the WAL state machine moves to archive and
> hence the following XLogShutdownWalRcv() code will not get hit:
>
> /*
>  * Before we leave XLOG_FROM_STREAM state, make sure that
>  * walreceiver is not active, so that it won't overwrite
>  * WAL that we restore from archive.
>  */
> if (WalRcvStreaming())
> ShutdownWalRcv();
>
> I agree with Kyotaro-san to reset InstallXLogFileSegmentActive before
> entering into the archive mode. Hence I tweaked the code introduced by
> the following commit a bit, the result v1 patch is attached herewith.
> Please review it.

I added it to the current commitfest to not lose track of it:
https://commitfest.postgresql.org/39/3814/.

-- 
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/




Re: Use SetInstallXLogFileSegmentActive() for setting XLogCtl->InstallXLogFileSegmentActive

2022-08-14 Thread Bharath Rupireddy
On Fri, Aug 12, 2022 at 4:17 AM Nathan Bossart  wrote:
>
> On Thu, Aug 11, 2022 at 09:42:18PM +0530, Bharath Rupireddy wrote:
> > Here's a small patch replacing the explicit setting of
> > XLogCtl->InstallXLogFileSegmentActive with the existing function
> > SetInstallXLogFileSegmentActive(), removes duplicate code and saves 4
> > LOC.
>
> LGTM

Thanks for reviewing. I added it to the current commitfest to not lose
track of it - https://commitfest.postgresql.org/39/3815/

--
Bharath Rupireddy
RDS Open Source Databases: https://aws.amazon.com/rds/postgresql/




Re: latest patches not updated in repos - [External Email]

2022-08-14 Thread Graaff, Selwyn
Thank you.

regards,
Selwyn

From: Devrim Gündüz 
Sent: Sunday, 14 August 2022 16:25
To: Tom Lane ; Graaff, Selwyn 
Cc: pgsql-hackers@lists.postgresql.org 
Subject: Re: latest patches not updated in repos - [External Email]

Hi,

On Sun, 2022-08-14 at 10:10 -0400, Tom Lane wrote:
> "Graaff, Selwyn"  writes:
> > I am not sure whom I need to contact but the patches were not
> > pushed to the repos as per release notes.
> > https://secure-web.cisco.com/12ONKcon5VEXiyKROUzZpHq3zopWQUOtcwCJpKyvlbCg08NMT_7-cA9ebiUSIh1tEjJ6yA5Rx1Dy2qLOy7DazeVIcz20AcBuG5aiC3u_Cv42McjVdQ1OfHHn16HL6WhYhWpmEa-4AL6v1ZCBVEw80WKNrIBM6vRX2NTvmhBzPnvpv4bJthhgiLDUAJGOJGnYX1YqBxmvuwrRmirg5OvCr5DNwmRGm9Ludl7qPGJ-Z4Fw_bPZC82Fusm0CgYirG_P-WW2PMmr4wc-0xdFutMamEYrKgU5bP50e3r71nR6p1EMVrfRnSS4QB6SB7ztsVHpAnGyWFCYTpVdb4TInedo5wA/https%3A%2F%2Fwww.postgresql.org%2Fdocs%2Frelease%2F
> > PostgreSQL: Release 
> > Notes
> > 11th August 2022: PostgreSQL 14.5, 13.8, 12.12, 11.17, 10.22, and
> > 15 Beta 3 Released! Quick Links. Documentation; Manuals. Archive;
> > Release Notes; Books; Tutorials ...
>
> I see them there.  Clear your browser cache, perhaps?

Actually I just pushed them. Only 13.8 updates were missing, sorry
about that.

Regards,

--
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR
 

This email is subject to a disclaimer.

Visit the FNB website and view the email disclaimer and privacy notice by 
clicking the "About FNB + Legal" and "Legal Matters" links.
If you are unable to access our website, please contact us to send you a copy 
of the email disclaimer or privacy notice.