|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Dilip Kumar <dilipbalaut(at)gmail(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Langote <amitlangote09(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: CVE-2017-7484-induced bugs, or, btree cmp functions are not leakproof?|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Thank you for creating the patch.
On 2018/10/28 20:35, Dilip Kumar wrote:
> On Sat, Oct 27, 2018 at 10:07 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> On Fri, Oct 26, 2018 at 1:12 PM Amit Langote wrote:
>>> On 2018/10/25 19:54, Dilip Kumar wrote:
>>>> 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 . I think that traversing
>>> append_rel_list every time would be inefficient.
>>>  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.
Sorry I forgot to cc you, but I'd posted a patch on the "speeding up
planning with partitions" thread, that's extracted from the bigger patch,
which adds inh_root_parent member to RelOptInfo . Find it attached
with this email.
One of the differences from your patch is that it makes inh_root_parent
work not just for partitioning, but to inheritance in general. Also, it
codes the changes to build_simple_rel to set inh_root_parent's value a bit
differently than your patch.
> Attached patch handles this issue.
I noticed a typo in your patch:
transalate_varattno -> translate_varattno
+transalate_varattno(Oid oldrelid, Oid newrelid, int old_attno)
+ Relation oldrelation = heap_open(oldrelid, NoLock);
It doesn't seem nice that it performs a heap_open on the parent relation.
+ att = TupleDescAttr(old_tupdesc, old_attno - 1);
+ attname = NameStr(att->attname);
+ newtup = SearchSysCacheAttName(newrelid, attname);
+ if (!newtup)
+ heap_close(oldrelation, NoLock);
+ return InvalidAttrNumber;
+ varattno = transalate_varattno(relid, rte->relid, var->varattno);
+ if (AttributeNumberIsValid(varattno))
+ relid = rte->relid;
+ varattno = var->varattno;
It's not possible for varattno to be invalid here, because the query on
inheritance parent only allows to select parent's columns, so we'd error
out before getting here if a column not present in the parent were selected.
Anyway, why don't we just use the child table's AppendRelInfo to get the
parent's version of varattno instead of creating a new function? It can
be done as shown in the attached revised version of the portion of the
patch changing selfuncs.c. Please take a look.
|Next Message||Pavan Deolasee||2018-10-29 09:23:54||Re: MERGE SQL statement for PG12|
|Previous Message||Pavel Stehule||2018-10-29 09:11:37||Re: PostgreSQL vs SQL/XML Standards|