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. > ... 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. 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. > I have also explored it to do it by checking parallel-safety of > testexpr before PARAM_EXEC params are replaced in testexpr, however > that won't work because we add PARAM_SUBLINK params at the time of > parse-analysis in transformSubLink(). AFAICS we don't do parallel-safety checks early enough to need to bother with that, so the existing handling of SubLink and PARAM_SUBLINK params should be fine. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers