Re: [HACKERS] Inadequate parallel-safety check for SubPlans

Amit Kapila writes: > On Wed, Apr 19, 2017 at 1:27 AM, Tom Lane wrote: >> I think leaving that sort of thing out is just creating a latent bug >> that is certain to bite you later. It's true that as long as the args >> list contains only Vars, it

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Wed, Apr 19, 2017 at 1:27 AM, Tom Lane wrote: > Amit Kapila writes: >> I have ended up doing something along the lines suggested by you (or >> at least what I have understood from your e-mail). Basically, pass >> the safe-param-ids list to

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

Amit Kapila writes: > I have ended up doing something along the lines suggested by you (or > at least what I have understood from your e-mail). Basically, pass > the safe-param-ids list to parallel safety function and decide based > on that if Param node reference in

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Mon, Apr 17, 2017 at 10:54 AM, Tom Lane wrote: > Amit Kapila writes: >> On Sun, Apr 16, 2017 at 9:30 PM, Tom Lane wrote: >>> FYI, I have this on my to-look-at list, and expect to fix it before Robert >>> returns from vacation.

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Mon, Apr 17, 2017 at 1:24 AM, Tom Lane wrote: > Amit Kapila writes: >> On Sun, Apr 16, 2017 at 9:30 PM, Tom Lane wrote: >>> FYI, I have this on my to-look-at list, and expect to fix it before Robert >>> returns from vacation. >

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

Amit Kapila writes: > On Sun, Apr 16, 2017 at 9:30 PM, Tom Lane wrote: >> FYI, I have this on my to-look-at list, and expect to fix it before Robert >> returns from vacation. > Let me know if an initial patch by someone else can be helpful? Sure,

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Sun, Apr 16, 2017 at 9:30 PM, Tom Lane wrote: > Noah Misch writes: >> On Wed, Apr 12, 2017 at 02:41:09PM -0400, Tom Lane wrote: >>> This is 100% wrong. It's failing to recurse into the subexpressions of >>> the SubPlan, which could very easily include

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

Noah Misch writes: > On Wed, Apr 12, 2017 at 02:41:09PM -0400, Tom Lane wrote: >> This is 100% wrong. It's failing to recurse into the subexpressions of >> the SubPlan, which could very easily include parallel-unsafe function >> calls. Example: > The above-described topic is

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Wed, Apr 12, 2017 at 02:41:09PM -0400, Tom Lane wrote: > While poking at the question of parallel_safe marking for Plans, > I noticed that max_parallel_hazard_walker() does this: > > /* We can push the subplans only if they are parallel-safe. */ > else if (IsA(node, SubPlan)) >

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Thu, Apr 13, 2017 at 1:05 AM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Apr 12, 2017 at 2:41 PM, Tom Lane wrote: >>> This is 100% wrong. It's failing to recurse into the subexpressions of >>> the SubPlan, which could very

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

Robert Haas writes: > On Wed, Apr 12, 2017 at 2:41 PM, Tom Lane wrote: >> This is 100% wrong. It's failing to recurse into the subexpressions of >> the SubPlan, which could very easily include parallel-unsafe function >> calls. > My understanding

Re: [HACKERS] Inadequate parallel-safety check for SubPlans

On Wed, Apr 12, 2017 at 2:41 PM, Tom Lane wrote: > While poking at the question of parallel_safe marking for Plans, > I noticed that max_parallel_hazard_walker() does this: > > /* We can push the subplans only if they are parallel-safe. */ > else if (IsA(node,

[HACKERS] Inadequate parallel-safety check for SubPlans

While poking at the question of parallel_safe marking for Plans, I noticed that max_parallel_hazard_walker() does this: /* We can push the subplans only if they are parallel-safe. */ else if (IsA(node, SubPlan)) return !((SubPlan *) node)->parallel_safe; This is 100% wrong. It's