Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Tue, Jun 6, 2017 at 2:15 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> FWIW, parse analysis is surely NOT the time for such a check. Triggers
>> might get added to a table between analysis and execution. I think you
>> might have to do it during executor startup.
> Hmm. My understanding: In analyzeCTE(), where the check is done in
> that patch, we hold a lock on the relation because we have a
> RangeTblEntry. That means that no triggers can be added concurrently
> in the case where you go on to execute the plan. If you save the plan
> for later execution, triggers created between then and lock
> reacquisition (because creating triggers touches pg_class) cause
> reanalysis.
No, they don't necessarily. One case that will certainly not be
covered by such a test is if the wCTE is an UPDATE or DELETE on
an inheritance hierarchy: parse analysis has no idea whether the
target table has children at all, let alone whether any of the
children have triggers that use transition tables. You really want
the test to happen after the planner has expanded inheritance.
Another problem is that the result of parse analysis is frozen for much
longer than you're thinking about in the case of stored rules or views.
We currently disallow views that contain writable CTEs, but I do not
think there's such a prohibition for rules.
More generally, the problem I've got with this proposal is that it causes
the presence of triggers to be significant much earlier in query execution
than they used to be. I'm concerned that this may break optimizations
that are already in place somewhere, or that we might want to add later.
regards, tom lane