On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Fri, Oct 26, 2018 at 1:12 PM Amit Langote > <langote_amit...@lab.ntt.co.jp> wrote: > > > > On 2018/10/25 19:54, Dilip Kumar wrote: > > > On Mon, Oct 22, 2018 at 7:47 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > >> Amit Langote <amitlangot...@gmail.com> writes: > > >>> But maybe for the case under question, that's irrelevant, because > > >>> we're only interested in access to inherited columns as those are the > > >>> only ones that can be accessed in queries via parent. > > >> > > >> Yeah, that's what I thought. It seems like it should be possible to base > > >> all stats access decisions off the table actually named in the query, > > >> because only columns appearing in that table could be referenced, and > > >> only > > >> that table's permissions actually get checked at runtime. > > >> > > >> I guess it's possible that a child table could have, say, an index on > > >> column X (inherited) and column Y (local) and that some aspect of costing > > >> might then be interested in the behavior of column Y, even though the > > >> query could only mention X not Y. But then we could fall back to the > > >> existing behavior. > > > > > > Basically, if the relation is RELOPT_OTHER_MEMBER_REL, we can > > > recursively fetch its parent until we reach to the base relation > > > (which is actually named in the query). And, once we have the base > > > relation we can check ACL on that and set vardata->acl_ok accordingly. > > > Additionally, for getting the parent RTI we need to traverse > > > "root->append_rel_list". Another alternative could be that we can add > > > parent_rti member in RelOptInfo structure. > > > > Adding parent_rti would be a better idea [1]. I think that traversing > > append_rel_list every time would be inefficient. > > [1] I've named it inh_root_parent in one of the patches I'm working on > > where I needed such a field (https://commitfest.postgresql.org/20/1778/) > Ok, Make sense. I have written a patch by adding this variable. > There is still one FIXME in the patch, basically, after getting the > baserel rte I need to convert child varattno to parent varattno > because in case of inheritance that can be different. Do we already > have any mapping from child attno to parent attno or we have to look > up the cache.
Attached patch handles this issue. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
bug_fix_childrel_stat_access_v2.patch
Description: Binary data
