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(dot)fujii(at)oss(dot)nttdata(dot)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