Re: TRUNCATE on foreign table

On 2021/04/16 15:13, Bharath Rupireddy wrote:
> On Fri, Apr 16, 2021 at 8:24 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> We are still discussing whether RESTRICT option should be pushed down to
>> a foreign data wrapper. But ISTM at least we could reach the consensus about
>> the drop of extra information for each foreign table. So what about applying
>> the attached patch and remove the extra information at first?
> 
> Thanks for the patch, here are some comments:

Thanks for the review!

> 
> 1) Maybe new empty lines would be better so that the code doesn't look
> cluttered:
>          relids = lappend_oid(relids, myrelid);   --> a new line after this.
>          /* Log this relation only if needed for logical decoding */
>          if (RelationIsLogicallyLogged(rel))
> 
>          relids = lappend_oid(relids, childrelid);  --> a new line after this.
>          /* Log this relation only if needed for logical decoding */
> 
>          relids = lappend_oid(relids, relid);  --> a new line after this.
>          /* Log this relation only if needed for logical decoding */
>          if (RelationIsLogicallyLogged(rel))

Applied. Attached is the updated version of the patch
(truncate_foreign_table_dont_pass_only_clause_v2.patch).
This patch includes the patch that Horiguchi-san posted upthead.
I'm thinking to commit this patch at first.



> 2) Instead of
>       on foreign tables.  <literal>rels</literal> is the list of
>       <structname>Relation</structname> data structure that indicates
>       a foreign table to truncate.
> 
> I think it is better with:
>       on foreign tables.  <literal>rels</literal> is the list of
>       <structname>Relation</structname> data structures, where each
>       entry indicates a foreign table to truncate.

Justin posted the patch that improves the documents including
this description. I think that we should revisit that patch.
Attached is the updated version of that patch.
(truncate_foreign_table_docs_v1.patch)


> 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?


> 4) Isn't it better to mention the "ONLY" option is not pushed to remote
> -- truncate with ONLY clause
> TRUNCATE ONLY tru_ftable_parent;
> 
> TRUNCATE ONLY tru_ftable;        -- truncate both parent and child
> SELECT count(*) FROM tru_ftable;   -- 0
> 
> 5) I may be missing something here, why is even after ONLY is ignored
> in the below truncate command, the sum is 126? Shouldn't it truncate
> both tru_ftable_parent and
> -- truncate with ONLY clause
> TRUNCATE ONLY tru_ftable_parent;
> SELECT sum(id) FROM tru_ftable_parent;  -- 126

Because TRUNCATE ONLY command doesn't truncate tru_ftable_child talbe
that inehrits tru_ftable_parent. No?

Regards,

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