Re: SQL:2011 application time

On Fri, Sep 10, 2021 at 6:50 PM Jaime Casanova
<jcasanov(at)systemguards(dot)com(dot)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(at)yugabyte(dot)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