On Fri, Apr 28, 2017 at 2:13 AM, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > It seems to me that there is no difference in behavior between > inheritance-based and declarative partitioning as far as statement-level > triggers are concerned (at least currently). In both the cases, we fire > statement-level triggers only for the table specified in the command.
OK. >>> By the way, code changes I made in the attached are such that a subsequent >>> patch could implement firing statement-level triggers of all the tables in >>> a partition hierarchy, which it seems we don't want to do. Should then >>> the code be changed to not create ResultRelInfos of all the tables but >>> only the root table (the one mentioned in the command)? You will see that >>> the patch adds fields named es_nonleaf_result_relations and >>> es_num_nonleaf_result_relations, whereas just es_root_result_relation >>> would perhaps do, for example. >> >> It seems better not to create any ResultRelInfos that we don't >> actually need, so +1 for such a revision to the patch. > > OK, done. It took a bit more work than I thought. So, this seems weird, because rootResultRelIndex is initialized even when splan->partitioned_rels == NIL, but isn't actually valid in that case. ExecInitModifyTable seems to think it's always valid, though. I think the way that you've refactored fireBSTriggers and fireASTriggers is a bit confusing. Instead of splitting out a separate function, how about just having the existing function begin with if (node->rootResultRelInfo) resultRelInfo = node->rootResultRelInfo; else resultRelInfo = node->resultRelInfo; ? I think the way you've coded it is a holdover from the earlier design where you were going to call it multiple times, but now that's not needed. Looks OK, otherwise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
