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 <z...@yugabyte.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 <oni...@heterodb.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 <oni...@heterodb.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.fu...@oss.nttdata.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 >> >
