On Sat, Jun 30, 2018 at 5:38 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > I also think that there's some horribly unsafe coding in > apply_scanjoin_target_to_paths: it clobbers a RelOptInfo's reltarget, > with no thought for whether that might affect things elsewhere, > and it also clobbers individual paths' PathTargets like so: > > if (tlist_same_exprs) > subpath->pathtarget->sortgrouprefs = > scanjoin_target->sortgrouprefs; > > with similarly little regard for whether those PathTarget structures are > shared with anything else.
Regarding this point specifically, I definitely was not thrilled about that hack, but it was tricky to avoid having the tlist_same_exprs case regress in terms of performance, so I didn't feel I could turn down performance optimizations without some evidence of an actual problem. For this to be a problem, it would have to be the case that somebody builds a path, returns it, and then later mutates the sortgrouprefs array in place. Does that happen? Off-hand I would have thought that you'd have to apply the correct sortgroupref labeling to a path before using it as a building block in some other path, just like it's not really OK to change the cost of a path once you've passed it to add_path() or add_partial_path(). Decisions might have already been made based on the old information. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
