Can insert action.circulation entries for copies/serials that do not exist.

Bug #749764 reported by Jason Stephenson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned
2.0
Fix Released
Undecided
Unassigned

Bug Description

While testing some load scripts with a trunk database, I saw that I could insert action.circulation entries with target_copy set to a number that did not correspond to an existing asset.copy.id or serial.unit.id. The attached patch for an after trigger on action.circulation will prevent this occurrence.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Signed-off-by: Jason Stephenson <email address hidden>
Signed-off-by: Jason Stephenson <email address hidden>

Revision history for this message
Jason Stephenson (jstephenson) wrote :
Revision history for this message
Mike Rylander (mrylander) wrote :

Alternate implementation proposed here: http://paste.lisp.org/display/121219

Testing, thoughts?

Revision history for this message
Jason Stephenson (jstephenson) wrote :

I had to modify it slightly before it would load. Changing the frist $f$ to $F$.

Also, after triggers should only return NULL according to the documenation. If they do return NEW, any changes made in the trigger are essentially discarded, since after triggers run logically after the insert has occurred.

As far as testing the functionality of the trigger, I haven't gotten that far into a load in the past week. I've had issues with targeting copies at the right asset.call_number that have taken precedence. I have another load going now so I should have some more information for you tomorrow or this weekend.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

For the record, here's the exact patch that I am testing this weekend.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Using the patch from Saturday works great.

I got 505,580 circulations in my last load, and none for copies that don't exist.

I see the log messages about missing asset.copy.ids when my program tries to insert a circulation on a copy that failed to load. (We seem to always have a trashed bib that stops 20 or copies from loading.)

I say the patch above is good to go. :)

Revision history for this message
Mike Rylander (mrylander) wrote :

Final collaborative version committed to trunk, 2.1 and 2.0.

Changed in evergreen:
status: New → Fix Committed
Changed in evergreen:
milestone: none → 2.1.0
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.