On Wed, Apr 19, 2017 at 1:27 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapil...@gmail.com> 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 input expression is safe. > > I did not like changing the signature of is_parallel_safe() for this: > that's getting a lot of call sites involved in something they don't care > about, and I'm not even sure that it's formally correct. The key thing > here is that a Param could only be considered parallel-safe in context. > That is, if you looked at the testexpr alone and asked if it could be > pushed down to a worker, the answer would have to be "no". Only when > you look at the whole SubPlan tree and ask if it can be pushed down > should the answer be "yes". Therefore, I feel pretty strongly that > what we want is for the safe_param_ids whitelist to be mutated > internally in is_parallel_safe() as it descends the tree. That way > it can give the right answer when considering a fragment of a plan. > I've committed a patch that does it like that. >
Thanks. >> ... Also, I think we don't need to check >> parallel-safety for splan->args as that also is related to correlated >> queries for which parallelism is not supported as of now. > > 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 would never be parallel-unsafe --- but > primnodes.h is pretty clear that one shouldn't assume that it will > stay that way. Sure, but the point I was trying to make was whenever subplan has args, I think it won't be parallel-safe as those args are used to pass params. > So it's better to recurse over the whole tree rather > than ignoring parts of it, especially if you're not going to document > the assumption you're making anywhere. > No problem. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers