On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Oct 26, 2018 at 1:12 PM Amit Langote
> <Langote_Amit_f8@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 <tgl@sss.pgh.pa.us> wrote:
> > >> Amit Langote <amitlangote09@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
