Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-10-26 Thread Michael Paquier
On Thu, Oct 26, 2017 at 3:05 AM, Kyotaro HORIGUCHI wrote: > The largest obstacle to do that is that walreceiver is not > utterly concerned to record internals. In other words, it doesn't > know what a record is. Teaching that introduces much complexity > and the complexity slows down the walreceiv

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-10-26 Thread Kyotaro HORIGUCHI
Hello. Thank you for looking this. At Mon, 16 Oct 2017 17:58:03 +0900, Michael Paquier wrote in > On Thu, Sep 7, 2017 at 12:33 PM, Kyotaro HORIGUCHI > wrote: > > At Wed, 6 Sep 2017 12:23:53 -0700, Andres Freund wrote > > in <20170906192353.ufp2dq7wm5fd6...@alap3.anarazel.de> > >> I'm not fol

Re: [HACKERS] [BUGS] BUG #14849: jsonb_build_object doesn't like VARIADIC calls very much

2017-10-22 Thread Tom Lane
Andrew Dunstan writes: > Sorry. Here it is. This comment is neither correct nor intelligible: /* important for value, key cannot being NULL */ I'd say just drop it. The checks for "could not determine data type" errors seem rather duplicative, too. The extern declaration seems to hav

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-10-16 Thread Michael Paquier
On Thu, Sep 7, 2017 at 12:33 PM, Kyotaro HORIGUCHI wrote: > At Wed, 6 Sep 2017 12:23:53 -0700, Andres Freund wrote > in <20170906192353.ufp2dq7wm5fd6...@alap3.anarazel.de> >> I'm not following. All we need to use is the beginning of the relevant >> records, that's easy enough to keep track of. W

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-10-05 Thread Robert Haas
On Tue, Oct 3, 2017 at 9:38 PM, Andres Freund wrote: >> Do you have any suggestion as to how we should transmit the blacklist to >> parallel workers? > > How about storing them in the a dshash table instead of dynahash? > Similar to how we're now dealing with the shared typmod registry stuff? > It

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-10-03 Thread Andres Freund
On 2017-10-03 19:53:41 -0400, Andrew Dunstan wrote: > On 09/27/2017 02:52 PM, Tom Lane wrote: > > Andrew Dunstan writes: > >> At this stage on reflection I agree it should be pulled :-( > > That seems to be the consensus, so I'll go make it happen. > > > >> I'm not happy about the idea of marking

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-10-03 Thread Tom Lane
Andrew Dunstan writes: > Do you have any suggestion as to how we should transmit the blacklist to > parallel workers? Perhaps serialize the contents into an array in DSM, then rebuild a hash table from that in the worker. Robert might have a better idea though. regards,

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-10-03 Thread Andrew Dunstan
On 09/27/2017 02:52 PM, Tom Lane wrote: > Andrew Dunstan writes: >> At this stage on reflection I agree it should be pulled :-( > That seems to be the consensus, so I'll go make it happen. > >> I'm not happy about the idea of marking an input function as not >> parallel safe, certainly not witho

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-27 Thread Tom Lane
Andrew Dunstan writes: > At this stage on reflection I agree it should be pulled :-( That seems to be the consensus, so I'll go make it happen. > I'm not happy about the idea of marking an input function as not > parallel safe, certainly not without a good deal of thought and > discussion that w

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Tom Lane
Andrew Dunstan writes: > I'm not happy about the idea of marking an input function as not > parallel safe, certainly not without a good deal of thought and > discussion that we don't have time for this cycle. Yeah, that aspect of it was bothering me too: it's easy to say "mark the function unsafe

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Andrew Dunstan
On 09/26/2017 06:04 PM, Andrew Dunstan wrote: > > On 09/26/2017 05:45 PM, Stephen Frost wrote: >> I've not been following along very closely- are we sure that ripping >> this out won't be worse than dealing with it in-place? Will pulling it >> out also require a post-RC1 catversion bump? >> >> >

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Andrew Dunstan
On 09/26/2017 05:45 PM, Stephen Frost wrote: > > I've not been following along very closely- are we sure that ripping > this out won't be worse than dealing with it in-place? Will pulling it > out also require a post-RC1 catversion bump? > > It shouldn't do AFAIK - the function signatures were

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Stephen Frost
Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Bruce Momjian writes: > > On Tue, Sep 26, 2017 at 04:07:02PM -0400, Tom Lane wrote: > >> Any other votes out there? > > > Well, I was concerned yesterday that we had a broken build farm so close > > to release. (I got consistent regression failu

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Bruce Momjian
On Tue, Sep 26, 2017 at 05:32:15PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Tue, Sep 26, 2017 at 04:07:02PM -0400, Tom Lane wrote: > >> Any other votes out there? > > > Well, I was concerned yesterday that we had a broken build farm so close > > to release. (I got consistent regressi

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Tom Lane
Bruce Momjian writes: > On Tue, Sep 26, 2017 at 04:07:02PM -0400, Tom Lane wrote: >> Any other votes out there? > Well, I was concerned yesterday that we had a broken build farm so close > to release. (I got consistent regression failures.) I think PG 11 would > be better for this feature change

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Bruce Momjian
On Tue, Sep 26, 2017 at 04:07:02PM -0400, Tom Lane wrote: > Andrew Dunstan writes: > > On 09/26/2017 02:37 PM, Tom Lane wrote: > >> ... and the buildfarm's not too happy. It looks like force_parallel_mode > >> breaks all the regression test cases around unsafe enums; which on > >> reflection is u

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Tom Lane
Andrew Dunstan writes: > On 09/26/2017 02:37 PM, Tom Lane wrote: >> ... and the buildfarm's not too happy. It looks like force_parallel_mode >> breaks all the regression test cases around unsafe enums; which on >> reflection is unsurprising, because parallel workers will not have access >> to the

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Andrew Dunstan
On 09/26/2017 02:37 PM, Tom Lane wrote: > I wrote: >> Pushed; sorry for the delay. > ... and the buildfarm's not too happy. It looks like force_parallel_mode > breaks all the regression test cases around unsafe enums; which on > reflection is unsurprising, because parallel workers will not have

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Tom Lane
I wrote: > Pushed; sorry for the delay. ... and the buildfarm's not too happy. It looks like force_parallel_mode breaks all the regression test cases around unsafe enums; which on reflection is unsurprising, because parallel workers will not have access to the parent's blacklist hash, so they wil

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-26 Thread Tom Lane
I wrote: > Andrew Dunstan writes: >> OK, that seems to be the consensus. So let's apply the blacklist patch >> and then separately remove the 'created in the same transaction' test. >> We'll need to adjust the regression tests and docs accordingly. > Agreed. I'll work on that in a little bit. P

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Tom Lane
Andrew Dunstan writes: > OK, that seems to be the consensus. So let's apply the blacklist patch > and then separately remove the 'created in the same transaction' test. > We'll need to adjust the regression tests and docs accordingly. Agreed. I'll work on that in a little bit.

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Andrew Dunstan
On 09/25/2017 01:34 PM, David E. Wheeler wrote: > On Sep 25, 2017, at 10:55, Andrew Dunstan > wrote: > >> Let's ask a couple of users who I think are or have been actually >> hurting on this point. Christophe and David, any opinions? > If I understand the issue correctly, I think I’d be fine wi

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread David E. Wheeler
On Sep 25, 2017, at 10:55, Andrew Dunstan wrote: > Let's ask a couple of users who I think are or have been actually > hurting on this point. Christophe and David, any opinions? If I understand the issue correctly, I think I’d be fine with requiring ALTER TYPE ADD LABEL to be disallowed in a t

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Christophe Pettus
> On Sep 25, 2017, at 07:55, Andrew Dunstan > wrote: > Let's ask a couple of users who I think are or have been actually > hurting on this point. Christophe and David, any opinions? Since about 90% of what I encounter in this area are automatically-generated migrations, having a clear set of (

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Andrew Dunstan
On 09/25/2017 10:42 AM, Tom Lane wrote: > Andrew Dunstan writes: >> On 09/25/2017 10:14 AM, Tom Lane wrote: >>> Oh ... I did not think we were on the same page, because your patch >>> didn't include removal of the same-transaction heuristic. It'd be >>> sensible to do that as a separate patch,

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Tom Lane
Andrew Dunstan writes: > On 09/25/2017 10:14 AM, Tom Lane wrote: >> Oh ... I did not think we were on the same page, because your patch >> didn't include removal of the same-transaction heuristic. It'd be >> sensible to do that as a separate patch, though, to make it easier >> to put back if we d

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Andrew Dunstan
On 09/25/2017 10:14 AM, Tom Lane wrote: > Andrew Dunstan writes: >> On 09/24/2017 07:06 PM, Tom Lane wrote: >>> So I think we should just stop with the blacklist test for v10, >>> and then see if we still get complaints (and exactly what they're >>> about) so that we can judge how much more work

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-25 Thread Tom Lane
Andrew Dunstan writes: > On 09/24/2017 07:06 PM, Tom Lane wrote: >> So I think we should just stop with the blacklist test for v10, >> and then see if we still get complaints (and exactly what they're >> about) so that we can judge how much more work the problem deserves. >> It's still ahead of wh

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-24 Thread Andrew Dunstan
On 09/24/2017 07:06 PM, Tom Lane wrote: > > So I think we should just stop with the blacklist test for v10, > and then see if we still get complaints (and exactly what they're > about) so that we can judge how much more work the problem deserves. > It's still ahead of where we were in previous re

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-24 Thread Tom Lane
Andrew Dunstan writes: > On 09/24/2017 04:37 PM, Tom Lane wrote: >> What we still need to debate is whether to remove the heuristic >> type-is-from-same-transaction test, making the user-visible behavior >> simply "you must commit an ALTER TYPE ADD VALUE before you can use the >> new value". I'm

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-24 Thread Andrew Dunstan
On 09/24/2017 04:37 PM, Tom Lane wrote: > Andrew Dunstan writes: >> OK, here's the finished patch. It has a pretty small footprint all >> things considered, and I think it guarantees that nothing that could be >> done in this area in 9.6 will be forbidden. That's probably enough to >> get us to

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-24 Thread Tom Lane
Andrew Dunstan writes: > OK, here's the finished patch. It has a pretty small footprint all > things considered, and I think it guarantees that nothing that could be > done in this area in 9.6 will be forbidden. That's probably enough to > get us to 10 without having to revert the whole thing, IST

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-24 Thread Andrew Dunstan
On 09/23/2017 06:06 PM, Tom Lane wrote: > Andrew Dunstan writes: >> OK, I think I'm convinced. Here's is the WIP code I put together for the >> blacklist. I'm was looking for a place to put the init call, but since >> it's possibly not going anywhere I stopped :-) . My initial thought >> about s

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Tom Lane
Andrew Dunstan writes: > OK, I think I'm convinced. Here's is the WIP code I put together for the > blacklist. I'm was looking for a place to put the init call, but since > it's possibly not going anywhere I stopped :-) . My initial thought > about substransactions was that we should ignore them f

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Andrew Dunstan
On 09/23/2017 03:52 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 09/23/2017 02:00 PM, Tom Lane wrote: >>> So I'm back to not being sure about the path forward. Maybe it would be >>> all right to say "the value added by ADD VALUE can't be used in the same >>> transaction, period". That's

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Tom Lane
Andrew Dunstan writes: > On 09/23/2017 02:00 PM, Tom Lane wrote: >> So I'm back to not being sure about the path forward. Maybe it would be >> all right to say "the value added by ADD VALUE can't be used in the same >> transaction, period". That's still a step forward compared to the pre-v10 >>

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Andrew Dunstan
On 09/23/2017 02:00 PM, Tom Lane wrote: > I wrote: >> Andrew Dunstan writes: >>> I see what you're saying, but my idea was slightly different. We would >>> only add to the hashtable I had in mind at the bottom AddEnumLabel(). >>> Any other value, whether added in the current transaction or not,

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Tom Lane
I wrote: > Andrew Dunstan writes: >> I see what you're saying, but my idea was slightly different. We would >> only add to the hashtable I had in mind at the bottom AddEnumLabel(). >> Any other value, whether added in the current transaction or not, should >> be safe, AIUI. > Oh, I see: a list of

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Andrew Dunstan
On 09/23/2017 11:16 AM, Tom Lane wrote: > Andrew Dunstan writes: > >>> The immediate question is do we care to design/implement such a thing >>> post-RC1. I'd have to vote "no". I think the most prudent thing to >>> do is revert 15bc038f9 and then have another go at it during the v11 >>> cycle

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Tom Lane
Andrew Dunstan writes: > On 09/22/2017 11:19 PM, Tom Lane wrote: >> Yeah, I was considering the same thing over dinner, though I'd phrase >> it oppositely: keep a list of enum type OIDs created in the current >> transaction, so that we could whitelist them. This could maybe become >> a problem if

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-23 Thread Andrew Dunstan
On 09/22/2017 11:19 PM, Tom Lane wrote: > Andrew Dunstan writes: >> On 09/22/2017 05:46 PM, Tom Lane wrote: >>> I'm not sure if that qualifies as a stop-ship problem, but it ain't >>> good, for sure. We need to look at whether we should revert 15bc038f9 >>> or somehow revise its rules. >> I won

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-22 Thread Tom Lane
Andrew Dunstan writes: > On 09/22/2017 05:46 PM, Tom Lane wrote: >> I'm not sure if that qualifies as a stop-ship problem, but it ain't >> good, for sure. We need to look at whether we should revert 15bc038f9 >> or somehow revise its rules. > I wonder if we wouldn't be better > doing this more d

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-22 Thread Andrew Dunstan
On 09/22/2017 05:46 PM, Tom Lane wrote: > bal...@obiserver.hu writes: >> PostgreSQL version: 10beta4 >> testdb=# begin; >> BEGIN >> testdb=# create type enumtype as enum ('v1', 'v2'); >> CREATE TYPE >> testdb=# alter type enumtype owner to testrole; >> ALTER TYPE >> testdb=# create table testtabl

Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

2017-09-22 Thread Tom Lane
bal...@obiserver.hu writes: > PostgreSQL version: 10beta4 > testdb=# begin; > BEGIN > testdb=# create type enumtype as enum ('v1', 'v2'); > CREATE TYPE > testdb=# alter type enumtype owner to testrole; > ALTER TYPE > testdb=# create table testtable (enumcolumn enumtype not null default 'v1'); > ER

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-06 Thread Kyotaro HORIGUCHI
Hello, At Wed, 6 Sep 2017 12:23:53 -0700, Andres Freund wrote in <20170906192353.ufp2dq7wm5fd6...@alap3.anarazel.de> > On 2017-09-06 17:36:02 +0900, Kyotaro HORIGUCHI wrote: > > The problem is that the current ReadRecord needs the first one of > > a series of continuation records from the same s

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-06 Thread Andres Freund
On 2017-09-06 17:36:02 +0900, Kyotaro HORIGUCHI wrote: > Hi, > > At Mon, 4 Sep 2017 17:17:19 +0900, Michael Paquier > wrote in > > > On Mon, Sep 4, 2017 at 4:04 PM, Andres Freund wrote: > > > I've not read through the thread, but this seems like the wrong approach > > > to me. The receiving s

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-06 Thread Kyotaro HORIGUCHI
Hi, At Mon, 4 Sep 2017 17:17:19 +0900, Michael Paquier wrote in > On Mon, Sep 4, 2017 at 4:04 PM, Andres Freund wrote: > > I've not read through the thread, but this seems like the wrong approach > > to me. The receiving side should use a correct value, instead of putting > > this complexity o

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-04 Thread Michael Paquier
On Mon, Sep 4, 2017 at 4:04 PM, Andres Freund wrote: > I've not read through the thread, but this seems like the wrong approach > to me. The receiving side should use a correct value, instead of putting > this complexity on the sender's side. Yes I agree with that. The current patch gives me a ba

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-04 Thread Andres Freund
Hi, On 2017-09-04 15:51:51 +0900, Kyotaro HORIGUCHI wrote: > SpinLockAcquire(&walsnd->mutex); > + oldFlushPtr = walsnd->flush; > walsnd->write = writePtr; > walsnd->flush = flushPtr; > walsnd->apply = applyPtr; > @@ -1821,7 +1836,

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-09-03 Thread Kyotaro HORIGUCHI
Hello, Thank you for reviewing this. At Mon, 28 Aug 2017 20:14:54 +0900, Michael Paquier wrote in > On Mon, Aug 28, 2017 at 8:02 PM, Kyotaro HORIGUCHI > wrote: > > The first patch (0001-) fixes this problem, preventing the > > problematic state of WAL segments by retarding restart LSN of a >

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-30 Thread Tom Lane
Amit Kapila writes: > On Tue, Aug 29, 2017 at 10:05 PM, Tom Lane wrote: >> If no objections, I'll do the additional legwork and push. > No objections. Done. Out of curiosity, I pushed just the rescan-param patch to the buildfarm to start with, to see if anything would fall over, and indeed som

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-30 Thread Robert Haas
On Wed, Aug 30, 2017 at 7:39 AM, Tom Lane wrote: > Amit Kapila writes: >> On Tue, Aug 29, 2017 at 10:05 PM, Tom Lane wrote: >> ! /* Make sure any existing workers are gracefully shut down */ >> ExecShutdownGatherWorkers(node); > >> The above call doesn't ensure the shutdown. It just ensures th

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-30 Thread Tom Lane
Amit Kapila writes: > On Tue, Aug 29, 2017 at 10:05 PM, Tom Lane wrote: > ! /* Make sure any existing workers are gracefully shut down */ > ExecShutdownGatherWorkers(node); > The above call doesn't ensure the shutdown. It just ensures that we > receive all messages from parallel workers. Basi

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-30 Thread Amit Kapila
On Tue, Aug 29, 2017 at 10:05 PM, Tom Lane wrote: > Amit Kapila writes: >> On Tue, Aug 29, 2017 at 8:32 AM, Robert Haas wrote: >>> There's already ExecParallelReinitialize, which could be made to walk >>> the nodes in addition to what it does already, but I don't understand >>> exactly what else

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-29 Thread Tom Lane
Amit Kapila writes: > On Tue, Aug 29, 2017 at 8:32 AM, Robert Haas wrote: >> There's already ExecParallelReinitialize, which could be made to walk >> the nodes in addition to what it does already, but I don't understand >> exactly what else needs fixing. > Sure, but it is not advisable to reset

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-29 Thread Amit Kapila
On Tue, Aug 29, 2017 at 8:32 AM, Robert Haas wrote: > On Mon, Aug 28, 2017 at 10:17 PM, Tom Lane wrote: >> In the meantime, I think what we should do is commit the bug fix more or >> less as I have it, and then work on Amit's concern about losing parallel >> efficiency by separating the resetting

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Robert Haas
On Mon, Aug 28, 2017 at 10:17 PM, Tom Lane wrote: > In the meantime, I think what we should do is commit the bug fix more or > less as I have it, and then work on Amit's concern about losing parallel > efficiency by separating the resetting of shared parallel-scan state > into a new plan tree trav

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Robert Haas writes: > On Mon, Aug 28, 2017 at 6:35 PM, Tom Lane wrote: >> Maybe parallel_aware should have more than two values, depending >> on whether the result of the node is context-dependent or not. > It seems likely the whole concept of parallel_aware is only only a > zero-order approxima

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Robert Haas
On Mon, Aug 28, 2017 at 6:35 PM, Tom Lane wrote: > but probably we should think of a more arm's-length way to do it. > Maybe parallel_aware should have more than two values, depending > on whether the result of the node is context-dependent or not. My original intent for the parallel_aware flag w

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Robert Haas writes: > On Mon, Aug 28, 2017 at 3:00 PM, Tom Lane wrote: >> Count the "that"s (you're failing to notice the next line). > OK, true. But "Academic literature" -> "The academic literature" is > just second-guessing, I think. No, it was more to avoid reflowing the paragraph (or leav

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Robert Haas
On Mon, Aug 28, 2017 at 3:00 PM, Tom Lane wrote: >> That sentence isn't wrong as written. > > Count the "that"s (you're failing to notice the next line). OK, true. But "Academic literature" -> "The academic literature" is just second-guessing, I think. >> I don't really understand the part abou

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Robert Haas writes: > On Mon, Aug 28, 2017 at 10:47 AM, Tom Lane wrote: > - fuller description. Academic literature on parallel query suggests that > + fuller description. The academic literature on parallel query suggests > That sentence isn't wrong as written. Count the "that"s (you're fail

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Robert Haas
On Mon, Aug 28, 2017 at 10:47 AM, Tom Lane wrote: > That seems like an unacceptably fragile assumption. Even if it happens to > be true today, we would need to fix it sooner or later. (And I kinda > suspect it's possible to break it today, anyway. Treating PARAM_EXEC > Params as parallel-restri

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Amit Kapila writes: > On Mon, Aug 28, 2017 at 6:34 PM, Tom Lane wrote: >> If what you're complaining about is that I put back the "if >> (outerPlan->chgParam == NULL)" test to allow postponement of the >> recursive ExecReScan call, I'm afraid that it's mere wishful >> thinking that omitting that

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Amit Kapila
On Mon, Aug 28, 2017 at 6:34 PM, Tom Lane wrote: > Amit Kapila writes: >> On Mon, Aug 28, 2017 at 6:01 PM, Tom Lane wrote: >>> Um, what's different about that than before? > >> Earlier, we perform the rescan of all the nodes before ExecProcNode, >> so workers will always start (restart) after th

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Amit Kapila writes: > On Mon, Aug 28, 2017 at 6:01 PM, Tom Lane wrote: >> Um, what's different about that than before? > Earlier, we perform the rescan of all the nodes before ExecProcNode, > so workers will always start (restart) after the scan descriptor is > initialized. If what you're compl

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Amit Kapila
On Mon, Aug 28, 2017 at 6:01 PM, Tom Lane wrote: > Amit Kapila writes: >> With this change, it is quite possible that during rescans workers >> will not do any work. > > Um, what's different about that than before? > Earlier, we perform the rescan of all the nodes before ExecProcNode, so workers

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Tom Lane
Amit Kapila writes: > With this change, it is quite possible that during rescans workers > will not do any work. Um, what's different about that than before? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-28 Thread Amit Kapila
On Mon, Aug 28, 2017 at 1:59 AM, Tom Lane wrote: > I wrote: >> I think that the correct fix probably involves marking each parallel scan >> plan node as dependent on a pseudo executor parameter, which the parent >> Gather or GatherMerge node would flag as being changed on each rescan. >> This woul

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-08-28 Thread Michael Paquier
On Mon, Aug 28, 2017 at 8:02 PM, Kyotaro HORIGUCHI wrote: > The first patch (0001-) fixes this problem, preventing the > problematic state of WAL segments by retarding restart LSN of a > physical replication slot in a certain condition. FWIW, I have this patch marked on my list of things to look

Re: [HACKERS] [BUGS] Bug in Physical Replication Slots (at least 9.5)?

2017-08-28 Thread Kyotaro HORIGUCHI
Hello, This problem still occurs on the master. I rebased this to the current master. At Mon, 3 Apr 2017 08:38:47 +0900, Michael Paquier wrote in > On Mon, Apr 3, 2017 at 7:19 AM, Venkata B Nagothi wrote: > > As we are already past the commitfest, I am not sure, what should i change > > the p

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-27 Thread Tom Lane
I wrote: > I think that the correct fix probably involves marking each parallel scan > plan node as dependent on a pseudo executor parameter, which the parent > Gather or GatherMerge node would flag as being changed on each rescan. > This would cue the plan layers in between that they cannot optimi

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-27 Thread Tom Lane
I wrote: > 4. The fact that the test succeeds on many machines implies that the > leader process is usually doing *all* of the work. This is in itself not > very good. Even on the machines where it fails, the fact that the tuple > counts are usually a pretty large fraction of the expected values

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-18 Thread Amit Kapila
On Fri, Aug 18, 2017 at 3:50 AM, Tom Lane wrote: > I wrote: >> Ah-hah, I see my dromedary box is one of the ones failing, so I'll >> have a look there. I can't reproduce it on my other machines. > > OK, so this is a whole lot more broken than I thought :-(. > > Bear in mind that the plan for this

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
I wrote: > Ah-hah, I see my dromedary box is one of the ones failing, so I'll > have a look there. I can't reproduce it on my other machines. OK, so this is a whole lot more broken than I thought :-(. Bear in mind that the plan for this (omitting uninteresting detail) is Nested Loop Left Join

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
Robert Haas writes: > On Thu, Aug 17, 2017 at 2:06 PM, Tom Lane wrote: >> Nope, spoke too soon. See buildfarm. > Whoa, that's not good. Ah-hah, I see my dromedary box is one of the ones failing, so I'll have a look there. I can't reproduce it on my other machines. I'm a bit suspicious that i

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Robert Haas
On Thu, Aug 17, 2017 at 2:06 PM, Tom Lane wrote: > I wrote: >> Pushed, with minor tidying of the test case. I think we can now >> close this open item. > > Nope, spoke too soon. See buildfarm. > > (Man, am I glad I insisted on a test case.) Whoa, that's not good. -- Robert Haas EnterpriseDB:

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
I wrote: > Pushed, with minor tidying of the test case. I think we can now > close this open item. Nope, spoke too soon. See buildfarm. (Man, am I glad I insisted on a test case.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
Amit Kapila writes: >> This will be fixed by the patch [1] (execrescan_gathermerge_v2.patch) >> I posted sometime back. The test case is slightly different, but may >> I can re post the patch with your test case. > I have kept the fix as it is but changed the test to match your test. Pushed, wi

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
Amit Kapila writes: > I think the another patch posted above to add a new guc for > enable_gather is still relevant and important. FWIW, I'm pretty much -1 on that. I don't see that it has any real-world use; somebody who wants to turn that off would likely really want to turn off parallelism al

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Amit Kapila
On Thu, Aug 17, 2017 at 8:08 PM, Amit Kapila wrote: > On Thu, Aug 17, 2017 at 7:49 PM, Tom Lane wrote: >> Amit Kapila writes: >>> On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane wrote: I should think it wouldn't be that expensive to create a test case, if you already have test cases that in

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Amit Kapila
On Thu, Aug 17, 2017 at 7:49 PM, Tom Lane wrote: > Amit Kapila writes: >> On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane wrote: >>> I should think it wouldn't be that expensive to create a test >>> case, if you already have test cases that invoke GatherMerge. >>> Adding a right join against a VALUES

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Tom Lane
Amit Kapila writes: > On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane wrote: >> I should think it wouldn't be that expensive to create a test >> case, if you already have test cases that invoke GatherMerge. >> Adding a right join against a VALUES clause with a small number of >> entries, and a non-merg

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-17 Thread Amit Kapila
On Thu, Aug 17, 2017 at 10:07 AM, Amit Kapila wrote: > On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane wrote: >> >>> I believe that between this commit and the test-coverage commit from >>> Andres, this open item is reasonably well addressed. If someone >>> thinks more needs to be done, please specify

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-16 Thread Thomas Munro
On Thu, Aug 17, 2017 at 4:37 PM, Amit Kapila wrote: > Note - enable_gathermerge is not present in postgresql.conf. I think > we should add it in the postgresql.conf.sample file. +1 See also https://www.postgresql.org/message-id/CAEepm=0b7ym9mzsviq1d-hnt4koarvejvsfayvfyknv-pvd...@mail.gmail.com

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-16 Thread Amit Kapila
On Tue, Aug 15, 2017 at 7:16 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Aug 15, 2017 at 7:31 AM, Amit Kapila wrote: >>> Attached patch fixes the issue for me. I have locally verified that >>> the gather merge gets executed in rescan path. I haven't added a test >>> case for the same

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Tatsuo Ishii
> On Thu, Aug 17, 2017 at 9:15 AM, Tatsuo Ishii wrote: >>> He meant logical replication, >> >> Oh I could not find he meant logical replication in the original >> report. > > The second message of the thread says so, but the first does not > mention logical replication at all. >>From here are men

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Michael Paquier
On Thu, Aug 17, 2017 at 9:15 AM, Tatsuo Ishii wrote: >> He meant logical replication, > > Oh I could not find he meant logical replication in the original > report. The second message of the thread says so, but the first does not mention logical replication at all. >From here are mentioned PG 9.6

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Tatsuo Ishii
> He meant logical replication, Oh I could not find he meant logical replication in the original report. > but the code in question here is the same > for streaming replication, or whatever it's called. Yes. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/inde

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Tatsuo Ishii
> In practice it's largely about architecture, I think. You definitely > need the same endianness and floating-point format, which are hardware, > and you need the same MAXALIGN, which is partly hardware but in principle > could be chosen differently in different ABI conventions for the same > har

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Peter Eisentraut
On 8/16/17 19:41, Michael Paquier wrote: > On Thu, Aug 17, 2017 at 8:13 AM, Tatsuo Ishii wrote: >> Do we allow streaming replication among different OS? > > No. WAL is a binary format. > >> I thought it is >> required that primary and standbys are same platform (in my >> understanding this means

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Tom Lane
Michael Paquier writes: > On Thu, Aug 17, 2017 at 8:13 AM, Tatsuo Ishii wrote: >> Do we allow streaming replication among different OS? > No. WAL is a binary format. >> I thought it is >> required that primary and standbys are same platform (in my >> understanding this means the same hardware a

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Michael Paquier
On Thu, Aug 17, 2017 at 8:13 AM, Tatsuo Ishii wrote: > Do we allow streaming replication among different OS? No. WAL is a binary format. > I thought it is > required that primary and standbys are same platform (in my > understanding this means the same hardware architecture and OS) in > streamin

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Tatsuo Ishii
> Thank you Tom Lane, > This patch fixes the problem. > > With this patch, streaming replication started working (replication to > Windows) > > (Tested for Linux to Windows replication) Do we allow streaming replication among different OS? I thought it is required that primary and standbys are s

Re: [HACKERS] [BUGS] [postgresql 10 beta3] unrecognized node type: 90

2017-08-16 Thread Robert Haas
On Tue, Aug 15, 2017 at 9:46 AM, Tom Lane wrote: > How big a deal do we think test coverage is? It looks like > ExecReScanGatherMerge is identical logic to ExecReScanGather, > which *is* covered according to coverage.postgresql.org, but > it wouldn't be too surprising if they diverge in future. >

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-16 Thread Augustine, Jobin
This patch is fixing similar problem reported in Logical replication also by another user. Message: 20170524173644.1440.53...@wrigleys.postgresql.org Bug reference: 14669 Logged by: Igor Neym

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-15 Thread Tom Lane
Andres Freund writes: > I don't see any relevant uses of sockets in older branches, did I miss > something? No, I think we don't need to go back further than v10. Even if it turns out we do, I'd just as soon let this get a bit of field testing first. regards, tom lane

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-15 Thread Andres Freund
On August 15, 2017 8:07:43 AM PDT, Andres Freund wrote: > > >On August 15, 2017 7:04:59 AM PDT, Tom Lane wrote: >>Peter Eisentraut writes: >>> On 8/14/17 10:57, Tom Lane wrote: I think we could commit add-connected-event-2.patch and call this issue resolved. >> >>> Would you like to

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-15 Thread Andres Freund
On August 15, 2017 7:04:59 AM PDT, Tom Lane wrote: >Peter Eisentraut writes: >> On 8/14/17 10:57, Tom Lane wrote: >>> I think we could commit add-connected-event-2.patch and call this >>> issue resolved. > >> Would you like to commit your patch then? > >It's really Andres' patch, but I can push

Re: [HACKERS] [BUGS] Replication to Postgres 10 on Windows is broken

2017-08-15 Thread Tom Lane
Peter Eisentraut writes: > On 8/14/17 10:57, Tom Lane wrote: >> I think we could commit add-connected-event-2.patch and call this >> issue resolved. > Would you like to commit your patch then? It's really Andres' patch, but I can push it. regards, tom lane -- Sent via

  1   2   3   4   5   6   7   8   9   10   >