Feedback-ID: ie0a040ee:Fastmail
Message-ID: <85dd3ad0-fa79-46e8-a08b-c426c76825f3@eisentraut.org>
Date: Thu, 21 Mar 2024 15:57:20 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: SQL:2011 application time
Content-Language: en-US
To: Paul Jungwirth <pj@illuminatedcomputing.com>,
 jian he <jian.universality@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com>
 <88518c81-dcdc-4c5b-9200-146b74a520ab@eisentraut.org>
 <113f5e25-4455-4bdb-98ec-56275705cc4e@illuminatedcomputing.com>
 <7bd1c8f9-a91a-41a3-990e-0f796ba692ec@eisentraut.org>
 <d1a793bd-d1f1-46d7-90b6-8b89b6feb1cc@illuminatedcomputing.com>
 <6c3edb2c-bc45-4f19-babd-8f4edf39b384@illuminatedcomputing.com>
 <41b57948-f4d3-455f-8880-e0ce7e61ffc1@illuminatedcomputing.com>
 <0d977c63-f4a6-43ba-aad0-35c06c55df11@eisentraut.org>
 <CACJufxFFeUHoBomd2856amDBccNVQZzA=Jrk1_FDdvSZ1+X7og@mail.gmail.com>
 <CACJufxEACgLY7YUvzo8j=iOB-z6-1QHAvMr8cUHRP7b_F_vo3A@mail.gmail.com>
 <CA+renyXHFFyq1n2+4cwcFoLXqzUhb6u6WCL6wjk8ZjP9WxTExw@mail.gmail.com>
 <f22f65c8-865d-4753-94cc-3b954c29ad5c@eisentraut.org>
 <feee527a-6c47-42b9-a947-2422adb45a1c@illuminatedcomputing.com>
 <CACJufxF8HyERm7SKBJ+duxAP9sTgVhaW6b-oWADANGgn-o4wPA@mail.gmail.com>
 <b4bc1ef8-4215-4cf6-933c-5ef2a2827efe@illuminatedcomputing.com>
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: <b4bc1ef8-4215-4cf6-933c-5ef2a2827efe@illuminatedcomputing.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/85dd3ad0-fa79-46e8-a08b-c426c76825f3%40eisentraut.org>
Precedence: bulk

On 20.03.24 17:21, Paul Jungwirth wrote:
> On 3/20/24 03:55, jian he wrote:
>> hi.
>> minor cosmetic issues, other than that, looks good.
>>
>> *pk_period = (indexStruct->indisexclusion);
>> to
>> *pk_period = indexStruct->indisexclusion;
>>
>> ... >
>> if (with_period && !fkconstraint->fk_with_period)
>> ereport(ERROR,
>> (errcode(ERRCODE_INVALID_FOREIGN_KEY),
>> errmsg("foreign key uses PERIOD on the referenced table but not the
>> referencing table")));
> 
> Both included in the new patches here.
> 
> Rebased to a0390f6ca6.

Two more questions:

1. In ri_triggers.c ri_KeysEqual, you swap the order of arguments to 
ri_AttributesEqual():

-           if (!ri_AttributesEqual(riinfo->ff_eq_oprs[i], 
RIAttType(rel, attnums[i]),
-                                   oldvalue, newvalue))
+           if (!ri_AttributesEqual(eq_opr, RIAttType(rel, attnums[i]),
+                                   newvalue, oldvalue))

But the declared arguments of ri_AttributesEqual() are oldvalue and 
newvalue, so passing them backwards is really confusing.  And the change 
does matter in the tests.

Can we organize this better?

2. There are some tests that error with

ERROR:  only b-tree indexes are supported for non-PERIOD foreign keys

But this is an elog() error, so should not normally be visible.  I 
suspect some other error should really show here, and the order of 
checks is a bit wrong or something?