Re: TRUNCATE on foreign tables

Hi,

The v3 patch updated the points below:
- 2nd arg of ExecForeignTruncate was changed to DropBehavior, not bool
- ExecuteTruncateGuts() uses a local hash table to track a pair of server-id
and list of the foreign tables managed by the server.
- Error message on truncate_check_rel() was revised as follows:
"foreign data wrapper \"%s\" on behalf of the foreign table \"%s\"
does not support TRUNCATE"
- deparseTruncateSql() of postgres_fdw generates entire remote SQL as
like other commands.
- Document SGML was updated.

Best regards,

2020年1月16日(木) 14:40 Michael Paquier <michael(at)paquier(dot)xyz>:
>
> On Wed, Jan 15, 2020 at 11:33:07PM +0900, Kohei KaiGai wrote:
> > 2020年1月15日(水) 17:11 Michael Paquier <michael(at)paquier(dot)xyz>:
> >> I have done a quick read through the patch. You have modified the
> >> patch to pass down to the callback a list of relation OIDs to execute
> >> one command for all, and there are tests for FKs so that coverage
> >> looks fine.
> >>
> >> Regression tests are failing with this patch:
> >> -- TRUNCATE doesn't work on foreign tables, either directly or
> >> recursively
> >> TRUNCATE ft2; -- ERROR
> >> -ERROR: "ft2" is not a table
> >> +ERROR: foreign-data wrapper "dummy" has no handler
> >> You visibly just need to update the output because no handlers are
> >> available for truncate in this case.
> >>
> > What error message is better in this case? It does not print "ft2" anywhere,
> > so user may not notice that "ft2" is the source of the error.
> > How about 'foreign table "ft2" does not support truncate' ?
>
> It sounds to me that this message is kind of right. This FDW "dummy"
> does not have any FDW handler at all, so it complains about it.
> Having no support for TRUNCATE is something that may happen after
> that. Actually, this error message from your patch used for a FDW
> which has a handler but no TRUNCATE support could be reworked:
> + if (!fdwroutine->ExecForeignTruncate)
> + ereport(ERROR,
> + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("\"%s\" is not a supported foreign table",
> + relname)));
> Something like "Foreign-data wrapper \"%s\" does not support
> TRUNCATE"?
>
> >> + <literal>frels_list</literal> is a list of foreign tables that are
> >> + connected to a particular foreign server; thus, these foreign tables
> >> + should have identical foreign server ID
> >> The list is built by the backend code, so that has to be true.
> >>
> >> + foreach (lc, frels_list)
> >> + {
> >> + Relation frel = lfirst(lc);
> >> + Oid frel_oid = RelationGetRelid(frel);
> >> +
> >> + if (server_id == GetForeignServerIdByRelId(frel_oid))
> >> + {
> >> + frels_list = foreach_delete_current(frels_list, lc);
> >> + curr_frels = lappend(curr_frels, frel);
> >> + }
> >> + }
> >> Wouldn't it be better to fill in a hash table for each server with a
> >> list of relations?
> >
> > It's just a matter of preference. A temporary hash-table with server-id
> > and list of foreign-tables is an idea. Let me try to revise.
>
> Thanks. It would not matter much for relations without inheritance
> children, but if truncating a partition tree with many foreign tables
> using various FDWs that could matter performance-wise.
> --
> Michael

--
HeteroDB, Inc / The PG-Strom Project
KaiGai Kohei <kaigai(at)heterodb(dot)com>