additional foreign key test coverage
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t39765psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 04:53 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t39765_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t39765_1 && git checkout t39765_1Patchset v1 (message #1) is on t39765_1
During the development of my recent patch "unused/redundant foreign key
code" [0]/messages/by-id/2fb8d28c-a4e1-f206-898b-69cd22a393a1@2ndquadrant.com/, I had developed a few additional test cases to increase the
coverage in ri_triggers.c. They are in the attached patches with
explanations. With these, coverage should be pretty complete, except
hard-to-trigger error cases. Interested reviewers can also follow along
on coverage.postgresql.org.
[0]: /messages/by-id/2fb8d28c-a4e1-f206-898b-69cd22a393a1@2ndquadrant.com/
/messages/by-id/2fb8d28c-a4e1-f206-898b-69cd22a393a1@2ndquadrant.com/
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
t39765_10001-Add-test-case-for-ON-DELETE-NO-ACTION-RESTRICT.patchtext/plain; charset=UTF-8; name=0001-Add-test-case-for-ON-DELETE-NO-ACTION-RESTRICT.patch; x-mac-creator=0; x-mac-type=0Download+12-5
0002-Increase-test-coverage-in-RI_FKey_pk_upd_check_requi.patchtext/plain; charset=UTF-8; name=0002-Increase-test-coverage-in-RI_FKey_pk_upd_check_requi.patch; x-mac-creator=0; x-mac-type=0Download+43-1
0003-Increase-test-coverage-in-RI_FKey_fk_upd_check_requi.patchtext/plain; charset=UTF-8; name=0003-Increase-test-coverage-in-RI_FKey_fk_upd_check_requi.patch; x-mac-creator=0; x-mac-type=0Download+13-2
0004-Increase-test-coverage-in-RI_Initial_Check.patchtext/plain; charset=UTF-8; name=0004-Increase-test-coverage-in-RI_Initial_Check.patch; x-mac-creator=0; x-mac-type=0Download+26-1
On 2018-Dec-04, Peter Eisentraut wrote:
During the development of my recent patch "unused/redundant foreign key
code" [0], I had developed a few additional test cases to increase the
coverage in ri_triggers.c. They are in the attached patches with
explanations. With these, coverage should be pretty complete, except
hard-to-trigger error cases. Interested reviewers can also follow along
on coverage.postgresql.org.
Hmm. One of the things I did for FKs on partitioned tables was remove
all the cases involving only unpartitioned tables, then run just the
foreign_key test and see what the coverage looked like -- in the first
versions, there were large swaths of uncovered code. That guided me to
add a few more tests to increase coverage in later versions. This is
all to say that I think it would be useful to include the case of
partitioned tables in the tests you add, where relevant.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 04/12/2018 14:23, Alvaro Herrera wrote:
On 2018-Dec-04, Peter Eisentraut wrote:
During the development of my recent patch "unused/redundant foreign key
code" [0], I had developed a few additional test cases to increase the
coverage in ri_triggers.c. They are in the attached patches with
explanations. With these, coverage should be pretty complete, except
hard-to-trigger error cases. Interested reviewers can also follow along
on coverage.postgresql.org.Hmm. One of the things I did for FKs on partitioned tables was remove
all the cases involving only unpartitioned tables, then run just the
foreign_key test and see what the coverage looked like -- in the first
versions, there were large swaths of uncovered code. That guided me to
add a few more tests to increase coverage in later versions. This is
all to say that I think it would be useful to include the case of
partitioned tables in the tests you add, where relevant.
I'm not sure I understand where partitioned tables come in here. In
ri_triggers.c, it's all dealing with single base tables. Certainly
other code elsewhere needs to know about partitions.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Dec-07, Peter Eisentraut wrote:
On 04/12/2018 14:23, Alvaro Herrera wrote:
Hmm. One of the things I did for FKs on partitioned tables was remove
all the cases involving only unpartitioned tables, then run just the
foreign_key test and see what the coverage looked like -- in the first
versions, there were large swaths of uncovered code. That guided me to
add a few more tests to increase coverage in later versions. This is
all to say that I think it would be useful to include the case of
partitioned tables in the tests you add, where relevant.I'm not sure I understand where partitioned tables come in here. In
ri_triggers.c, it's all dealing with single base tables. Certainly
other code elsewhere needs to know about partitions.
Well, certain features (say, referential actions) needed some specific
code changes when FKs appeared in partitioned tables. I didn't notice
those at first, and only noticed when I added tests involving
partitioned tables. I'm just saying if you add for the simple case, you
might miss bugs when whatever feature you're covering is used with
partitioned tables.
I see one example right in your 0001 patch, where your code calls
ri_restrict. That one needs to add ONLY or not depending on
partitionedness. I think you don't need to do anything here because
the !is_no_action case is already covered for partitioned tables.
Another potential example in 0002 (and 0003): in the covered function we
do this,
if (ri_NullCheck(RelationGetDescr(pk_rel), old_row, riinfo, true) != RI_KEYS_NONE_NULL)
are we using the correct tuple descriptor? Keep in mind that partition
can have different column layout than parent. (In this case it's not a
problem, because the pk_rel is not yet allowed to be partitioned, so if
you commit this soon, it will be my problem not yours).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hi!
I tested this patch and it applied cleanly and all tests passed. I haven't looked if the changes to tests are reasonable or extensive to cover all aspects of what they want to cover.
Mitar
On 09/01/2019 09:20, Mi Tar wrote:
I tested this patch and it applied cleanly and all tests passed. I haven't looked if the changes to tests are reasonable or extensive to cover all aspects of what they want to cover.
I have committed this with additional tests for partitioned tables, as
requested by Álvaro.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services