2022年7月8日(金) 17:10 Yugo NAGATA <nag...@sraoss.co.jp>:
>
> On Fri, 8 Jul 2022 16:50:10 +0900
> Ian Lawrence Barwick <barw...@gmail.com> wrote:
>
> > 2022年7月8日(金) 14:06 Fujii Masao <masao.fu...@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
