Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> writes: > While reviewing aggregate pushdown patch  we noticed that > RelOptInfos for upper relations do not have relids set.
Indeed, because they don't correspond to any particular scan relation or set of scan relations. I have in mind that in future releases, any particular upperrel might have its own definition of what the relids mean --- for example, in UPPERREL_SETOP it would likely be useful for the relids to represent the set of leaf SELECTs that have been merged in a particular path. You could imagine UPPERREL_WINDOW using the relids to track which window functions have been implemented in a path, whenever we get around to considering multiple window function orderings. None of that's there yet. > create_foreignscan_plan() copies the relids from RelOptInfo into > ForeignScan::fs_relids. That field is used to identify the RTIs > covered by the ForeignScan. That's fine for scan/join paths. If you want to pay attention to relids for an upper rel, it's up to you to know what they mean. I would not counsel assuming that they have anything at all to do with baserel RTIs. > We could prevent the crash by passing input_rel->relids to > fetch_upper_rel() in create_grouping_path() as seen in the attached > patch. I think this is fundamentally wrongheaded. If we go that route, the only valid relids for any upper path would be the union of all baserel RTIs, making it rather pointless to carry the value around at all, and definitely making it impossible to use the field to distinguish different partial implementations of the same upperrel. You should look to root->all_baserels, instead, if that's the value you want when considering an upperrel Path. regards, tom lane -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers