Re: [HACKERS] Declarative partitioning - another take

On 2017/04/26 3:58, Robert Haas wrote:
> On Mon, Apr 24, 2017 at 6:43 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> The reason it doesn't work is that we do not allocate ResultRelInfos for
>> partitioned tables (not even for the root partitioned table in the
>> update/delete cases), because the current implementation assumes they are
>> not required.  That's fine only so long as we consider that no rows are
>> inserted into them, no indexes need to be updated, and that no row-level
>> triggers are to be fired.  But it misses the fact that we do allow
>> statement-level triggers on partitioned tables.  So, we should fix things
>> such that ResultRelInfos are allocated so that any statement-level
>> triggers are fired. But there are following questions to consider:
>>
>> 1. Should we consider only the root partitioned table or all partitioned
>>    tables in a given hierarchy, including the child partitioned tables?
>>    Note that this is related to a current limitation of updating/deleting
>>    inheritance hierarchies that we do not currently fire per-statement
>>    triggers of the child tables.  See the TODO item in wiki:
>>    https://wiki.postgresql.org/wiki/Todo#Triggers, which says: "When
>>    statement-level triggers are defined on a parent table, have them fire
>>    only on the parent table, and fire child table triggers only where
>>    appropriate"
>>
>> 2. Do we apply the same to inserts on the partitioned tables?  Since
>>    insert on a partitioned table implicitly affects its partitions, one
>>    might say that we would need to fire per-statement insert triggers of
>>    all the partitions.
> 
> It seems to me that it doesn't make a lot of sense to fire the
> triggers for some tables involved in the hierarchy and not others.  I
> suppose the consistent thing to do here is to make sure that we fire
> the statement triggers for all tables in the partitioning hierarchy
> for all operations (insert, update, delete, etc.).
> 
> TBH, I don't like that very much.  I'd rather fire the triggers only
> for the table actually named in the query and skip all the others,
> mostly because it seems like it would be faster and less likely to
> block future optimizations; eventually, I'd like to consider not even
> locking the children we're not touching, but that's not going to work
> if we have to check them all for triggers.  So what I'd prefer -- on
> the totally unprincipled basis that it would let us improve
> performance in the future -- if you operate on a partition directly,
> you fire the partition's triggers, but if you operate on the parent,
> only the parent's triggers fire.

FWIW, I too prefer the latter, that is, fire only the parent's triggers.
In that case, applying only the patch 0001 will do.

Thanks,
Amit