Re: TRUNCATE on foreign table

Onishi-san,

The v5 patch contains full-contents of "src/backend/commands/tablecmds.c.orig".
Please check it.

2021年3月28日(日) 2:37 Kazutaka Onishi <onishi(at)heterodb(dot)com>:
>
> Fujii-san,
>
> Thank you for your review!
> Now I prepare v5 patch and I'll answer to your each comment. please
> check this again.
> m(_ _)m
>
> 1. In postgres-fdw.sgml, "and truncatable" should be appended into the
> above first description?
> 2. truncate.sgml should be updated because, for example, it contains
> the above descriptions.
>
> Yeah, you're right. I've fixed it.
>
>
>
> 3. Don't we need to document the detail information about frels_extra?
>
> I've written about frels_extra into fdwhander.sgml.
>
>
>
> 4. postgres_fdw determines whether to specify ONLY or not by checking
> whether the passed extra value is zero or not.
>
> Please refer this:
> https://www.postgresql.org/message-id/CAOP8fzb-t3WVNLjGMC%2B4sV4AZa9S%3DMAQ7Q6pQoADMCf_1jp4ew%40mail.gmail.com
> > Negative value means that foreign-tables are not specified in the TRUNCATE
> > command, but truncated due to dependency (like partition's child leaf).
>
> I've added this information into fdwhandler.sgml.
>
>
>
> 5. For example, we can easily do that by truncate foreign tables
> before local ones. Thought?
>
> Umm... yeah, I feel it's better procedure, but not so required because
> TRUNCATE is NOT called frequently.
> Certainly, we already have postgresIsForeignUpdatable() to check
> whether the foreign table is updatable or not.
> Following this way, we have to add postgresIsForeignTruncatable() to check.
> However, Unlike UPDATE, TRUNCATE is NOT called frequently. Current
> procedure is inefficient but works correctly.
> Thus, I feel postgresIsForeignTruncatable() is not needed.
>
>
> 6. XLOG_HEAP_TRUNCATE record is written even for the truncation of a
> foreign table. Why is this necessary?
>
> Please give us more time to investigate this.
>
> 2021年3月25日(木) 3:47 Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>:
> >
> >
> >
> > On 2021/03/13 18:57, Kazutaka Onishi wrote:
> > > I have fixed the patch to pass check-world test. :D
> >
> > Thanks for updating the patch! Here are some review comments from me.
> >
> >
> > By default all foreign tables using <filename>postgres_fdw</filename> are assumed
> > to be updatable. This may be overridden using the following option:
> >
> > In postgres-fdw.sgml, "and truncatable" should be appended into
> > the above first description? Also "option" in the second description
> > should be a plural form "options"?
> >
> >
> > <command>TRUNCATE</command> is not currently supported for foreign tables.
> > This implies that if a specified table has any descendant tables that are
> > foreign, the command will fail.
> >
> > truncate.sgml should be updated because, for example, it contains
> > the above descriptions.
> >
> >
> > + <literal>frels_extra</literal> is same length with
> > + <literal>frels_list</literal>, that delivers extra information of
> > + the context where the foreign-tables are truncated.
> > + </para>
> >
> > Don't we need to document the detail information about frels_extra?
> > Otherwise the developers of FDW would fail to understand how to
> > handle the frels_extra when trying to make their FDWs support TRUNCATE.
> >
> >
> > + relids_extra = lappend_int(relids_extra, (recurse ? 0 : 1));
> > + relids_extra = lappend_int(relids_extra, -1);
> >
> > postgres_fdw determines whether to specify ONLY or not by checking
> > whether the passed extra value is zero or not. That is, for example,
> > using only 0 and 1 for extra values is enough for the purpose. But
> > ExecuteTruncate() sets three values 0, -1 and 1 as extra ones. Why are
> > these three values necessary?
> >
> >
> > With the patch, if both local and foreign tables are specified as
> > the target tables to truncate, TRUNCATE command tries to truncate
> > foreign tables after truncating local ones. That is, if "truncatable"
> > option is set to false or enough permission to truncate is not granted
> > yet in the foreign server, an error will be thrown after the local tables
> > are truncated. I don't think this is good order of processings. IMO,
> > instead, we should check whether foreign tables can be truncated
> > before any actual truncation operations. For example, we can easily
> > do that by truncate foreign tables before local ones. Thought?
> >
> >
> > XLOG_HEAP_TRUNCATE record is written even for the truncation of
> > a foreign table. Why is this necessary?
> >
> > Regards,
> >
> > --
> > Fujii Masao
> > Advanced Computing Technology Center
> > Research and Development Headquarters
> > NTT DATA CORPORATION

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