Re: Partitioned tables and relfilenode

On Wed, Mar 8, 2017 at 5:36 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I don't quite understand the need for the change to set_rel_size().
>> The rte->inh case is handled before reaching the new code, and IIUC
>> the !rte->inh case should never happen given the changes to
>> expand_inherited_rtentry.  Or am I confused?
>
> In expand_inherited_rtentry(), patch only changes the rule about the
> minimum number of child RTEs required to keep rte->inh true.  In the
> traditional case, it is 2 (the table itself and at least one child).  For
> a partitioned table, since we don't want to scan the table itself, that
> becomes 1 (at least one leaf partition).
>
> So, it's still possible for rte->inh to become false if the required
> minimum is not met, even for partitioned tables.

OK.

>> -    rel = mtstate->resultRelInfo->ri_RelationDesc;
>> +    nominalRTE = rt_fetch(node->nominalRelation, estate->es_range_table);
>> +    nominalRel = heap_open(nominalRTE->relid, NoLock);
>>
>> No lock?
>
> Hmm, I think I missed that a partitioned parent table would not be locked
> by the *executor* at all after applying this patch.  As of now, InitPlan()
> takes care of locking all the result relations in the
> PlannedStmt.resultRelations list.  This patch removes partitioned
> parent(s) from appearing in this list.  But that makes me wonder - aren't
> the locks taken by expand_inherited_rtentry() kept long enough?  Why does
> InitPlan need to acquire them again?  I see this comment in
> expand_inherited_rtentry:

Parse-time locks, plan-time locks, and execution-time locks are all
separate.  See the header comments for AcquireRewriteLocks in
rewriteHandler.c; think about a view, where the parsed query has been
stored and is later rewritten and planned.  Similarly, a plan can be
reused even after the transaction that created that plan has
committed; see AcquireExecutorLocks in plancache.c.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company