On 9/24/23 21:52, jian he wrote:
> 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.
Rebased v16 patches attached.
> 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);
> }
Fixed.
> 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 reworked those two paragraphs to incorporate those suggestions and
hopefully clarify the idea bit further. (I'll revise these again once I
support multiple WITHOUT OVERLAPS columns.)
> I just `git apply`, then ran the test, and one test failed. Some minor
> changes need to make the test pass.
I couldn't reproduce this. If you're still seeing a failure please let
me know what you're seeing.
These patches also fix a problem I found with FKs when used with
btree_gist. Privately I'm using the script below [1] to re-run all my
tests with that extension and int+range columns. I'd like to add
something similar to contrib/btree_gist. I'm open to advice how best to
do that if anyone has any!
[1] #!/bin/bash
set -eu
# without_overlaps
cat ../src/test/regress/sql/without_overlaps.sql | \
sed -E 's/int4range/integer/g' | \
sed -E 's/valid_at integer/valid_at int4range/' | \
sed -E 's/int8range/bigint/g' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),\1\]'"'"'/\1/g' | \
cat > ./sql/without_overlaps.sql
cat ../src/test/regress/expected/without_overlaps.out | \
sed -E 's/int4range/integer/g' | \
sed -E 's/valid_at integer/valid_at int4range/' | \
sed -E 's/incompatible types: integer and tsrange/incompatible types:
int4range and tsrange/' | \
sed -E 's/int8range/bigint/g' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),\1\]'"'"'/\1/g' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),-?[[:digit:]]+\)'"'"'/\1/g' | \
sed -E 's/\[(-?[[:digit:]]+),\1\]/\1/g' | \
sed -E 's/\[(-?[[:digit:]]+),-?[[:digit:]]+\)/\1/g' | \
sed -E 'N;P;s/^ +id [^\n]+\n-+(\+.*)$/----\1/p;D' | \
sed -E
's/^----------\+-----------\+-----------\+----------\+---------$/----------+---------+-----------+----------+---------/'
| \
sed -E
's/^----\+-------------------------\+--------\+-------$/----+-------------------------+-----+-------/'
| \
cat > ./expected/without_overlaps.out
# for_portion_of
cat ../src/test/regress/sql/for_portion_of.sql | \
sed -E 's/int4range/integer/g' | \
sed -E 's/valid_at integer/valid_at int4range/' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),\1\]'"'"'/\1/g' | \
cat > ./sql/for_portion_of.sql
cat ../src/test/regress/expected/for_portion_of.out | \
sed -E 's/int4range/integer/g' | \
sed -E 's/valid_at integer/valid_at int4range/' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),\1\]'"'"'/\1/g' | \
sed -E 's/'"'"'\[(-?[[:digit:]]+),-?[[:digit:]]+\)'"'"'/\1/g' | \
sed -E 's/\[(-?[[:digit:]]+),\1\]/\1/g' | \
sed -E 's/\[(-?[[:digit:]]+),-?[[:digit:]]+\)/\1/g' | \
sed -E 'N;P;s/^ +id [^\n]+\n-+(\+.*)$/----\1/p;D' | \
cat > ./expected/for_portion_of.out
Regards,
--
Paul ~{:-)
pj@illuminatedcomputing.com