> On May 15, 2017, at 6:49 AM, Mark Dilger <hornschnorter@gmail.com> wrote:
>
> I can confirm that this fixes the crash that I was seeing. I have read
> through the patch briefly, but will give it a more thorough review in the
> next few hours.
My only negative comment is that your patch follows a precedent of using
event trigger commands named for AlterTable for operations other than
an ALTER TABLE command. You clearly are not starting the precedent
here, as they are already used for IndexStmt and ViewStmt processing, but
expanding the precedent by using them in DefineRelation, where they were
not used before, seems to move in the wrong direction. Either the functions
should be renamed to something more general to avoid implying that the
function is ALTER TABLE specific, or different functions should be defined
and used, or ...? I am uncertain what the right solution would be.
Thoughts?
Mark Dilger