Re: relation OID in ReorderBufferToastReplace error message

2021-09-16 Thread Amit Kapila
On Thu, Jul 15, 2021 at 6:14 AM Jeremy Schneider wrote: > > On 7/2/21 18:57, Jeremy Schneider wrote: > > The process of trying to understand this recent incident has given me some > new insight about what information would be helpful up front in this error > message for faster resolution. > >

RE: Column Filtering in Logical Replication

2021-09-16 Thread houzj.f...@fujitsu.com
On Thurs, Sep 16, 2021 10:37 PM Alvaro Herrera wrote: > On 2021-Sep-16, Alvaro Herrera wrote: > > Actually, something like this might be better: > > > PublicationObjSpec: > > > | TABLE qualified_name > > { > >

Re: Logical replication keepalive flood

2021-09-16 Thread Amit Kapila
On Thu, Aug 12, 2021 at 8:03 AM Peter Smith wrote: > > That base data is showing there are similar numbers of keepalives sent > as there are calls made to WalSndWaitForWal. IIUC it means that mostly > the loop is sending the special keepalives on the *first* iteration, > but by the time of the

Re: walsender timeout on logical replication set

2021-09-16 Thread Amit Kapila
On Mon, Sep 13, 2021 at 7:01 AM Kyotaro Horiguchi wrote: > > Hello. > > As reported in [1] it seems that walsender can suffer timeout in > certain cases. It is not clearly confirmed, but I suspect that > there's the case where LogicalRepApplyLoop keeps running the innermost > loop without

Re: Improve logging when using Huge Pages

2021-09-16 Thread Kyotaro Horiguchi
At Fri, 17 Sep 2021 00:13:41 +0900, Fujii Masao wrote in > > > On 2021/09/08 11:17, Kyotaro Horiguchi wrote: > > I don't dislike the message, but I'm not sure I like the message is > > too verbose, especially about it has DETAILS. It seems to me something > > like the following is sufficient,

Re: Column Filtering in Logical Replication

2021-09-16 Thread vignesh C
On Thu, Sep 16, 2021 at 7:20 PM Alvaro Herrera wrote: > > On 2021-Sep-16, vignesh C wrote: > > > diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y > > index e3068a374e..c50bb570ea 100644 > > --- a/src/backend/parser/gram.y > > +++ b/src/backend/parser/gram.y > > Yeah, on a quick

Re: Split xlog.c

2021-09-16 Thread Kyotaro Horiguchi
Hello. At Thu, 16 Sep 2021 11:23:46 +0300, Heikki Linnakangas wrote in > Here is another rebase. I have several comments on this. 0001: I understand this is almost simple relocation of code fragments. But it seems introducing some behavioral changes. PublishStartProcessInformation()

Re: Refactoring postgres_fdw code to rollback remote transaction

2021-09-16 Thread Fujii Masao
On 2021/09/17 11:40, Zhihong Yu wrote: +           goto fail;          /* Trouble clearing prepared statements */ The label fail can be removed. Under the above condition,  entry->changing_xact_state is still true. You can directly return. Thanks for the review! Yes, you're right. Attached

Re: Refactoring postgres_fdw code to rollback remote transaction

2021-09-16 Thread Zhihong Yu
On Thu, Sep 16, 2021 at 7:31 PM Fujii Masao wrote: > Hi, > > In postgres_fdw, pgfdw_xact_callback() and pgfdw_subxact_callback() do > almost the same thing to rollback remote toplevel- and sub-transaction. > But their such rollback logics are implemented separately and > in different way. Which

Refactoring postgres_fdw code to rollback remote transaction

2021-09-16 Thread Fujii Masao
Hi, In postgres_fdw, pgfdw_xact_callback() and pgfdw_subxact_callback() do almost the same thing to rollback remote toplevel- and sub-transaction. But their such rollback logics are implemented separately and in different way. Which would decrease the readability and maintainability, I think. So

Re: Estimating HugePages Requirements?

2021-09-16 Thread Michael Paquier
On Thu, Sep 16, 2021 at 09:26:56PM +, Bossart, Nathan wrote: > I'm not sure I agree on this one. The documentation for huge_pages > [0] and shared_memory_type [1] uses the same phrasing multiple times, > and the new shared_memory_size GUC uses it as well [2]. I don't see > anything in the

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Andres Freund
Hi, On 2021-09-16 16:38:04 +0200, Magnus Hagander wrote: > On Wed, Sep 15, 2021 at 10:48 PM Andres Freund wrote: > > > > On 2021-09-15 10:47:33 -0400, Andrew Dunstan wrote: > > > this is an open item for release 14. Is someone going to commit? > > > > Will do. Although I do wish the original

Re: Add jsonlog log_destination for JSON server logs

2021-09-16 Thread Michael Paquier
On Thu, Sep 16, 2021 at 05:27:20PM -0400, Sehrope Sarkuni wrote: > Attached three patches refactor the syslogger handling of file based > destinations a bit ... and then a lot. > > First patch adds a new constant to represent both file based destinations. > This should make it easier to ensure

Re: testing with pg_stat_statements

2021-09-16 Thread Julien Rouhaud
On Fri, Sep 17, 2021 at 3:08 AM Andrew Dunstan wrote: > > So I tested it > out with a modified buildfarm client with this line added in the initdb > step where it's adding the extra_config to postgresql.conf: > > print $handle "shared_preload_libraries = 'pg_stat_statements'\n"; > > The good

Re: Add jsonlog log_destination for JSON server logs

2021-09-16 Thread Sehrope Sarkuni
The previous patches failed on the cfbot Appveyor Windows build. That also makes me question whether I did the EXEC_BACKEND testing correctly as it should have caught that compile error. I'm going to look into that. In the meantime the attached should correct that part of the build. Regards, --

Re: Possible fault with resolve column name (plpgsql)

2021-09-16 Thread Ranier Vilela
Em qui., 16 de set. de 2021 às 17:05, Tom Lane escreveu: > Ranier Vilela writes: > > Found by llvm scan build. > > Argument with 'nonnull' attribute passed null pl/plpgsql/src/pl_comp.c > > resolve_column_ref > > This is somewhere between pointless and counterproductive. Not if you've ever

Re: Add jsonlog log_destination for JSON server logs

2021-09-16 Thread Sehrope Sarkuni
Attached three patches refactor the syslogger handling of file based destinations a bit ... and then a lot. First patch adds a new constant to represent both file based destinations. This should make it easier to ensure additional destinations are handled in "For all file destinations..."

Re: Possible fault with resolve column name (plpgsql)

2021-09-16 Thread Tom Lane
Ranier Vilela writes: > Found by llvm scan build. > Argument with 'nonnull' attribute passed null pl/plpgsql/src/pl_comp.c > resolve_column_ref This is somewhere between pointless and counterproductive. colname won't be used unless the switch has set nnames_field (and the identified number of

Possible fault with resolve column name (plpgsql)

2021-09-16 Thread Ranier Vilela
Hi, Found by llvm scan build. Argument with 'nonnull' attribute passed null pl/plpgsql/src/pl_comp.c resolve_column_ref Proceed? regards, Ranier Vilela fix_possible_fault_resolve_columnname_plpgsql.patch Description: Binary data

testing with pg_stat_statements

2021-09-16 Thread Andrew Dunstan
After the recent case where the SQL/JSON patches had an error that only exhibited when pg_stat_statement was preloaded, I decided to see if there were any other such cases in our committed code. So I tested it out with a modified buildfarm client with this line added in the initdb step where it's

Re: [PATCH] support tab-completion for single quote input with equal sign

2021-09-16 Thread Kyotaro Horiguchi
At Sat, 04 Sep 2021 10:18:24 -0400, Tom Lane wrote in > I kind of wonder if it isn't time to enlist the help of psqlscan.l > instead of doubling down on the idea that tab-complete.c should have > its own half-baked SQL lexer. So, I played with this idea and came up with the attached WIP. The

Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails

2021-09-16 Thread Andreas Joseph Krogh
På torsdag 16. september 2021 kl. 18:57:39, skrev Tom Lane mailto:t...@sss.pgh.pa.us>>: [...] > FWIW; I saw this Open Item was set to fixed, but I'm still getting this error > in 388726753b638fb9938883bdd057b2ffe6f950f5 The open item was not about that parser shortcoming, nor did this patch

Re: Estimating HugePages Requirements?

2021-09-16 Thread Justin Pryzby
+ * and the hugepage-related mmap flags to use into *mmap_flags. If huge pages + * is not supported, *hugepagesize and *mmap_flags will be set to 0. nitpick: *are* not supported, as you say elsewhere. + gettext_noop("Shows the number of huge pages needed for the main

Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails

2021-09-16 Thread Tom Lane
Andreas Joseph Krogh writes: > På torsdag 16. september 2021 kl. 01:40:31, skrev Tom Lane > >: > [...] > regression=# with recursive cte (x,r) as ( > select 42 as x, row(i, 2.3) as r from generate_series(1,3) i > union all > select x, row((c.r).f1, 4.5) from cte

Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails

2021-09-16 Thread Andreas Joseph Krogh
På torsdag 16. september 2021 kl. 01:40:31, skrev Tom Lane mailto:t...@sss.pgh.pa.us>>: [...] regression=# with recursive cte (x,r) as ( select 42 as x, row(i, 2.3) as r from generate_series(1,3) i union all select x, row((c.r).f1, 4.5) from cte c ) select * from cte; ERROR: record type

Re: Partial index "microvacuum"

2021-09-16 Thread Peter Geoghegan
On Thu, Sep 16, 2021 at 4:45 AM Marko Tiikkaja wrote: > Huh. Interesting. I'm sorry, I wasn't aware of this work and didn't > have version 14 at hand. But it looks like both the partial index as > well as the secondary index on (id::text) get cleaned up nicely there. That's great. I

Re: Teach pg_receivewal to use lz4 compression

2021-09-16 Thread gkokolatos
‐‐‐ Original Message ‐‐‐ On Wednesday, September 15th, 2021 at 08:46, Michael Paquier mich...@paquier.xyz wrote: Hi, thank you for the review. > On Fri, Sep 10, 2021 at 08:21:51AM +, gkokola...@pm.me wrote: > > > Agreed. A default value of 5, which is in the middle point of

Re: Improve logging when using Huge Pages

2021-09-16 Thread Fujii Masao
On 2021/09/08 11:17, Kyotaro Horiguchi wrote: I don't dislike the message, but I'm not sure I like the message is too verbose, especially about it has DETAILS. It seems to me something like the following is sufficient, or I'd like see it even more concise. "fall back anonymous shared memory

Re: Improve logging when using Huge Pages

2021-09-16 Thread Fujii Masao
On 2021/09/07 22:16, Justin Pryzby wrote: On Tue, Sep 07, 2021 at 07:12:36PM +0900, Fujii Masao wrote: One big concern about the patch is that log message is always reported when shared memory fails to be allocated with huge pages enabled when huge_pages=try. Since huge_pages=try is the

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Magnus Hagander
On Wed, Sep 15, 2021 at 10:48 PM Andres Freund wrote: > > On 2021-09-15 10:47:33 -0400, Andrew Dunstan wrote: > > this is an open item for release 14. Is someone going to commit? > > Will do. Although I do wish the original committer would have chimed in at > some point... Crap. My apologies for

Re: Column Filtering in Logical Replication

2021-09-16 Thread Alvaro Herrera
On 2021-Sep-16, Alvaro Herrera wrote: Actually, something like this might be better: > PublicationObjSpec: > | TABLE qualified_name > { > $$ = > makeNode(PublicationObjSpec); >

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Laurenz Albe
On Thu, 2021-09-16 at 02:22 -0700, Andres Freund wrote: > I pushed this. The only substantive change I made is that I moved the > MyBackendType == B_BACKEND check into a new pgstat_should_report_connstat(), > and called that from pgstat_report_connect() and > pgstat_report_disconnect(). Otherwise

Re: Column Filtering in Logical Replication

2021-09-16 Thread Alvaro Herrera
On 2021-Sep-16, vignesh C wrote: > diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y > index e3068a374e..c50bb570ea 100644 > --- a/src/backend/parser/gram.y > +++ b/src/backend/parser/gram.y Yeah, on a quick glance this looks all wrong. Your PublicationObjSpec production should

Re: Column Filtering in Logical Replication

2021-09-16 Thread Amit Kapila
On Thu, Sep 16, 2021 at 6:14 PM Alvaro Herrera wrote: > > On 2021-Sep-16, Amit Kapila wrote: > > > I think the problem here is that with the proposed grammar we won't be > > always able to distinguish names at the gram.y stage. Some post > > parsing analysis is required to attribute the right

Re: EXPLAIN(VERBOSE) to CTE with SEARCH BREADTH FIRST fails

2021-09-16 Thread Andrew Dunstan
On 9/15/21 7:40 PM, Tom Lane wrote: > I wrote: >> I do not think that patch is a proper solution, but we do need to do >> something about this. > I poked into this and decided it's an ancient omission within ruleutils.c. > The reason we've not seen it before is probably that you can't get to the

Re: Column Filtering in Logical Replication

2021-09-16 Thread Alvaro Herrera
On 2021-Sep-16, Amit Kapila wrote: > I think the problem here is that with the proposed grammar we won't be > always able to distinguish names at the gram.y stage. Some post > parsing analysis is required to attribute the right type to name as is > done in the patch. Doesn't it work to stuff

Re: Logical replication keepalive flood

2021-09-16 Thread Amit Kapila
On Thu, Sep 16, 2021 at 6:29 AM houzj.f...@fujitsu.com wrote: > > From Tuesday, September 14, 2021 1:39 PM Greg Nancarrow > wrote: > > However, the problem I found is that, with the patch applied, there is > > a test failure when running “make check-world”: > > > > t/006_logical_decoding.pl

Re: Partial index "microvacuum"

2021-09-16 Thread Marko Tiikkaja
On Wed, Sep 15, 2021 at 7:25 PM Peter Geoghegan wrote: > What about v14? There were significant changes to the > microvacuum/index deletion stuff in that release: > > https://www.postgresql.org/docs/14/btree-implementation.html#BTREE-DELETION Huh. Interesting. I'm sorry, I wasn't aware of this

Re: Signed vs Unsigned (take 2) (src/backend/storage/ipc/procarray.c)

2021-09-16 Thread Ranier Vilela
Em qui., 16 de set. de 2021 às 01:13, Fujii Masao < masao.fu...@oss.nttdata.com> escreveu: > > > On 2021/09/15 21:27, Ranier Vilela wrote: > > I found this in the commit log in the patch. I agree that these > patches > > are refactoring ones. But I'm thinking that it's worth doing >

Re: [BUG] Unexpected action when publishing partition tables

2021-09-16 Thread Amit Kapila
On Thu, Sep 16, 2021 at 7:15 AM houzj.f...@fujitsu.com wrote: > > On Tuesday, September 14, 2021 10:41 PM vignesh C wrote: > > On Tue, Sep 7, 2021 at 11:38 AM houzj.f...@fujitsu.com > > wrote: > > Thanks for the comment. > Attach new version patches which clean the table at the end. > + * For

Re: pgstat_send_connstats() introduces unnecessary timestamp and UDP overhead

2021-09-16 Thread Andres Freund
Hi, On 2021-09-08 06:11:56 +0200, Laurenz Albe wrote: > I have gone over your patch and made the following changes: > > - cache the last report time in a static variable pgLastSessionReportTime > - add a comment to explain why we only track normal backends > - pgindent > - an attempt at a commit

Re: Gather performance analysis

2021-09-16 Thread Dilip Kumar
On Sat, Aug 28, 2021 at 5:04 PM Zhihong Yu wrote: > >> >> * mqh_partial_bytes, mqh_expected_bytes, and mqh_length_word_complete >> >> + Sizemqh_send_pending; >> boolmqh_length_word_complete; >> boolmqh_counterparty_attached; >> >> I wonder if mqh_send_pending

Re: [Proposal] Global temporary tables

2021-09-16 Thread Pavel Stehule
Hi looks so this patch is broken again. Please, can you do rebase? Regards Pavel čt 16. 9. 2021 v 8:28 odesílatel wenjing napsal: > > > > > > > > >