Tom Lane <tgl@sss.pgh.pa.us> writes:
> Yeah, I was just looking at the IfSupported variant. In the structure
> I just suggested (separate ProcessSlowUtility function), we could make
> that work by having switch cases for some statements in both functions,
I've done it the way you propose here, and then in the Slow variant we
have two set of cases again: those with some manual transactionnal
behavior or some other code complexities, and the really simple ones.
The attached patch involves a second layer of distinction to simplify
the code fully and remove all the Event Trigger related macrology that I
didn't much like. Maybe that's going a tad too far, see what you think.
Of course the patch passes make check.
Also, some switch cases are now duplicated for the sole benefit of
keeping some compiler help, but I don't know how much help we're talking
about now that we have 3 different switches. It seemed cleaner to do it
that way as it allows to ERROR out in the very last default case.
Finally, I've been surprised to find out that those cases are only
triggering for "ddl_event_start" (and not "ddl_event_end"), I think
that's a bug we should be fixing:
case T_AlterDomainStmt:
case T_DefineStmt:
case T_IndexStmt: /* CREATE INDEX */
The T_IndexStmt should be triggering ddl_event_end when stmt->concurrent
is false, and that's not the case in the code in the master's branch.
The two other cases should just be moved to the later switch so that
both start and end triggers happen.
Or maybe I'm missing something here. Given that it might as well be the
case, I did only refactor the code keeping its current behavior rather
than fixing what I think is a bug while doing so.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
