On Wed, Nov 27, 2019 at 3:25 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Amit Langote <amitlangot...@gmail.com> writes: > > If inh_root_relid meant that, it would no longer be useful to > > examine_variable. In examine_variable, we need to map a child table's > > relid to the relid of its root parent table. If the root parent > > itself is under a UNION ALL subquery parent, then inh_root_relid of > > all relations in that ancestry chain would point to the UNION ALL > > subquery parent, which is not what examine_variable would want to use, > > because it's really looking for the root "table". > > Hm, I see. Still, the definition seems quite ad-hoc and of uncertain > usefulness to any other use-case. Given that checking permissions for > access to an expression index's stats is a pretty uncommon thing to > be doing, I don't really want to let it drive the definition of a > new RelOptInfo field. > > The other reason that I'm on the warpath against this field is that > it makes the patch un-back-patchable, and I'd like to be able to fix > this problem in the back branches.
Both arguments make sense. > Given the existence of the append_rel_array array, it's not really > difficult or expensive to use that to chain up to the root parent, > as in the attached simplified patch. We could only use this back > to v11 where append_rel_array was added, but that's still a lot > better than no back-patched fix at all. I agree. > I've not studied the test case too closely yet, other than to verify > that it does fail without the code fix :-). Other than that, though, > I think this patch is committable for v11 through HEAD. Thanks for committing. Regards, Amit
