Thread: pgsql: Rewrite some RI code to avoid using SPI
Rewrite some RI code to avoid using SPI Modify the subroutines called by RI trigger functions that want to check if a given referenced value exists in the referenced relation to simply scan the foreign key constraint's unique index, instead of using SPI to execute SELECT 1 FROM referenced_relation WHERE ref_key = $1 This saves a lot of work, especially when inserting into or updating a referencing relation. This rewrite allows to fix a PK row visibility bug caused by a partition descriptor hack which requires ActiveSnapshot to be set to come up with the correct set of partitions for the RI query running under REPEATABLE READ isolation. We now set that snapshot indepedently of the snapshot to be used by the PK index scan, so the two no longer interfere. The buggy output in src/test/isolation/expected/fk-snapshot.out of the relevant test case added by commit 00cb86e75d6d has been corrected. (The bug still exists in branch 14, however, but this fix is too invasive to backpatch.) Author: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Reviewed-by: Li Japin <japinli@hotmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CA+HiwqGkfJfYdeq5vHPh6eqPKjSbfpDDY+j-kXYFePQedtSLeg@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/99392cdd78b788295e52b9f4942fa11992fd5ba9 Modified Files -------------- src/backend/executor/execPartition.c | 174 ++++++++- src/backend/executor/nodeLockRows.c | 161 ++++---- src/backend/utils/adt/ri_triggers.c | 564 ++++++++++++++++------------ src/include/executor/execPartition.h | 6 + src/include/executor/executor.h | 8 + src/test/isolation/expected/fk-snapshot.out | 4 +- src/test/isolation/specs/fk-snapshot.spec | 5 +- 7 files changed, 605 insertions(+), 317 deletions(-)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Rewrite some RI code to avoid using SPI
Just for the record, I didn't approve of that patch, and I don't
think cramming it in a few hours before feature freeze is a good
way to proceed.
(1) We've added enough instability to the tree this week already.
(2) I'm still quite unhappy about the idea that this particular
type of FK check will be done using fundamentally different methods
than every other type of FK check. I think that is inevitably
going to lead to semantic inconsistencies.
regards, tom lane
On 2022-Apr-07, Tom Lane wrote: > Just for the record, I didn't approve of that patch, and I don't > think cramming it in a few hours before feature freeze is a good > way to proceed. > (1) We've added enough instability to the tree this week already. Several animals failed already in ways that look obviously connected to this commit, so I can't disagree: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=grison&dt=2022-04-07%2020%3A12%3A22 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lapwing&dt=2022-04-07%2020%3A40%3A16 https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=florican&dt=2022-04-07%2021%3A18%3A46 > (2) I'm still quite unhappy about the idea that this particular > type of FK check will be done using fundamentally different methods > than every other type of FK check. I think that is inevitably > going to lead to semantic inconsistencies. I must have misread, then, that you were not as adamantly opposed to the idea as in your first email to the thread. I'll revert, keeping the new test. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2022-Apr-07, Tom Lane wrote:
>> (1) We've added enough instability to the tree this week already.
> Several animals failed already in ways that look obviously connected to
> this commit, so I can't disagree:
Yeah, I saw that after complaining. Looks like all and only the
32-bit animals were unhappy. Something we'll want to run to ground
when there's less time pressure.
regards, tom lane