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

On 2021/04/22 17:56, Justin Pryzby wrote:
> On Thu, Apr 22, 2021 at 03:36:25PM +0900, Fujii Masao wrote:
>> diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
>> index 553524553b..69aa66e73e 100644
>> --- a/doc/src/sgml/fdwhandler.sgml
>> +++ b/doc/src/sgml/fdwhandler.sgml
>> @@ -1076,27 +1076,25 @@ ExecForeignTruncate(List *rels,
>>                       bool restart_seqs);
>>       <para>
>> -     <literal>behavior</literal> defines how foreign tables should
>> -     be truncated, using as possible values <literal>DROP_RESTRICT</literal>,
>> -     which means that <literal>RESTRICT</literal> option is specified,
>> -     and <literal>DROP_CASCADE</literal>, which means that
>> -     <literal>CASCADE</literal> option is specified, in
>> -     <command>TRUNCATE</command> command.
>> +     <literal>behavior</literal> is either <literal>DROP_RESTRICT</literal>
>> +     or <literal>DROP_CASCADE</literal>, which indicates that the
>> +     <literal>RESTRICT</literal> or <literal>CASCADE</literal> option was
>> +     requested in the original <command>TRUNCATE</command> command,
>> +     respectively.
> 
> Now that I reread this, I would change "which indicates" to "indicating".

Fixed. Thanks for reviewing the patch!
I will post the updated version of the patch later.


> 
>> -     <literal>restart_seqs</literal> is set to <literal>true</literal>
>> -     if <literal>RESTART IDENTITY</literal> option is specified in
>> -     <command>TRUNCATE</command> command.  It is <literal>false</literal>
>> -     if <literal>CONTINUE IDENTITY</literal> option is specified.
>> +     If <literal>restart_seqs</literal> is <literal>true</literal>,
>> +     the original <command>TRUNCATE</command> command requested the
>> +     <literal>RESTART IDENTITY</literal> option, otherwise
>> +     <literal>CONTINUE IDENTITY</literal> option.
> 
> should it say "specified" instead of requested ?
> Or should it say "requested the RESTART IDENTITY behavior" ?
> 
> Also, I think it should say "..otherwise, the CONTINUE IDENTITY behavior was
> requested".

Fixed.

  
>> +++ b/doc/src/sgml/ref/truncate.sgml
>> @@ -173,7 +173,7 @@ TRUNCATE [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [
>>   
>>     <para>
>>      <command>TRUNCATE</command> can be used for foreign tables if
>> -   the foreign data wrapper supports, for instance,
>> +   supported by the foreign data wrapper, for instance,
>>      see <xref linkend="postgres-fdw"/>.
> 
> what does "for instance" mean here?  I think it should be removed.

Removed.


> 
>> +++ b/doc/src/sgml/fdwhandler.sgml
>> @@ -1111,6 +1099,15 @@ ExecForeignTruncate(List *rels, List *rels_extra,
>>        if <literal>CONTINUE IDENTITY</literal> option is specified.
>>       </para>
>>   
>> +    <para>
>> +     Note that information about <literal>ONLY</literal> options specified
>> +     in the original <command>TRUNCATE</command> command is not passed to
>> +     <function>ExecForeignTruncate</function>.  This is the same behavior as
>> +     for the callback functions for <command>SELECT</command>,
>> +     <command>UPDATE</command> and  <command>DELETE</command> on
> 
> There's an extra space before DELETE

Fixed.


> 
>> diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
>> index 5320accf6f..d03731b7d4 100644
>> --- a/doc/src/sgml/postgres-fdw.sgml
>> +++ b/doc/src/sgml/postgres-fdw.sgml
>> @@ -69,6 +69,13 @@
>>     have privileges to do these things.)
>>    </para>
>>   
>> + <para>
>> +  Note that <literal>ONLY</literal> option specified in
> 
> add "the" to say: "the ONLY"

Fixed.


> 
>> +  <command>SELECT</command>, <command>UPDATE</command>,
>> +  <command>DELETE</command> or <command>TRUNCATE</command>
>> +  has no effect when accessing or modifyung the remote table.
> 
> modifying

Fixed.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION