Re: Improve description of XLOG_RUNNING_XACTS

2022-10-16 Thread Michael Paquier
On Mon, Oct 17, 2022 at 09:58:31AM +0530, Amit Kapila wrote: > Okay, let's wait for two or three days and see if anyone thinks > differently, otherwise, I'll push v3 after a bit more testing. No objections from here if you want to go ahead with v3 and print the full set of subxids on top of the

RE: Data is copied twice when specifying both child and parent table in publication

2022-10-16 Thread wangw.f...@fujitsu.com
On Wed, Oct 5, 2022 at 23:05 PM Osumi, Takamichi/大墨 昂道 wrote: > Hi, thank you for the updated patches! > > > Here are my minor review comments for HEAD v12. Thanks for your comments. > (1) typo & suggestion to reword one comment > > > +* Publications support

Re: archive modules

2022-10-16 Thread Michael Paquier
On Mon, Oct 17, 2022 at 01:46:39PM +0900, Kyotaro Horiguchi wrote: > As the code written, when archive library is being added while archive > command is already set, archiver first emits seemingly positive > message "restarting archive process because of..", then errors out > after the resatart

RE: Data is copied twice when specifying both child and parent table in publication

2022-10-16 Thread wangw.f...@fujitsu.com
On Wed, Oct 5, 2022 at 11:08 AM Peter Smith wrote: > Hi Wang-san. Here are my review comments for HEAD_v12-0001 patch. Thanks for your comments. > == > > 1. Missing documentation. > > In [1] you wrote: > > I think the behaviour of multiple publications with parameter >

Re: fix archive module shutdown callback

2022-10-16 Thread Michael Paquier
On Mon, Oct 17, 2022 at 02:30:52PM +0900, Kyotaro Horiguchi wrote: > At Mon, 17 Oct 2022 13:51:52 +0900, Michael Paquier > wrote in >> I am not sure to understand what you mean here. The shutdown callback >> is available once the archiver process has loaded the library defined >> in

Re: fix archive module shutdown callback

2022-10-16 Thread Kyotaro Horiguchi
At Sun, 16 Oct 2022 13:39:14 +0530, Bharath Rupireddy wrote in > On Sun, Oct 16, 2022 at 3:43 AM Nathan Bossart > wrote: > > > > Hi hackers, > > > > Presently, when an archive module sets up a shutdown callback, it will be > > called upon ERROR/FATAL (via PG_ENSURE_ERROR_CLEANUP), when the

Re: fix archive module shutdown callback

2022-10-16 Thread Kyotaro Horiguchi
At Mon, 17 Oct 2022 13:51:52 +0900, Michael Paquier wrote in > On Sun, Oct 16, 2022 at 01:39:14PM +0530, Bharath Rupireddy wrote: > > Is the shutdown callback meant to be called only after the archive > > library is loaded? The documentation [1] says that it just gets called > > before the

Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

2022-10-16 Thread Maciek Sakrejda
On Thu, Oct 13, 2022 at 10:29 AM Melanie Plageman wrote: > I think that it makes sense to count both the initial buffers added to > the ring and subsequent shared buffers added to the ring (either when > the current strategy buffer is pinned or in use or when a bulkread > rejects dirty strategy

Re: fix archive module shutdown callback

2022-10-16 Thread Michael Paquier
On Sun, Oct 16, 2022 at 01:39:14PM +0530, Bharath Rupireddy wrote: > Is the shutdown callback meant to be called only after the archive > library is loaded? The documentation [1] says that it just gets called > before the archiver process exits. If this is true, can we just place >

Re: archive modules

2022-10-16 Thread Kyotaro Horiguchi
At Fri, 14 Oct 2022 14:42:56 -0700, Nathan Bossart wrote in > As promised... As the code written, when archive library is being added while archive command is already set, archiver first emits seemingly positive message "restarting archive process because of..", then errors out after the

Re: Improve description of XLOG_RUNNING_XACTS

2022-10-16 Thread Amit Kapila
On Mon, Oct 17, 2022 at 6:46 AM Kyotaro Horiguchi wrote: > > At Sun, 16 Oct 2022 12:32:56 +0530, Bharath Rupireddy > wrote in > > On Sat, Oct 15, 2022 at 4:58 PM Amit Kapila wrote: > > > > > > > I spent some time today reading this. As others said upthread, the > > > > output can be more

Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

2022-10-16 Thread Amit Kapila
On Mon, Oct 17, 2022 at 8:39 AM houzj.f...@fujitsu.com wrote: > > While working on some logical replication related features. > I found the HINT message could be improved when I tried to add a publication > to > a subscription which was disabled. > > alter subscription sub add publication pub2;

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/16/22 22:17, Matthias van de Meent wrote: > First of all, it's really great to see that this is being worked on. > > On Sun, 16 Oct 2022 at 16:34, Tomas Vondra > wrote: >> Try to formulate the whole algorithm. Maybe I'm missing something. >> >> The current algorithm is something like this:

Re: Schema variables - new implementation for Postgres 15

2022-10-16 Thread Julien Rouhaud
Hi, On Thu, Oct 13, 2022 at 07:41:32AM +0200, Pavel Stehule wrote: > > > I fixed the commit message of 0001 patch. Fixed shadowed variables too. Thanks! > > > > There is a partially open issue, where I and Julien are not sure about a > > solution, and we would like to ask for the community's

Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

2022-10-16 Thread houzj.f...@fujitsu.com
Hi, While working on some logical replication related features. I found the HINT message could be improved when I tried to add a publication to a subscription which was disabled. alter subscription sub add publication pub2; -- ERROR: ALTER SUBSCRIPTION with refresh is not allowed for disabled

RE: [Proposal] Add foreign-server health checks infrastructure

2022-10-16 Thread osumi.takami...@fujitsu.com
On Wednesday, October 5, 2022 6:27 PM kuroda.hay...@fujitsu.com wrote: > Thanks for giving many comments! Based on off and on discussions, I modified > my patch. Here are my other quick review comments on v16. (1) v16-0001 : definition of a new structure CheckingRemoteServersCallbackItem can

Re: Unnecessary lateral dependencies implied by PHVs

2022-10-16 Thread Richard Guo
On Mon, Oct 10, 2022 at 10:35 AM Richard Guo wrote: > As we know when we pull up a simple subquery, if the subquery is within > the nullable side of an outer join, lateral references to non-nullable > items may have to be turned into PlaceHolderVars. I happened to wonder > what should we do

Re: thinko in basic_archive.c

2022-10-16 Thread Michael Paquier
On Sat, Oct 15, 2022 at 02:10:26PM -0700, Nathan Bossart wrote: > On Sat, Oct 15, 2022 at 10:19:05AM +0530, Bharath Rupireddy wrote: >> Can you please help me understand how name collisions can happen with >> temp file names including WAL file name, timestamp to millisecond >> scale, and PID?

Re: [PATCH] comment fixes for delayChkptFlags

2022-10-16 Thread Michael Paquier
On Fri, Oct 14, 2022 at 04:36:36PM -0500, David Christensen wrote: > Enclosed is a trivial fix for a typo and misnamed field I noted when doing > some code review. (Few days after the fact) Thanks. There was a second location where delayChkpt was still mentioned, so fixed also that while on it.

Re: Add regular expression testing for user name mapping in the peer authentication TAP test

2022-10-16 Thread Michael Paquier
On Sat, Oct 15, 2022 at 07:54:30AM +0200, Drouvot, Bertrand wrote: > Right. Giving a second thought to the non matching case, I think I'd prefer > to concatenate the system_user to the system_user instead. This is what v2 > does. Fine by me, so applied v2. Thanks! -- Michael signature.asc

Re: Fix error message for MERGE foreign tables

2022-10-16 Thread Richard Guo
On Fri, Oct 14, 2022 at 10:43 PM Tom Lane wrote: > Richard Guo writes: > > Or maybe we can make it even earlier, when we expand an RTE for a > > partitioned table and add result tables to leaf_result_relids. > > I'm not really on board with injecting command-type-specific logic into >

Re: create subscription - improved warning message

2022-10-16 Thread Peter Smith
On Sun, Oct 16, 2022 at 12:14 AM Amit Kapila wrote: > > On Fri, Oct 14, 2022 at 8:22 AM Peter Smith wrote: > > > > On Thu, Oct 13, 2022 at 9:07 AM Peter Smith wrote: > > > ... > > PSA a patch for adding examples of how to activate a subscription that > > was originally created in a disconnected

RE: [Proposal] Add foreign-server health checks infrastructure

2022-10-16 Thread osumi.takami...@fujitsu.com
Hi, On Wednesday, October 5, 2022 6:27 PM kuroda.hay...@fujitsu.com wrote: > Thanks for giving many comments! Based on off and on discussions, I modified > my patch. Thank you for your patch set ! While reviewing and testing the new v16, I've met a possible issue by slightly adjusting the

Re: TRAP: FailedAssertion("prev_first_lsn < cur_txn->first_lsn", File: "reorderbuffer.c", Line: 927, PID: 568639)

2022-10-16 Thread Masahiko Sawada
On Thu, Oct 13, 2022 at 4:08 PM Amit Kapila wrote: > > On Wed, Oct 12, 2022 at 11:18 AM Masahiko Sawada > wrote: > > > > Summarizing this issue, the assertion check in AssertTXNLsnOrder() > > fails as reported because the current logical decoding cannot properly > > handle the case where the

Re: Improve description of XLOG_RUNNING_XACTS

2022-10-16 Thread Kyotaro Horiguchi
At Sun, 16 Oct 2022 12:32:56 +0530, Bharath Rupireddy wrote in > On Sat, Oct 15, 2022 at 4:58 PM Amit Kapila wrote: > > > > > I spent some time today reading this. As others said upthread, the > > > output can be more verbose if all the backends are running max > > > subtransactions or

Re: New "single-call SRF" APIs are very confusingly named

2022-10-16 Thread Michael Paquier
On Sun, Oct 16, 2022 at 03:09:14PM -0700, Andres Freund wrote: > On 2022-10-15 11:41:08 +0900, Michael Paquier wrote: >> -/* flag bits for SetSingleFuncCall() */ >> -#define SRF_SINGLE_USE_EXPECTED 0x01/* use expectedDesc as tupdesc >> */ >> -#define SRF_SINGLE_BLESS0x02/*

Re: New "single-call SRF" APIs are very confusingly named

2022-10-16 Thread Michael Paquier
On Sun, Oct 16, 2022 at 03:04:43PM -0700, Andres Freund wrote: > Yes - it'd introduce an ABI break, i.e. an already compiled extension > referencing SetSingleFuncCall() wouldn't fail to load into an upgraded sever, > due to the reference to the SetSingleFuncCall, which wouldn't exist anymore.

Re: macos ventura SDK spews warnings

2022-10-16 Thread Tom Lane
Andres Freund writes: > On 2022-10-16 16:45:24 -0400, Tom Lane wrote: >> What I *am* seeing, in the 9.5 and 9.6 branches, is a ton of >> >> ld: warning: -undefined dynamic_lookup may not work with chained fixups >> >> apparently because we are specifying -Wl,-undefined,dynamic_lookup >> which

Re: New "single-call SRF" APIs are very confusingly named

2022-10-16 Thread Andres Freund
Hi, On 2022-10-15 11:41:08 +0900, Michael Paquier wrote: > Attached is a patch for HEAD and REL_15_STABLE to switch this stuff with new > names, with what's needed for ABI compatibility. My plan would be to keep > the compatibility parts only in 15, and drop them from HEAD. -- Michael Looks

Re: New "single-call SRF" APIs are very confusingly named

2022-10-16 Thread Andres Freund
Hi, On 2022-10-16 13:22:41 -0700, Melanie Plageman wrote: > On Fri, Oct 14, 2022 at 7:41 PM Michael Paquier wrote: > - * SetSingleFuncCall > + * Compatibility function for v15. > + */ > +void > +SetSingleFuncCall(FunctionCallInfo fcinfo, bits32 flags) > +{ > + InitMaterializedSRF(fcinfo, flags);

Re: macos ventura SDK spews warnings

2022-10-16 Thread Andres Freund
Hi, On 2022-10-16 16:45:24 -0400, Tom Lane wrote: > I wrote: > > I think the correct, future-proof fix is to s/REF/REF_P/ in the > > grammar. > > Done like that, after which I found that the pre-v12 branches are > compiling perfectly warning-free with the 13.0 SDK, despite nothing > having been

Re: macos ventura SDK spews warnings

2022-10-16 Thread Tom Lane
I wrote: > I think the correct, future-proof fix is to s/REF/REF_P/ in the > grammar. Done like that, after which I found that the pre-v12 branches are compiling perfectly warning-free with the 13.0 SDK, despite nothing having been done about sprintf. This confused me mightily, but after digging

Re: New "single-call SRF" APIs are very confusingly named

2022-10-16 Thread Melanie Plageman
On Fri, Oct 14, 2022 at 7:41 PM Michael Paquier wrote: > On Fri, Oct 14, 2022 at 05:09:46PM -0400, Melanie Plageman wrote: > > To summarize, I am in support of renaming SetSingleFuncCall() -> > > InitMaterializedSRF() and SRF_SINGLE_USE_EXPECTED -> > > MAT_SRF_USE_EXPECTED_TUPLE_DESC (or just

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Matthias van de Meent
First of all, it's really great to see that this is being worked on. On Sun, 16 Oct 2022 at 16:34, Tomas Vondra wrote: > Try to formulate the whole algorithm. Maybe I'm missing something. > > The current algorithm is something like this: > > 1. request info about ranges from the BRIN opclass >

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Matthias van de Meent
On Sun, 16 Oct 2022 at 16:42, Tom Lane wrote: > > It also seems kind of unfair to decide > that the relevant comparison point is a seqscan rather than a > btree indexscan. I think the comparison against full table scan seems appropriate, as the benefit of BRIN is less space usage when compared

Re: macos ventura SDK spews warnings

2022-10-16 Thread Tom Lane
I wrote: > So I pushed (1), but on the way to testing (2), I discovered a totally > independent problem with the 13.0 SDK in older branches: > In file included from ../../../src/include/postgres.h:46: > In file included from ../../../src/include/c.h:1387: > In file included from

Re: macos ventura SDK spews warnings

2022-10-16 Thread Tom Lane
Andres Freund writes: > On 2022-10-15 21:00:00 -0400, Tom Lane wrote: >> After further thought, I think the best compromise is just that: >> >> (1) apply s/sprintf/snprintf/ patch in branches back to v12, where >> we began to require C99. >> >> (2) in v11 and back to 9.2, enable -Wno-deprecated

Re: allowing for control over SET ROLE

2022-10-16 Thread Stephen Frost
Greetings, * Nathan Bossart (nathandboss...@gmail.com) wrote: > On Fri, Sep 30, 2022 at 04:34:32PM -0400, Robert Haas wrote: > > That thread has not reached an entirely satisfying conclusion. > > However, the behavior that was deemed outright buggy over there has > > been fixed. The remaining

Re: use has_privs_of_role() for pg_hba.conf

2022-10-16 Thread Stephen Frost
Greetings, * Nathan Bossart (nathandboss...@gmail.com) wrote: > On Sat, Oct 08, 2022 at 11:46:50AM -0400, Robert Haas wrote: > > Now there may be some other scenario in which the patch is going in > > exactly the right direction, and if I knew what it was, maybe I'd > > agree that the patch was a

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/16/22 16:42, Zhihong Yu wrote: > ... > > I don't have good answer w.r.t. splitting the page range [0,127] now. > Let me think more about it. > Sure, no problem. > The 10 step flow (subject to changes down the road) should be either > given in the description of the patch or, written as

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/16/22 16:41, Tom Lane wrote: > Tomas Vondra writes: >> I forgot to mention one important issue in my list yesterday, and that's >> memory consumption. > > TBH, this is all looking like vastly more complexity than benefit. > It's going to be impossible to produce a reliable cost estimate

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Zhihong Yu
On Sun, Oct 16, 2022 at 7:33 AM Tomas Vondra wrote: > > > On 10/16/22 16:01, Zhihong Yu wrote: > > > > > > On Sun, Oct 16, 2022 at 6:51 AM Tomas Vondra > > mailto:tomas.von...@enterprisedb.com>> > > wrote: > > > > On 10/15/22 14:33, Tomas Vondra wrote: > > > Hi, > > > > > > ... >

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tom Lane
Tomas Vondra writes: > I forgot to mention one important issue in my list yesterday, and that's > memory consumption. TBH, this is all looking like vastly more complexity than benefit. It's going to be impossible to produce a reliable cost estimate given all the uncertainty, and I fear that will

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/16/22 16:01, Zhihong Yu wrote: > > > On Sun, Oct 16, 2022 at 6:51 AM Tomas Vondra > mailto:tomas.von...@enterprisedb.com>> > wrote: > > On 10/15/22 14:33, Tomas Vondra wrote: > > Hi, > > > > ... > > > > There's a bunch of issues with this initial version of the

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/16/22 03:36, Zhihong Yu wrote: > > > On Sat, Oct 15, 2022 at 8:23 AM Tomas Vondra > mailto:tomas.von...@enterprisedb.com>> > wrote: > > On 10/15/22 15:46, Zhihong Yu wrote: > >... > >     8) Parallel version is not supported, but I think it shouldn't be > >     

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Zhihong Yu
On Sun, Oct 16, 2022 at 6:51 AM Tomas Vondra wrote: > On 10/15/22 14:33, Tomas Vondra wrote: > > Hi, > > > > ... > > > > There's a bunch of issues with this initial version of the patch, > > usually described in XXX comments in the relevant places.6) > > > > ... > > I forgot to mention one

Re: PATCH: Using BRIN indexes for sorted output

2022-10-16 Thread Tomas Vondra
On 10/15/22 14:33, Tomas Vondra wrote: > Hi, > > ... > > There's a bunch of issues with this initial version of the patch, > usually described in XXX comments in the relevant places.6) > > ... I forgot to mention one important issue in my list yesterday, and that's memory consumption. The way

Re: fix archive module shutdown callback

2022-10-16 Thread Bharath Rupireddy
On Sun, Oct 16, 2022 at 3:43 AM Nathan Bossart wrote: > > Hi hackers, > > Presently, when an archive module sets up a shutdown callback, it will be > called upon ERROR/FATAL (via PG_ENSURE_ERROR_CLEANUP), when the archive > library changes (via HandlePgArchInterrupts()), and upon normal shutdown.

Re: archive modules

2022-10-16 Thread Bharath Rupireddy
On Sat, Oct 15, 2022 at 3:13 AM Nathan Bossart wrote: > > On Fri, Oct 14, 2022 at 11:51:30AM -0700, Nathan Bossart wrote: > > On Fri, Oct 14, 2022 at 12:10:18PM +0530, Bharath Rupireddy wrote: > >> 2) I think we have a problem - set archive_mode and archive_library > >> and start the server, then

Re: Improve description of XLOG_RUNNING_XACTS

2022-10-16 Thread Bharath Rupireddy
On Sat, Oct 15, 2022 at 4:58 PM Amit Kapila wrote: > > > I spent some time today reading this. As others said upthread, the > > output can be more verbose if all the backends are running max > > subtransactions or subtransactions overflow occurred in all the > > backends. > > > > As far as I can