On Fri, Sep 10, 2021 at 6:50 PM Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:
>
> patch 01: does apply but gives a compile warning (which is fixed by patch
> 02)
> [snip]
> patch 03: produces these compile errors.
I did a rebase and fixed this new error, as well as the warnings.
On Mon, Sep 6, 2021 at 1:40 PM Zhihong Yu <zyu@yugabyte.com> wrote:
>
> + * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
>
> It seems the year (2018) should be updated to 2021.
Done.
> For RemovePeriodById(), it seems table_open() can be called after SearchSysCache1(). This way, if
HeapTupleIsValid(tup)is true, table_open() can be skipped.
This seems like it permits a race condition when two connections both
try to drop the period, right?
> For tablecmds.c, AT_PASS_ADD_PERIOD is defined as 5 with AT_PASS_ADD_CONSTR etc moved upward. Do we need to consider
compatibility?
I don't think there is a compatibility problem---can you explain?
These symbols aren't used outside tablecmds.c and the values aren't
saved anywhere AFAIK.
> There are a few TODO's such as:
> Are they going to be addressed in the next round of patches ?
These are mostly questions I'm hoping a reviewer can help me answer,
but I'll take a pass through them and see which I can remove myself.
Several are for adding support for partitioned tables, where I would
definitely appreciate help.
> There seems to be some overlap between ATExecAddPeriod() and AddRelationNewPeriod().
> Is it possible to reduce code duplication ?
I've refactored those functions to remove some duplication, but I
think I prefer the old version---let me know if you have suggestions
to avoid the duplication in a nicer way.
Oh also I realized fp_triggers.c wasn't included in the last few patch
files---I'm sorry about that!
Latest files attached. Thanks for the reviews!
Paul