Re: Support TRUNCATE triggers on foreign tables

On Fri, 8 Jul 2022 16:50:10 +0900
Ian Lawrence Barwick <barwick(at)gmail(dot)com> wrote:

> 2022年7月8日(金) 14:06 Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>