On Wed, Sep 20, 2023 at 10:50 AM Paul Jungwirth
<pj@illuminatedcomputing.com> wrote:
>
> On 9/17/23 20:11, jian he wrote:
> > small issues so far I found, v14.
>
> Thank you again for the review! v15 is attached.
>
hi. some tiny issues.
IN src/backend/utils/adt/ri_triggers.c
else {
appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
pk_only, pkrelname);
}
should change to
else
{
appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
pk_only, pkrelname);
}
----
It would be better, we mention it somewhere:
by default, you can only have a primary key(range_type[...],
range_type WITHOUT OVERLAPS).
preceding without overlaps, all columns (in primary key) data types
only allowed range types.
-------------------------------
The WITHOUT OVERLAPS value must be a range type and is used to
constrain the record's applicability to just that interval (usually a
range of dates or timestamps).
"interval", I think "period" or "range" would be better. I am not sure
we need to mention " must be a range type, not a multi range type".
---------------------------------------------
I just `git apply`, then ran the test, and one test failed. Some minor
changes need to make the test pass.