Re: TRUNCATE on foreign table

Continuing previous review...

+ relids_extra = lappend_int(relids_extra,
TRUNCATE_REL_CONTEXT__CASCADED);

I wonder if TRUNCATE_REL_CONTEXT_CASCADING is better
than TRUNCATE_REL_CONTEXT__CASCADED. Note the removal of the extra
underscore.
In English, we say: truncation cascading to foreign table.

w.r.t. Bharath's question on using hash table, I think the reason is that
the search would be more efficient:

+ ft_info = hash_search(ft_htab, &server_oid, HASH_ENTER, &found);
and
+ while ((ft_info = hash_seq_search(&seq)) != NULL)

+ * Now go through the hash table, and process each entry associated to
the
+ * servers involved in the TRUNCATE.

associated to -> associated with

Should the hash table be released at the end of ExecuteTruncateGuts() ?

Cheers

On Sat, Apr 3, 2021 at 7:38 AM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:

> Hi,
> + <command>TRUNCATE</command> for each foreign server being involved
> + in one <command>TRUNCATE</command> command (note that invocations
>
> The 'being' in above sentence can be omitted.
>
> + the context where the foreign-tables are truncated. It is a list of
> integers and same length with
>
> There should be a verb between 'and' and same :
> It is a list of integers and has same length with
>
> + * Information related to truncation of foreign tables. This is used for
> + * the elements in a hash table *that* uses the server OID as lookup key,
>
> The 'uses' is for 'This' (the struct), so 'that' should be 'and':
>
> the elements in a hash table and uses
>
> Alternatively:
>
> the elements in a hash table. It uses
>
> + relids_extra = lappend_int(relids_extra, (recurse ?
> TRUNCATE_REL_CONTEXT__NORMAL : TRUNCATE_REL_CONTEXT__ONLY));
>
> I am curious: isn't one underscore enough in the identifier (before NORMAL
> and ONLY) ?
>
> I suggest naming them TRUNCATE_REL_CONTEXT_NORMAL and
> TRUNCATE_REL_CONTEXT_ONLY
>
> Cheers
>
> On Sat, Apr 3, 2021 at 6:46 AM Kazutaka Onishi <onishi(at)heterodb(dot)com>
> wrote:
>
>> Sorry but I found the v7 patch has typo and it can't be built...
>> I attached fixed one(v8).
>>
>> 2021年4月3日(土) 9:53 Kazutaka Onishi <onishi(at)heterodb(dot)com>:
>> >
>> > All,
>> >
>> > Thank you for discussion.
>> > I've updated the patch (v6->v7) according to the conclusion.
>> >
>> > I'll show the modified points:
>> > 1. Comments for ExecuteTuncate()
>> > 2. Replacing extra value in frels_extra with integer to label.
>> > 3. Skipping XLOG_HEAP_TRUNCATE on foreign table
>> >
>> > Regards,
>> >
>> > 2021年4月2日(金) 11:44 Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>:
>> > >
>> > >
>> > >
>> > > On 2021/04/02 9:37, Kohei KaiGai wrote:
>> > > > It is fair enough for me to reverse the order of actual truncation.
>> > > >
>> > > > How about the updated comments below?
>> > > >
>> > > > This is a multi-relation truncate. We first open and grab
>> exclusive
>> > > > lock on all relations involved, checking permissions (local
>> database
>> > > > ACLs even if relations are foreign-tables) and otherwise
>> verifying
>> > > > that the relation is OK for truncation. In CASCADE mode,
>> ...(snip)...
>> > > > Finally all the relations are truncated and reindexed. If any
>> foreign-
>> > > > tables are involved, its callback shall be invoked prior to
>> the truncation
>> > > > of regular tables.
>> > >
>> > > LGTM.
>> > >
>> > >
>> > > >> BTW, the latest patch doesn't seem to be applied cleanly to the
>> master
>> > > >> because of commit 27e1f14563. Could you rebase it?
>> > > >>
>> > > > Onishi-san, go ahead. :-)
>> > >
>> > > +1
>> > >
>> > > Regards,
>> > >
>> > > --
>> > > Fujii Masao
>> > > Advanced Computing Technology Center
>> > > Research and Development Headquarters
>> > > NTT DATA CORPORATION
>>
>