On Wed, Sep 20, 2023 at 10:50 AM Paul Jungwirth <p...@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.