Re: TRUNCATE on foreign table

On 2021/04/22 20:27, Bharath Rupireddy wrote:
> On Thu, Apr 22, 2021 at 12:06 PM Fujii Masao
> <masao.fujii@oss.nttdata.com> wrote:
>> On 2021/04/22 9:39, Bharath Rupireddy wrote:
>>> One comment on truncate_foreign_table_docs_v1.patch:
>>> 1) I think it is "to be truncated"
>>> +     <literal>rels</literal> is a list of <structname>Relation</structname>
>>> +     data structures for each foreign table to truncated.
>>
>> Fixed. Thanks!
>>
>>> How about a slightly changed phrasing like below?
>>> +     <literal>rels</literal> is a list of <structname>Relation</structname>
>>> +     data structures of foreign tables to truncate.
>> Either works at least for me. If you think that this phrasing is
>> more precise or better, I'm ok with that and will update the patch again.
> 
> IMO, "rels is a list of Relation data structures of foreign tables to
> truncate." looks better.

Fixed.

Thanks for reviewing the patches.
Attached are the updated versions of the patches.
These patches include the fixes pointed by Justin.


> 
>>>>> 3) How about adding an extra para(after below para in
>>>>> postgres_fdw.sgml) on WHY we don't push "ONLY" to foreign tables while
>>>>> truncating? We could add to the same para for other options if at all
>>>>> we don't choose to push them.
>>>>>      <command>DELETE</command>, or <command>TRUNCATE</command>.
>>>>>      (Of course, the remote user you have specified in your user mapping must
>>>>>      have privileges to do these things.)
>>>>
>>>> I agree to document the behavior that ONLY option is always ignored
>>>> for foreign tables. But I'm not sure if we can document WHY.
>>>> Because I could not find the past discussion about why ONLY option is
>>>> ignored on SELECT, etc... Maybe it's enough to document the behavior?
>>>
>>> +1 to specify in the documentation about ONLY option is always
>>> ignored.
>>
>> Added.
>>
>>> But can we specify the WHY part within deparseTruncateSql, it
>>> will be there for developer reference? I feel it's better if this
>>> change goes with truncate_foreign_table_dont_pass_only_clause_v2.patch
>>
>> I added this information into fdwhandler.sgml because the developers
>> usually read fdwhandler.sgml.
> 
> Thanks!
> 
> +    <para>
> +     Note that information about <literal>ONLY</literal> options specified
> +     in the original <command>TRUNCATE</command> command is not passed to
> 
> I think it is not "information about", no? We just don't pass ONLY
> option  instead we skip it. IMO, we can say "Note that the ONLY option
> specified with a foreign table in the original TRUNCATE command is
> skipped and not passed to ExecForeignTruncate."

Probably I still fail to understand your point.
But if "information about" is confusing, I'm ok to
remove that. Fixed.


> 
> +     <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
> +     a foreign table.
> 
> How about "This behaviour is similar to the callback functions of
> SELECT, UPDATE, DELETE on a foreign table"?

Fixed.

Regards,

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