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

On Mon, Oct 10, 2022 at 12:16 PM John Naylor wrote: > Thanks for that! Now I can show clear results on some aspects in a simple way. The attached patches (apply on top of v6) Forgot the patchset... -- John Naylor EDB: http://www.enterprisedb.com radix-v6-addendum-jcn1.tar.gz Description:

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

The following is not quite a full review, but has plenty to think about. There is too much to cover at once, and I have to start somewhere... My main concerns are that internal APIs: 1. are difficult to follow 2. lead to poor branch prediction and too many function calls Some of the

Re: create subscription - improved warning message

On Mon, Oct 10, 2022 at 10:10 AM Tom Lane wrote: > > Amit Kapila writes: > > Yeah, this message looks better than the current one. However, when I > > tried to do what HINT says, it doesn't initiate replication. It gives > > me the below error: > > > postgres=# Alter subscription sub1 refresh

Re: create subscription - improved warning message

Amit Kapila writes: > Yeah, this message looks better than the current one. However, when I > tried to do what HINT says, it doesn't initiate replication. It gives > me the below error: > postgres=# Alter subscription sub1 refresh publication; > ERROR: ALTER SUBSCRIPTION ... REFRESH is not

Re: create subscription - improved warning message

On Mon, Oct 10, 2022 at 4:40 AM Peter Smith wrote: > > But if it's OK to do that then: > - maybe it should mention the connection since the connect=false was > what caused this warning. > - maybe saying 'replication' instead of 'collection of data' would be > more consistent with the pgdocs for

Re: ps command does not show walsender's connected db

On Mon, Oct 10, 2022 at 8:00 AM bt22nakamorit wrote: > > appendStringInfo will append extra space after null (since "%s "), so > the ps entry will look less neat in that case. > How about we check whether port->database_name is null or not, instead > of making it unconditional? > It will look

Remove an unnecessary LSN calculation while validating WAL page header

Hi, It looks like we have an unnecessary XLogSegNoOffsetToRecPtr() in XLogReaderValidatePageHeader(). We pass the start LSN of the WAL page and check if it matches with the LSN that was stored in the WAL page header (xlp_pageaddr). We find segno, offset and LSN again using

Re: Allow file inclusion in pg_hba and pg_ident files

Hi, On Sun, Sep 18, 2022 at 01:06:12AM +0800, Julien Rouhaud wrote: > On Tue, Aug 16, 2022 at 02:10:30PM +0800, Julien Rouhaud wrote: > > > > On Sat, Jul 30, 2022 at 04:09:36PM +0800, Julien Rouhaud wrote: > > > > > > > > - 0001: the rule_number / mapping_number addition in the views in a > > >

Re: Support logical replication of DDLs

Hi, Please include all known information about how this feature looks from the user's POV. Ideally, this information should be in the form of PGDOCS updates included in patch 1. I think documenting these details should not be deferred - reviewers will want to experiment with the feature,

Unnecessary lateral dependencies implied by PHVs

Hi hackers, 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 about the PHVs if the outer join is reduced to inner

Re: ps command does not show walsender's connected db

2022-10-09 18:30 Bharath Rupireddy wrote: -if (!am_walsender) +if (!am_walsender || am_db_walsender) appendStringInfo(_data, "%s ", port->database_name); Can the appendStringInfo be just unconditional? That is more readable IMO. We want the database_name to be appended whenever

Re: Unify "In" Sublink to EXIST Sublink for better optimize opportunity

Hi: On Thu, Oct 6, 2022 at 3:24 PM Andy Fan wrote: > > Due to the implementation of convert_ANY_sublink_to_join, we have > limitations below, which has been discussed at [1] [2]. > > if (contain_vars_of_level((Node *) subselect, 1)) > return NULL; > > I'm thinking if we can do the

Re: create subscription - improved warning message

Peter Smith writes: > On Sat, Oct 8, 2022 at 2:23 AM Tom Lane wrote: >> I think maybe a better message would be along the lines of >> WARNING: subscription was created, but is not up-to-date >> HINT: You should now run %s to initiate collection of data. > [ how about ] > WARNING: subscription

Re: create subscription - improved warning message

On Sat, Oct 8, 2022 at 2:23 AM Tom Lane wrote: > > Peter Smith writes: > > WARNING: tables were not subscribed, you will have to run ALTER > > SUBSCRIPTION ... REFRESH PUBLICATION to subscribe the tables > > > When I first encountered the above CREATE SUBSCRIPTION warning message > > I thought

Re: Switching XLog source from archive to streaming when primary available

On Sun, Oct 09, 2022 at 02:39:47PM +0530, Bharath Rupireddy wrote: > We can give it a chance to restore from pg_wal before switching to > streaming to not change any behaviour of the state machine. But, not > definitely by setting currentSource to XLOG_FROM_WAL, we basically > never explicitly set

Re: Simplifying our Trap/Assert infrastructure

On Sun, Oct 09, 2022 at 05:08:39PM -0400, Tom Lane wrote: > Something I thought about but forgot to mention in the initial email: > is it worth sprinkling these macros with "unlikely()"? I think that > compilers might assume the right thing automatically based on noticing > that

Re: use has_privs_of_role() for pg_hba.conf

On Sun, Oct 09, 2022 at 10:19:51AM +0900, Michael Paquier wrote: > Even if the patch is at the end rejected, I think that the test is > still useful once you switch its logic to use membership and not > inherited privileges for the roles created, and there is zero coverage > for "samplegroup" and

Re: Simplifying our Trap/Assert infrastructure

Nathan Bossart writes: > On Sun, Oct 09, 2022 at 03:51:57PM -0400, Tom Lane wrote: >> Hence, I propose the attached. > The patch LGTM. It might be worth removing usages of AssertArg and > AssertState, too, but that can always be done separately. Something I thought about but forgot to mention

Re: Simplifying our Trap/Assert infrastructure

On Sun, Oct 09, 2022 at 03:51:57PM -0400, Tom Lane wrote: > I happened to notice that the Trap and TrapMacro macros defined in c.h > have a grand total of one usage apiece across our entire code base. > It seems a little pointless and confusing to have them at all, since > they're essentially

Simplifying our Trap/Assert infrastructure

I happened to notice that the Trap and TrapMacro macros defined in c.h have a grand total of one usage apiece across our entire code base. It seems a little pointless and confusing to have them at all, since they're essentially Assert/AssertMacro but with the inverse condition polarity. I'm also

Re: ps command does not show walsender's connected db

On Fri, Oct 7, 2022 at 7:25 PM Fujii Masao wrote: > > Thanks for updating the patch! LGTM. -if (!am_walsender) +if (!am_walsender || am_db_walsender) appendStringInfo(_data, "%s ", port->database_name); Can the appendStringInfo be just unconditional? That is more readable IMO.

Re: [BUG] Logical replica crash if there was an error in a function.

Hello! Thanks for reply! On 24.09.2022 20:27, Tom Lane wrote: I think you're solving the problem in the wrong place. The real issue is why are we not setting up ActivePortal correctly when running user-defined code in a logrep worker? During a common query from the backend ActivePortal

Re: Switching XLog source from archive to streaming when primary available

On Sun, Oct 9, 2022 at 3:22 AM Nathan Bossart wrote: > > As I mentioned upthread [0], I'm still a little concerned that this patch > will cause the state machine to go straight from archive recovery to > streaming replication, skipping recovery from pg_wal. > > [0]