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

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

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

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); >

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

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 >

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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 >

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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. --

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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 as

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

On Tue, Aug 15, 2017 at 7:31 AM, Amit Kapila wrote: > On Tue, Aug 15, 2017 at 8:22 AM, Noah Misch wrote: >> On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote: >>> "Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" >>>

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

On Tue, Aug 15, 2017 at 8:22 AM, Noah Misch wrote: > On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote: >> "Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" >> writes: >> > ERROR: XX000: unrecognized node type: 90 >> > LOCATION: ExecReScan,