Re: Included xid in restoring reorder buffer changes from disk log message

2021-04-29 Thread Dilip Kumar
On Thu, Apr 29, 2021 at 9:45 PM vignesh C wrote: > > Hi, > > While debugging one of the logical decoding issues, I found that xid was not > included in restoring reorder buffer changes from disk log messages. > Attached a patch for it. I felt including the XID will be helpful in > debugging. T

Re: Replication slot stats misgivings

2021-04-29 Thread vignesh C
On Fri, Apr 30, 2021 at 5:55 AM Masahiko Sawada wrote: > > On Thu, Apr 29, 2021 at 9:44 PM vignesh C wrote: > > > > > > > > On Thu, Apr 29, 2021 at 3:06 PM Amit Kapila wrote: > > > > > > On Wed, Apr 28, 2021 at 5:01 PM Amit Kapila > > > wrote: > > > > > > > > On Wed, Apr 28, 2021 at 4:51 PM Ma

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Dilip Kumar
On Fri, Apr 30, 2021 at 11:09 AM Bharath Rupireddy wrote: > > On Fri, Apr 30, 2021 at 10:51 AM Dilip Kumar wrote: > > > > On Fri, Apr 30, 2021 at 10:43 AM Bharath Rupireddy > > wrote: > > > > > > On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar > > > wrote: > > > > In this function, we already hav

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Bharath Rupireddy
On Fri, Apr 30, 2021 at 10:51 AM Dilip Kumar wrote: > > On Fri, Apr 30, 2021 at 10:43 AM Bharath Rupireddy > wrote: > > > > On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar wrote: > > > In this function, we already have the "defel" variable then I do not > > > understand why you are using one extra

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Dilip Kumar
On Fri, Apr 30, 2021 at 10:43 AM Bharath Rupireddy wrote: > > On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar wrote: > > In this function, we already have the "defel" variable then I do not > > understand why you are using one extra variable and assigning defel to > > that? > > If the goal is to jus

Re: Remove redundant variable from transformCreateStmt

2021-04-29 Thread Bharath Rupireddy
On Fri, Apr 30, 2021 at 7:07 AM Justin Pryzby wrote: > > On Thu, Apr 29, 2021 at 02:39:42PM -0400, Alvaro Herrera wrote: > > On 2021-Apr-29, Tom Lane wrote: > > > Alvaro Herrera writes: > > > > I'd do it like this. Note I removed an if/else block in addition to > > > > your changes. > > > > > >

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Bharath Rupireddy
On Fri, Apr 30, 2021 at 10:17 AM Dilip Kumar wrote: > In this function, we already have the "defel" variable then I do not > understand why you are using one extra variable and assigning defel to > that? > If the goal is to just improve the error message then you can simply > use defel->defname?

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Dilip Kumar
On Fri, Apr 30, 2021 at 8:16 AM Bharath Rupireddy wrote: > > On Thu, Apr 29, 2021 at 10:44 PM Alvaro Herrera > wrote: > > > > On 2021-Apr-29, vignesh C wrote: > > > > > Thanks for the comments, please find the attached v3 patch which has > > > the change for the first part. > > > > Looks good to

Re: [BUG] "FailedAssertion" reported when streaming in logical replication

2021-04-29 Thread Dilip Kumar
On Thu, Apr 29, 2021 at 5:24 PM Amit Kapila wrote: > > On Tue, Apr 27, 2021 at 5:18 PM Dilip Kumar wrote: > > > > I have modified the patch based on the above comments. > > > > The patch looks good to me. I have slightly modified the comments and > commit message. See, what you think of the attac

Re: Use simplehash.h instead of dynahash in SMgr

2021-04-29 Thread David Rowley
I've attached an updated patch. I forgot to call SH_ENTRY_CLEANUP, when it's defined during SH_RESET. I also tided up a couple of comments and change the code to use pg_rotate_right32(.., 31) instead of adding a new function for pg_rotate_left32 and calling that to shift left 1 bit. David v3-0

Re: [PATCH] Identify LWLocks in tracepoints

2021-04-29 Thread Craig Ringer
On Wed, 14 Apr 2021, 22:29 Robert Haas, wrote: > On Tue, Apr 13, 2021 at 10:42 PM Craig Ringer > wrote: > > I'd really love it if a committer could add an explanatory comment or > > two in the area though. I'm happy to draft a comment patch if anyone > > agrees my suggestion is sensible. The key

Re: [PATCH] Identify LWLocks in tracepoints

2021-04-29 Thread Craig Ringer
On Thu, 29 Apr 2021 at 15:31, Peter Eisentraut wrote: > > So if you could produce a separate patch that adds the > > _ENABLED guards targeting PG14 (and PG13), that would be helpful. > > Here is a proposed patch for this. LGTM. Applies and builds fine on master and (with default fuzz) on REL_13_

Re: Replication slot stats misgivings

2021-04-29 Thread Amit Kapila
On Thu, Apr 29, 2021 at 12:07 PM Amit Kapila wrote: > > On Thu, Apr 29, 2021 at 11:14 AM Masahiko Sawada > wrote: > > > > > > > > How about doing both of the above suggestions? Alternatively, we can > > > wait for both 'drop' and 'create' message to be delivered but that > > > might be overkill.

Re: Asymmetric partition-wise JOIN

2021-04-29 Thread Andrey V. Lepikhov
On 11/30/20 7:43 PM, Anastasia Lubennikova wrote: This entry was inactive during this CF, so I've marked it as returned with feedback. Feel free to resubmit an updated version to a future commitfest. I return the patch to commitfest. My current reason differs from reason of origin author. This

Re: Result Cache node shows per-worker info even for workers not launched

2021-04-29 Thread David Rowley
On Wed, 28 Apr 2021 at 23:05, Amit Khandekar wrote: > > On Wed, 28 Apr 2021 at 13:54, David Rowley wrote: > > I've attached a patch to do this. The explain.c part is pretty similar > > to your patch, I just took my original code and comment. > > Sounds good. And thanks for the cleanup patch, and

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Bharath Rupireddy
On Thu, Apr 29, 2021 at 10:44 PM Alvaro Herrera wrote: > > On 2021-Apr-29, vignesh C wrote: > > > Thanks for the comments, please find the attached v3 patch which has > > the change for the first part. > > Looks good to me. I would only add parser_errposition() to the few > error sites missing th

Re: Add client connection check during the execution of the query

2021-04-29 Thread Thomas Munro
On Sat, Apr 3, 2021 at 9:27 AM Thomas Munro wrote: > Pushed! Thanks to all who contributed. Here's something I wanted to park here to look into for the next cycle: it turns out that kqueue's EV_EOF flag also has the right semantics for this. That leads to the idea of exposing the event via the

Re: Remove redundant variable from transformCreateStmt

2021-04-29 Thread Justin Pryzby
On Thu, Apr 29, 2021 at 02:39:42PM -0400, Alvaro Herrera wrote: > On 2021-Apr-29, Tom Lane wrote: > > Alvaro Herrera writes: > > > I'd do it like this. Note I removed an if/else block in addition to > > > your changes. > > > > > I couldn't convince myself that this is worth pushing though; eithe

Re: Replication slot stats misgivings

2021-04-29 Thread Masahiko Sawada
On Thu, Apr 29, 2021 at 9:44 PM vignesh C wrote: > > > > On Thu, Apr 29, 2021 at 3:06 PM Amit Kapila wrote: > > > > On Wed, Apr 28, 2021 at 5:01 PM Amit Kapila wrote: > > > > > > On Wed, Apr 28, 2021 at 4:51 PM Masahiko Sawada > > > wrote: > > > > > > > > On Wed, Apr 28, 2021 at 6:39 PM Amit K

Re: function for testing that causes the backend to terminate

2021-04-29 Thread Joe Conway
On 4/29/21 6:56 AM, Dave Cramer wrote: For testing unusual situations I'd like to be able to cause a backend to terminate due to something like a segfault. Do we currently have this in testing ? If you can run SQL as a superuser from that backend, try: COPY (SELECT pg_backend_pid()) TO PROGR

Re: Patch to allow pg_filedump to support reading of pg_filenode.map

2021-04-29 Thread Richard Yen
On Thu, Apr 29, 2021 at 12:05 PM Justin Pryzby wrote: > I think you should be able to avoid crashing if passed a non-relmapper > file. > Make sure not to loop over more mappings than exist in the relmapper file > of > the given size. > > I guess you should warn if the number of mappings is too la

Re: Patch to allow pg_filedump to support reading of pg_filenode.map

2021-04-29 Thread Justin Pryzby
I think you should be able to avoid crashing if passed a non-relmapper file. Make sure not to loop over more mappings than exist in the relmapper file of the given size. I guess you should warn if the number of mappings is too large for the file's size. And then "cap" the number of mappings to th

Re: Remove redundant variable from transformCreateStmt

2021-04-29 Thread Alvaro Herrera
On 2021-Apr-29, Tom Lane wrote: > Alvaro Herrera writes: > > I'd do it like this. Note I removed an if/else block in addition to > > your changes. > > > I couldn't convince myself that this is worth pushing though; either we > > push it to all branches (which seems unwarranted) or we create > >

Re: Patch to allow pg_filedump to support reading of pg_filenode.map

2021-04-29 Thread Richard Yen
Thanks for the feedback, Justin. I've gone ahead and switched to use memcmp. I also refactored it to: 1. Don't assume that any file with first 4 bytes matching the relmapper magic number is a pg_relnode.map file 2. Don't assume the pg_relnode.map file is uncorrupted and intact; perform a check o

Re: Race condition in InvalidateObsoleteReplicationSlots()

2021-04-29 Thread Álvaro Herrera
On 2021-Apr-07, Andres Freund wrote: > I'm also confused by the use of ConditionVariableTimedSleep(timeout = > 10). Why do we need a timed sleep here in the first place? And why with > such a short sleep? I was scared of the possibility that a process would not set the CV for whatever reason, cau

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread Alvaro Herrera
On 2021-Apr-29, vignesh C wrote: > Thanks for the comments, please find the attached v3 patch which has > the change for the first part. Looks good to me. I would only add parser_errposition() to the few error sites missing that. -- Álvaro Herrera39°49'30"S 73°17'

Re: Enhanced error message to include hint messages for redundant options error

2021-04-29 Thread vignesh C
On Mon, Apr 26, 2021 at 9:24 PM Alvaro Herrera wrote: > > On 2021-Apr-26, Bharath Rupireddy wrote: > > > I agree that we can just be clear about the problem. Looks like the > > majority of the errors "conflicting or redundant options" are for > > redundant options. So, wherever "conflicting or red

Included xid in restoring reorder buffer changes from disk log message

2021-04-29 Thread vignesh C
Hi, While debugging one of the logical decoding issues, I found that xid was not included in restoring reorder buffer changes from disk log messages. Attached a patch for it. I felt including the XID will be helpful in debugging. Thoughts? Regards, Vignesh From 9d3ee45b7b2c0d625af888579035a0fb9a1

Re: Remove redundant variable from transformCreateStmt

2021-04-29 Thread Tom Lane
Alvaro Herrera writes: > I'd do it like this. Note I removed an if/else block in addition to > your changes. > I couldn't convince myself that this is worth pushing though; either we > push it to all branches (which seems unwarranted) or we create > back-patching hazards. Yeah ... an advantage

Re: default_tablespace doc and partitioned rels

2021-04-29 Thread Alvaro Herrera
On 2021-Apr-16, Justin Pryzby wrote: > If this parameter is set when a partitioned table is created, the partitioned > table's tablespace will be set to the given tablespace, and which will be the > default tablespace for partitions create in the future, even if > default_tablespace itself has sin

Re: Remove redundant variable from transformCreateStmt

2021-04-29 Thread Alvaro Herrera
I'd do it like this. Note I removed an if/else block in addition to your changes. I couldn't convince myself that this is worth pushing though; either we push it to all branches (which seems unwarranted) or we create back-patching hazards. -- Álvaro Herrera39°49'30"S

Re: Remove post-increment in function quote_identifier of pg_upgrade

2021-04-29 Thread Alvaro Herrera
On 2021-Apr-29, Tom Lane wrote: > (On the other hand, if it were written the other way already, I'd also > argue to leave it like that. Basically, this sort of change is just not > worth troubling over. It doesn't improve things meaningfully and it > creates back-patching hazards.) This argumen

Re: pg_hba.conf.sample wording improvement

2021-04-29 Thread Stephen Frost
Greetings, * Magnus Hagander (mag...@hagander.net) wrote: > On Thu, Apr 29, 2021 at 7:08 AM Peter Eisentraut > wrote: > > On 28.04.21 16:09, Alvaro Herrera wrote: > > > Looking at it now, I wonder how well do the "hostno" options work. If I > > > say "hostnogssenc", is an SSL-encrypted socket go

Re: tool to migrate database

2021-04-29 Thread silvio brandani
concepts similar and related to migration/replication are applied with the software tcapture , please give a look at https://www.tcapture.net/ Il giorno gio 29 apr 2021 alle ore 16:34 Bruce Momjian ha scritto: > On Tue, Mar 23, 2021 at 09:49:57AM +0100, Joel Jacobson wrote: > > I recently rea

Re: Addition of authenticated ID to pg_stat_activity

2021-04-29 Thread Magnus Hagander
On Tue, Apr 27, 2021 at 8:25 PM Andres Freund wrote: > > Hi, > > On 2021-04-27 12:40:29 -0400, Stephen Frost wrote: > > So, what fields are people really looking at when querying > > pg_stat_activity interactively? User, database, pid, last query, > > transaction start, query start, state, wait e

Re: Remove post-increment in function quote_identifier of pg_upgrade

2021-04-29 Thread Tom Lane
Justin Pryzby writes: > On Thu, Apr 29, 2021 at 06:35:28PM +0530, Vaibhav Dalvi wrote: >> If my understanding is correct then '++' is not needed in the >> above highlighted statement which is leading to overhead. > I don't think the integer increment during pg_upgrade is a meaningful > overhead.

Re: Remove post-increment in function quote_identifier of pg_upgrade

2021-04-29 Thread Justin Pryzby
On Thu, Apr 29, 2021 at 06:35:28PM +0530, Vaibhav Dalvi wrote: > Hi, > > The function quote_identifier has extra post-increment operation as > highlighted below, > > char * > quote_identifier(const char *s) > { >char *result = pg_malloc(strlen(s) * 2 + 3); >char *r = result; > >*

Re: Unresolved repliaction hang and stop problem.

2021-04-29 Thread Alvaro Herrera
Cc'ing Lukasz Biegaj because of the pgsql-general thread. On 2021-Apr-29, Amit Kapila wrote: > On Wed, Apr 28, 2021 at 7:36 PM Alvaro Herrera > wrote: > > ... It's strange that replication worked for them on pg10 though and > > broke on 13. What did we change anything to make it so? > > No i

Remove post-increment in function quote_identifier of pg_upgrade

2021-04-29 Thread Vaibhav Dalvi
Hi, The function quote_identifier has extra post-increment operation as highlighted below, char * quote_identifier(const char *s) { char *result = pg_malloc(strlen(s) * 2 + 3); char *r = result; *r++ = '"'; while (*s) { if (*s == '"') *r++ = *s; *r++ = *s;

Re: Replication slot stats misgivings

2021-04-29 Thread vignesh C
On Thu, Apr 29, 2021 at 3:06 PM Amit Kapila wrote: > > On Wed, Apr 28, 2021 at 5:01 PM Amit Kapila wrote: > > > > On Wed, Apr 28, 2021 at 4:51 PM Masahiko Sawada wrote: > > > > > > On Wed, Apr 28, 2021 at 6:39 PM Amit Kapila wrote: > > > > @@ -1369,7 +1369,7 @@ ReorderBufferIterTXNNext(ReorderB

Re: [BUG] "FailedAssertion" reported when streaming in logical replication

2021-04-29 Thread Amit Kapila
On Tue, Apr 27, 2021 at 5:18 PM Dilip Kumar wrote: > > I have modified the patch based on the above comments. > The patch looks good to me. I have slightly modified the comments and commit message. See, what you think of the attached? I think we can leave the test for this as there doesn't seem t

Re: pg_dump new feature: exporting functions only. Bad or good idea ?

2021-04-29 Thread ahsan hadi
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested Few minor comments : - The latest patch has some hunk failures -

Re: function for testing that causes the backend to terminate

2021-04-29 Thread Bharath Rupireddy
On Thu, Apr 29, 2021 at 4:27 PM Dave Cramer wrote: > For testing unusual situations I'd like to be able to cause a backend to > terminate due to something like a segfault. Do we currently have this in > testing ? Well, you could use pg_terminate_backend which sends SIGTERM to the backend. Howev

function for testing that causes the backend to terminate

2021-04-29 Thread Dave Cramer
For testing unusual situations I'd like to be able to cause a backend to terminate due to something like a segfault. Do we currently have this in testing ? Dave Cramer

Re: [BUG] "FailedAssertion" reported when streaming in logical replication

2021-04-29 Thread Dilip Kumar
On Thu, Apr 29, 2021 at 12:09 PM tanghy.f...@fujitsu.com wrote: > > > On Thursday, April 29, 2021 3:18 PM, Dilip Kumar wrote > > >I tried to think about how to write a test case for this scenario, but > >I think it will not be possible to generate an automated test case for this. > >Basically, we

Re: [PATCH] We install pg_regress and isolationtester but not pg_isolation_regress

2021-04-29 Thread Magnus Hagander
On Thu, Apr 29, 2021 at 4:41 AM Tom Lane wrote: > > Michael Paquier writes: > > On Wed, Apr 28, 2021 at 12:44:45PM +0300, Aleksander Alekseev wrote: > >> I just wanted to let you know that TimescaleDB uses > >> pg_isolation_regress and occasionally there are reports from some > >> suffering/puzzl

Re: pg_hba.conf.sample wording improvement

2021-04-29 Thread Magnus Hagander
On Thu, Apr 29, 2021 at 7:08 AM Peter Eisentraut wrote: > > On 28.04.21 16:09, Alvaro Herrera wrote: > > Looking at it now, I wonder how well do the "hostno" options work. If I > > say "hostnogssenc", is an SSL-encrypted socket good? If I say > > "hostnossl", is a GSS-encrypted socket good? If

Re: Replication slot stats misgivings

2021-04-29 Thread Amit Kapila
On Wed, Apr 28, 2021 at 5:01 PM Amit Kapila wrote: > > On Wed, Apr 28, 2021 at 4:51 PM Masahiko Sawada wrote: > > > > On Wed, Apr 28, 2021 at 6:39 PM Amit Kapila wrote: > > @@ -1369,7 +1369,7 @@ ReorderBufferIterTXNNext(ReorderBuffer *rb, > ReorderBufferIterTXNState *state) > * Update the tota

Re: [HACKERS] logical decoding of two-phase transactions

2021-04-29 Thread Peter Smith
On Tue, Apr 27, 2021 at 6:17 PM vignesh C wrote: > > On Wed, Apr 21, 2021 at 12:13 PM Peter Smith wrote: > > > > On Tue, Apr 20, 2021 at 3:45 PM Peter Smith wrote: > > > > > > Please find attached the latest patch set v73`* > > > > > > Differences from v72* are: > > > > > > * Rebased to HEAD @ t

Re: [HACKERS] logical decoding of two-phase transactions

2021-04-29 Thread Peter Smith
On Tue, Apr 27, 2021 at 1:41 PM vignesh C wrote: > > On Wed, Apr 21, 2021 at 12:13 PM Peter Smith wrote: > > > > On Tue, Apr 20, 2021 at 3:45 PM Peter Smith wrote: > > > > > > Please find attached the latest patch set v73`* > > > > > > Differences from v72* are: > > > > > > * Rebased to HEAD @ t

Re: [HACKERS] logical decoding of two-phase transactions

2021-04-29 Thread Peter Smith
On Mon, Apr 26, 2021 at 9:22 PM vignesh C wrote: > > On Wed, Apr 21, 2021 at 12:13 PM Peter Smith wrote: > > > > On Tue, Apr 20, 2021 at 3:45 PM Peter Smith wrote: > > > > > > Please find attached the latest patch set v73`* > > > > > > Differences from v72* are: > > > > > > * Rebased to HEAD @ t

Re: Replication slot stats misgivings

2021-04-29 Thread vignesh C
On Thu, Apr 29, 2021 at 11:14 AM Masahiko Sawada wrote: > > On Thu, Apr 29, 2021 at 11:55 AM Amit Kapila wrote: > > > > On Thu, Apr 29, 2021 at 4:58 AM Masahiko Sawada > > wrote: > > > > > > On Thu, Apr 29, 2021 at 5:41 AM Tom Lane wrote: > > > > > > > > It seems that the test case added by f5

Re: [PATCH] Identify LWLocks in tracepoints

2021-04-29 Thread Peter Eisentraut
On 14.04.21 15:20, Peter Eisentraut wrote: On 12.04.21 07:46, Craig Ringer wrote: > To use systemtap semaphores (the _ENABLED macros) you need to run     dtrace > -g to generate a probes.o then link that into postgres. > > I don't think we do that. I'll double check soon.   

Re: Skip temporary table schema name from explain-verbose output.

2021-04-29 Thread Amul Sul
On Wed, Apr 28, 2021 at 7:56 PM Tom Lane wrote: > > Greg Stark writes: > >> On Tue, Apr 27, 2021 at 7:08 PM Bharath Rupireddy > >> >> Make sense, we would lose the ability to differentiate temporary > >> tables from the auto_explain logs. > > > There's no useful differentiation that can be done

Re: SQL-standard function body

2021-04-29 Thread Peter Eisentraut
On 27.04.21 04:44, Justin Pryzby wrote: On Thu, Apr 22, 2021 at 04:04:18PM -0400, Jeff Janes wrote: This commit break line continuation prompts for unbalanced parentheses in the psql binary. Skimming through this thread, I don't see that this is intentional or has been noticed before. with psq