Paul A Jungwirth <pj@illuminatedcomputing.com> writes:
> Thanks for taking a look! I changed places we say `-- not unique` to
> now say `-- overlaps`. v5 is attached to my reply to jian he.
I pushed this patch, but not without significant revisions:
* I didn't think the case had been made for adding this field to
typcache. There's only one user, it's in a very minority-interest
code path, and it'd add both space in TypeCacheEntry and a
noticeable number of cycles to TYPECACHE_DOMAIN_BASE_INFO requests.
(Admittedly the extra-cycles complaint is weak, since we expect
TypeCacheEntries to live a long time once populated.) The other
problem was that the patch was buggy anyway: there is more than
one place in typcache.c that'd have to change. I just moved the
get_typtype(typcache->domainBaseType) call to runtime instead.
We can revisit that if more potential uses appear.
* I thought the number of new regression test cases was quite
excessive compared to the value. On my machine, the runtime for
without_overlaps.sql went from ~420ms to ~500ms, and it's already
the slowest in its parallel group by a good margin. I cut it back to
just testing the base domain-over-range and range-over-domain cases,
which added only ~5ms. (To be clear, I think it was worth making
those tests just to verify there are no other holes. But carrying
them forward forevermore is another matter.)
* I happened to notice that the loop just above the changes in
transformIndexConstraint was buggy: it had "break" where it
should have had "continue". That bug can be demonstrated with
CREATE TABLE temporal_rng4 (
id int4range,
bogus int,
valid_at int4
);
ALTER TABLE temporal_rng4 DROP COLUMN bogus;
ALTER TABLE temporal_rng4 ADD
CONSTRAINT temporal_rng4_pk UNIQUE (id, valid_at WITHOUT OVERLAPS);
which gave different errors with and without the DROP COLUMN
step.
regards, tom lane
