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
