Re: Support TRUNCATE triggers on foreign tables

2022年7月8日(金) 17:10 Yugo NAGATA <nagata@sraoss.co.jp>:
>
> On Fri, 8 Jul 2022 16:50:10 +0900
> Ian Lawrence Barwick <barwick@gmail.com> wrote:
>
> > 2022年7月8日(金) 14:06 Fujii Masao <masao.fujii@oss.nttdata.com>:
> > > On 2022/07/08 11:19, Yugo NAGATA wrote:
> > > >> You added "foreign tables" for BEFORE statement-level trigger as the above, but ISTM that you also needs to do that for AFTER statement-level trigger. No?
> > > >
> > > > Oops, I forgot it. I attached the updated patch.
> > >
> > > Thanks for updating the patch! LGTM.
> > > Barring any objection, I will commit the patch.
> >
> > An observation: as-is the patch would make it possible to create a truncate
> > trigger for a foreign table whose FDW doesn't support truncation, which seems
> > somewhat pointless, possible source of confusion etc.:
> >
> >     postgres=# CREATE TRIGGER ft_trigger
> >       AFTER TRUNCATE ON fb_foo
> >       EXECUTE FUNCTION fb_foo_trg();
> >     CREATE TRIGGER
> >
> >     postgres=# TRUNCATE fb_foo;
> >     ERROR:  cannot truncate foreign table "fb_foo"
> >
> > It would be easy enough to check for this, e.g.:
> >
> >     else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
> >     {
> >         FdwRoutine *fdwroutine = GetFdwRoutineForRelation(rel, false);
> >
> >         if (!fdwroutine->ExecForeignTruncate)
> >             ereport(ERROR,
> >                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> >                      errmsg("foreign data wrapper does not support
> > table truncation")));
> >         ...
> >
> > which results in:
> >
> >     postgres=# CREATE TRIGGER ft_trigger
> >       AFTER TRUNCATE ON fb_foo
> >       EXECUTE FUNCTION fb_foo_trg();
> >     ERROR:  foreign data wrapper does not support table truncation
> >
> > which IMO is preferable to silently accepting DDL which will never
> > actually do anything.
>
> At beginning, I also thought such check would be necessary, but I noticed that
> it is already possible to create insert/delete/update triggers for a foreign
> table whose FDW doesn't support such operations. So, I discarded this idea from
> the proposed patch for consistency.
>
> If we want to add such prevention, we will need similar checks for
> INSERT/DELETE/UPDATE not only TRUNCATE. However, I think such fix is independent
> from this and it can be proposed as another patch.

Ah OK, makes sense from that point of view. Thanks for the clarification!

Regards

Ian Barwick