valid_z3950_attr_type check constraint can cause errors during pg_restore

Bug #1506534 reported by Galen Charlton on 2015-10-15
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned

Bug Description

The valid_z3950_attr_type check constraint on the config.z3950_index_field_map table can cause czifm rows to not be loaded during a pg_restore. In particular, if czifm gets loaded *before* config.z3950_attr is populated, any row in czifm that has a non-null z3950_attr_type would violate the check constraint, causing czifm to not be populated by pg_restore.

Changing the check constraint to a trigger should avoid the problem.

Evergreen master

Ben Shum (bshum) wrote :

Marking this issue confirmed. We've seen this occur during our pg_restores where it would randomly fail on this.

Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
Jason Boyer (jboyer) wrote :

I'm interested in getting some discussion going on this. (There's no time to worry about it for the upgrade I'm doing in 3 days which is the only time I remember it, so I'm just dropping the constraint temporarily) Does a trigger need to do anything more than raise an exception to properly emulate our use of a check here? (i.e. is there an attendant code change required to make this transition complete?) I don't mind throwing some code/testing at it once the end of year pressure is off, but I want to make sure I fully understand what needs doing rather than going off blindly in this direction or that.

Galen Charlton (gmc) wrote :

It would be sufficient for the trigger to throw an exception.

Jason Boyer (jboyer) wrote :

I found some spare tuits on the shelf while a database rebuilds and have a branch to test here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1506534_z3950_attr_check

It removes the check, replaces the existing function with a trigger function, and applies a constraint trigger to the table so it can be deferred. It worked as expected for me and a simple test method is described below. I didn't know if this warranted a pgtap test since it's so basic (Could be used as an example of the simplest possible constraint trigger impl). I can work on a simple test, but I wanted to get this out there in case anyone wanted to throw some manual testing at it.

Testing:
Pre-patch:
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr) VALUES ('Title2', 5, 5); -- Sucess, z3950_attr_type is null, z3950_attr defined
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr_type) VALUES ('Title3', 5, 'title'); -- Success, z3950_attr_type exists in config.z3950_attr seed data
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr_type) VALUES ('Title4', 5, 'sadface'); -- Failure! z3950_attr_type does not exist in config.z3950_attr seed data

Post-patch:
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr) VALUES ('Title2', 5, 5); -- Sucess, z3950_attr_type is null, z3950_attr defined, trigger isn't called in this case.
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr_type) VALUES ('Title3', 5, 'title'); -- Success, z3950_attr_type exists in config.z3950_attr seed data
INSERT INTO config.z3950_index_field_map (label, metabib_field, z3950_attr_type) VALUES ('Title4', 5, 'sadface'); -- Failure! z3950_attr_type does not exist in config.z3950_attr seed data. If in a transaction, does not fail until COMMIT;

Jason Boyer (jboyer) wrote :

Do to my inability to get git (someday I'll remember that commit -a is not like add . ; commit) there was no upgrade script to test initially. Now there's an upgrade script, 002.schema... is updated, and there's a pgtap test that has been force-pushed to that branch.

Jason Boyer (jboyer) on 2015-12-30
Changed in evergreen:
milestone: none → 2.next
Jason Boyer (jboyer) on 2016-01-15
tags: added: pullrequest
Ben Shum (bshum) wrote :

Pushed to master, but added a small followup commit to fix a typo in the 002.schema file

"z3950attr_name_is_valid" should be "z3950_attr_name_is_valid" with an underscore between.

Changed in evergreen:
milestone: 2.next → 2.10-beta
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers