Re: Handle infinite recursion in logical replication setup

2022-08-28 Thread Peter Smith
Here are some review comments for patch v42-0002: == 1. doc/src/sgml/logical-replication.sgml copy_data = true There are a couple of these tags where there is a trailing space before the . I guess it is doing no harm, but it is doing no good either, so IMO better to get rid of the space.

Re: Making Vars outer-join aware

2022-08-28 Thread Richard Guo
On Fri, Aug 19, 2022 at 2:45 AM Tom Lane wrote: > Here's a rebase up to HEAD, mainly to get the cfbot back in sync > as to what's the live patch. Noticed another different behavior from previous. When we try to reduce JOIN_LEFT to JOIN_ANTI, we want to know if the join's own quals are strict fo

Re: Handle infinite recursion in logical replication setup

2022-08-28 Thread Peter Smith
Here are some review comments for patch v42-0001: == 1. Commit message A later review comment below suggests some changes to the WARNING message so if those changes are made then the example in this commit message also needs to be modified. == 2. doc/src/sgml/ref/alter_subscription.sgm

Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns

2022-08-28 Thread Amit Kapila
On Sat, Aug 27, 2022 at 7:06 PM Masahiko Sawada wrote: > > On Sat, Aug 27, 2022 at 7:24 PM Amit Kapila wrote: > > > > On Sat, Aug 27, 2022 at 1:06 PM Masahiko Sawada > > wrote: > > > > > > > > > > > > > I think then we should change this code in the master branch patch > > > > > with an additio

Support tls-exporter as channel binding for TLSv1.3

2022-08-28 Thread Michael Paquier
Hi all, RFC9266, that has been released not so long ago, has added tls-exporter as a new channel binding type: https://www.rfc-editor.org/rfc/rfc5929.html An advantage over tls-server-end-point, AFAIK, is that this prevents man-in-the-middle attacks even if the attacker holds the server's private

Re: use ARM intrinsics in pg_lfind32() where available

2022-08-28 Thread Nathan Bossart
On Mon, Aug 29, 2022 at 11:25:50AM +0700, John Naylor wrote: > + uint32 nelem_per_vector = sizeof(Vector32) / sizeof(uint32); > + uint32 nelem_per_iteration = 4 * nelem_per_vector; > > Using local #defines would be my style. I don't have a reason to > object to this way, but adding const makes the

Re: Reducing the chunk header sizes on all memory context types

2022-08-28 Thread David Rowley
On Mon, 29 Aug 2022 at 10:39, David Rowley wrote: > One more try to make CFbot happy. After a bit more revision, mostly updating outdated comments and naming adjustments, I've pushed this. Per the benchmark results I showed in [1], due to the performance of having the AllocSet free list pointers

Re: Insertion Sort Improvements

2022-08-28 Thread John Naylor
On Fri, Aug 26, 2022 at 9:06 PM Benjamin Coutu wrote: > > Another idea could be to run a binary insertion sort and use a much higher > threshold. This could significantly cut down on comparisons (especially with > presorted runs, which are quite common in real workloads). Comparisons that must

Re: Removing unneeded self joins

2022-08-28 Thread Andrey Lepikhov
On 8/29/22 04:39, Zhihong Yu wrote: On Fri, Aug 26, 2022 at 3:02 PM Zhihong Yu > wrote: Hi, For v36-0001-Remove-self-joins.patch : bq removes inner join of plane table to itself plane table -> plain table For relation_has_unique_index_ext(), it

Re: privileges for ALTER ROLE/DATABASE SET

2022-08-28 Thread Noah Misch
On Fri, Jul 22, 2022 at 03:25:16PM -0700, Nathan Bossart wrote: > On Fri, Jul 22, 2022 at 04:16:14PM -0400, Tom Lane wrote: > > Clearly, you need enough privilege to SET the parameter, and you need > > some sort of management privilege on the target role or DB. There > > might be room to discuss w

Re: use ARM intrinsics in pg_lfind32() where available

2022-08-28 Thread Tom Lane
John Naylor writes: > I wonder if we should explain briefly what saturating arithmetic is. I > had never encountered it outside of a SIMD programming context. +1, it's at least worth a sentence to define the term. regards, tom lane

Re: use ARM intrinsics in pg_lfind32() where available

2022-08-28 Thread John Naylor
On Sun, Aug 28, 2022 at 10:58 AM Nathan Bossart wrote: > > Here is a new patch set in which I've attempted to address all feedback. Looks in pretty good shape. Some more comments: + uint32 nelem_per_vector = sizeof(Vector32) / sizeof(uint32); + uint32 nelem_per_iteration = 4 * nelem_per_vector;

Re: standby promotion can create unreadable WAL

2022-08-28 Thread Kyotaro Horiguchi
At Mon, 29 Aug 2022 13:13:52 +0900 (JST), Kyotaro Horiguchi wrote in > we return it to the caller. Is it worth to do a small refactoring > like the attached? If no, I'm fine with the proposed patch including > the added assertion. Mmm. That seems wrong. So forget about that. The proposed pat

Re: standby promotion can create unreadable WAL

2022-08-28 Thread Kyotaro Horiguchi
At Sun, 28 Aug 2022 10:16:21 +0530, Dilip Kumar wrote in > On Fri, Aug 26, 2022 at 7:53 PM Robert Haas wrote: > > v2 attached. > > The patch LGTM, this patch will apply on master and v15. PFA patch > for back branches. StandbyMode is obviously wrong. On the other hand I thought that !Archiv

patch: Add missing descriptions for rmgr APIs

2022-08-28 Thread kuroda.hay...@fujitsu.com
Hi hackers, While reading codes related with logical decoding, I thought that following comment in rmgrlist.h is not consistent. > /* symbol name, textual name, redo, desc, identify, startup, cleanup */ This comment describes a set of APIs that the resource manager should have, but functions fo

Re: Handle infinite recursion in logical replication setup

2022-08-28 Thread Amit Kapila
On Mon, Aug 29, 2022 at 8:24 AM vignesh C wrote: > > On Fri, Aug 26, 2022 at 9:52 AM Dilip Kumar wrote: > > > > IMHO, since the user has specifically asked for origin=NONE but we do > > not have any way to detect the origin during initial sync so I think > > this could be documented and we can al

Re: Handle infinite recursion in logical replication setup

2022-08-28 Thread vignesh C
On Fri, Aug 26, 2022 at 9:52 AM Dilip Kumar wrote: > > On Mon, Aug 22, 2022 at 9:19 AM houzj.f...@fujitsu.com > wrote: > > > > > Jonathan, Sawada-San, Hou-San, and others, what do you think is the best > > > way > > > to move forward here? > > > > I think it's fine to throw a WARNING in this cas

Re: [PATCH] Add native windows on arm64 support

2022-08-28 Thread Michael Paquier
On Sun, Aug 28, 2022 at 11:51:19AM -0400, Tom Lane wrote: > I have no idea either. I agree there *shouldn't* be any connection, > so if ASLR is somehow triggering this then whatever is failing is > almost certainly buggy on its own terms. But there's a lot of > moving parts here (mumble libxml mu

Re: Fix possible bogus array out of bonds (src/backend/access/brin/brin_minmax_multi.c)

2022-08-28 Thread Kyotaro Horiguchi
At Fri, 26 Aug 2022 10:28:50 -0300, Ranier Vilela wrote in > At function has_matching_range, if variable ranges->nranges == 0, > we exit quickly with a result equal to false. > > This means that nranges can be zero. > It occurs then that it is possible then to occur an array out of bonds, in >

Re: Removing unneeded self joins

2022-08-28 Thread Zhihong Yu
On Fri, Aug 26, 2022 at 3:02 PM Zhihong Yu wrote: > Hi, > For v36-0001-Remove-self-joins.patch : > > bq removes inner join of plane table to itself > > plane table -> plain table > > For relation_has_unique_index_ext(), it seems when extra_clauses is NULL, > there is no need to compute `exprs`. >

Re: effective_multixact_freeze_max_age issue

2022-08-28 Thread Nathan Bossart
On Sun, Aug 28, 2022 at 11:38:09AM -0700, Peter Geoghegan wrote: > On Tue, Aug 2, 2022 at 4:12 PM Peter Geoghegan wrote: >> That is, we only need to make sure that the "multiXactCutoff must >> always be <= oldestMxact" invariant holds once, by checking for it >> directly. The same thing happens wi

Re: Cleaning up historical portability baggage

2022-08-28 Thread Thomas Munro
On Mon, Aug 29, 2022 at 9:40 AM Tom Lane wrote: > Here's another bit of baggage handling: fixing up the places that > were afraid to use fflush(NULL). We could doubtless have done > this years ago (indeed, I found several places already using it) > but as long as we're making a push to get rid of

Re: Reducing the chunk header sizes on all memory context types

2022-08-28 Thread David Rowley
On Sun, 28 Aug 2022 at 23:04, David Rowley wrote: > I'll try that one again... One more try to make CFbot happy. David From 118022b089279ea7a6b1d43ca9a105a3203905e3 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 9 Jun 2022 20:09:22 +1200 Subject: [PATCH v8] Improve performance of and re

Re: Slight refactoring of state check in pg_upgrade check_ function

2022-08-28 Thread Nathan Bossart
On Sun, Aug 28, 2022 at 10:42:24PM +0200, Daniel Gustafsson wrote: > I noticed that the pg_upgrade check_ functions were determining failures found > in a few different ways. Some keep a boolen flag variable, and some (like > check_for_incompatible_polymorphics) check the state of the script fileh

Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

2022-08-28 Thread Nathan Bossart
On Sat, Aug 27, 2022 at 02:06:32PM +0530, Bharath Rupireddy wrote: > PSA v17 patch with reorderbuffer.c changes reverted. LGTM -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: Cleaning up historical portability baggage

2022-08-28 Thread Tom Lane
Here's another bit of baggage handling: fixing up the places that were afraid to use fflush(NULL). We could doubtless have done this years ago (indeed, I found several places already using it) but as long as we're making a push to get rid of obsolete code, doing it now seems appropriate. One thin

Re: add checkpoint stats of snapshot and mapping files of pg_logical dir

2022-08-28 Thread Nathan Bossart
On Thu, Aug 25, 2022 at 10:12:00AM +0530, Bharath Rupireddy wrote: > Please review the v12 patch attached. LGTM. I've marked this as ready-for-committer. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: CI and test improvements

2022-08-28 Thread Andres Freund
Hi, On 2022-08-28 12:10:29 -0500, Justin Pryzby wrote: > On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote: > > > --- /dev/null > > > +++ b/src/tools/ci/windows-compiler-warnings > > > > Wouldn't that be doable as something like > > sh -c 'if test -s file; then cat file;exit 1; fi" > >

Re: replacing role-level NOINHERIT with a grant-level option

2022-08-28 Thread Nathan Bossart
On Fri, Aug 26, 2022 at 02:46:59PM -0400, Robert Haas wrote: > - membership is via admin which has the > NOINHERIT > - attribute. After: > + membership is via admin which was granted using > + WITH INHERIT FALSE After: nitpick: I believe there should be a period before "After." Otherwis

Slight refactoring of state check in pg_upgrade check_ function

2022-08-28 Thread Daniel Gustafsson
I noticed that the pg_upgrade check_ functions were determining failures found in a few different ways. Some keep a boolen flag variable, and some (like check_for_incompatible_polymorphics) check the state of the script filehandle which is guaranteed to be set (with the error message referring to

Re: [RFC] building postgres with meson - v12

2022-08-28 Thread Andres Freund
Hi, On 2022-08-28 12:08:07 -0500, Justin Pryzby wrote: > with_temp_install is repeated twice in prove_check: > > > Subject: [PATCH v12 02/15] Split TESTDIR into TESTLOGDIR and TESTDATADIR > > > > - TESTDIR='$(CURDIR)/tmp_check' $(with_temp_install) > > PGPORT='6$(DEF_PGPORT)' \ > > + TESTL

Re: effective_multixact_freeze_max_age issue

2022-08-28 Thread Peter Geoghegan
On Tue, Aug 2, 2022 at 4:12 PM Peter Geoghegan wrote: > That is, we only need to make sure that the "multiXactCutoff must > always be <= oldestMxact" invariant holds once, by checking for it > directly. The same thing happens with OldestXmin/FreezeLimit. That > seems like a simpler foundation. It'

Re: Expand palloc/pg_malloc API

2022-08-28 Thread Peter Eisentraut
On 12.08.22 09:31, Peter Eisentraut wrote: In talloc, the talloc() function itself allocates an object of a given type.  To allocate something of a specified size, you'd use talloc_size().  So those names won't map exactly.  I'm fine with palloc_object() if that is clearer. I think the _ptrty

Re: CI and test improvements

2022-08-28 Thread Justin Pryzby
On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote: > > --- /dev/null > > +++ b/src/tools/ci/windows-compiler-warnings > > Wouldn't that be doable as something like > sh -c 'if test -s file; then cat file;exit 1; fi" > inside .cirrus.yml? I had written it inline in a couple ways, like

Re: [RFC] building postgres with meson - v12

2022-08-28 Thread Justin Pryzby
with_temp_install is repeated twice in prove_check: > Subject: [PATCH v12 02/15] Split TESTDIR into TESTLOGDIR and TESTDATADIR > > > - TESTDIR='$(CURDIR)/tmp_check' $(with_temp_install) > PGPORT='6$(DEF_PGPORT)' \ > + TESTLOGDIR='$(CURDIR)/tmp_check/log' $(with_temp_install) \ >

Re: CI and test improvements

2022-08-28 Thread Andres Freund
Hi, On 2022-08-28 09:44:47 -0500, Justin Pryzby wrote: > On Sat, May 28, 2022 at 10:37:41AM -0500, Justin Pryzby wrote: > > I'm anticipating the need to further re-arrange the patch set - it's not > > clear > > which patches should go first. Maybe some patches should be dropped in > > favour >

Convert *GetDatum() and DatumGet*() macros to inline functions

2022-08-28 Thread Peter Eisentraut
I once wrote code like this: char *oid = get_from_somewhere(); ... values[i++] = ObjectIdGetDatum(oid); This compiles cleanly and even appears to work in practice, except of course it doesn't. The FooGetDatum() macros just cast whatever you give it to Datum, without checking wh

Re: [PATCH] Add native windows on arm64 support

2022-08-28 Thread Tom Lane
Andres Freund writes: > On 2022-08-28 10:09:53 -0400, Tom Lane wrote: >> -x | pre&deeppost >> +x | pre&deeppost > Pretty weird, agreed. But hard to see how it could be caused by the > randomization change, except that perhaps it could highlight a preexisting > bug? I have no idea either. I agre

Re: [PATCH] Add native windows on arm64 support

2022-08-28 Thread Andres Freund
Hi, On 2022-08-28 10:09:53 -0400, Tom Lane wrote: > Michael Paquier writes: > > True enough. I have applied the patch to re-enable that on HEAD. > > Let's now see what happens in the next couple of days. Popcorn is > > ready here. > > Hmm: > > https://buildfarm.postgresql.org/cgi-bin/show_log.p

Re: CI and test improvements

2022-08-28 Thread Justin Pryzby
Resending with a problematic address removed... On Sat, May 28, 2022 at 10:37:41AM -0500, Justin Pryzby wrote: > I'm anticipating the need to further re-arrange the patch set - it's not clear > which patches should go first. Maybe some patches should be dropped in favour > of the meson project.

Re: [PATCH] Add native windows on arm64 support

2022-08-28 Thread Tom Lane
Michael Paquier writes: > True enough. I have applied the patch to re-enable that on HEAD. > Let's now see what happens in the next couple of days. Popcorn is > ready here. Hmm: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2022-08-28%2010%3A30%3A29 Maybe that's just un

Re: Reducing the chunk header sizes on all memory context types

2022-08-28 Thread David Rowley
On Fri, 26 Aug 2022 at 17:16, David Rowley wrote: > The CFbot just alerted me to the cplusplus check was failing with the > v5 patch, so here's v6. I'll try that one again... David From 90e48eef5709e8cdfb474f8798fa0aef1d0158b1 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 9 Jun 2022 20

Re: Change pfree to accept NULL argument

2022-08-28 Thread Peter Eisentraut
On 22.08.22 20:30, Tom Lane wrote: I'm not very convinced that the benefits of making pfree() more like free() are worth those costs. We could ameliorate the first objection if we wanted to back-patch 0002, I guess. (FWIW, no objection to your 0001. 0004 and 0005 seem okay too; they don't touc

Re: [PATCH] Add native windows on arm64 support

2022-08-28 Thread Michael Paquier
On Sat, Aug 27, 2022 at 12:27:57PM -0700, Andres Freund wrote: > I checked, and it looks like we didn't add --disable-dynamicbase, so ASLR > wasn't disabled for mingw builds. True enough. I have applied the patch to re-enable that on HEAD. Let's now see what happens in the next couple of days. P