Re: TRUNCATE on foreign tables - Mailing list pgsql-hackers

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@paquier.xyz>:
>
> On Wed, Jan 15, 2020 at 11:33:07PM +0900, Kohei KaiGai wrote:
> > 2020年1月15日(水) 17:11 Michael Paquier <michael@paquier.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@heterodb.com>