Bug 9523 - Trying to import staged marc records where a matched bib has been deleted caused the import to hang
Summary: Trying to import staged marc records where a matched bib has been deleted cau...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-31 20:05 UTC by Kyle M Hall (khall)
Modified: 2014-05-26 21:04 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 9523 - Trying to import staged marc records where a matched bib has been deleted caused the import to hang (1.21 KB, patch)
2013-01-31 20:09 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 9523 - Trying to import staged marc records where a matched bib has been deleted caused the import to hang (1.24 KB, patch)
2013-02-07 14:56 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 9523 - Trying to import staged marc records where a matched bib has been deleted caused the import to hang (1.24 KB, patch)
2013-02-07 14:58 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 9523: importing staged bib records hangs if a matched bib has been deleted (1.25 KB, patch)
2013-03-12 22:24 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 9523: importing staged bib records hangs if a matched bib has been deleted (1.30 KB, patch)
2013-04-10 15:12 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2013-01-31 20:05:56 UTC
If a MARC file is staged and has found matching records already in Koha, but one of those records that was matched on is deleted before the staged records are imported, the import will stop cold at the missing record and just hang there indefinitely.
Comment 1 Kyle M Hall (khall) 2013-01-31 20:09:07 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-02-06 23:45:53 UTC
Your patch changes the query to

SELECT candidate_match_id
LEFT JOIN biblio ON ( candidate_match_id = biblionumber )
FROM   import_record_matches
WHERE  import_record_id = ?
ORDER BY score DESC, candidate_match_id DESC;

This is invalid SQL ... JOIN clauses come after FROM clauses.  Even were the SQL statement constructed correctly, the left join by itself would make no difference to the query results, as the candidate_match_ids would get returned whether or not the matching bib was still attached.

The patch might *appear* to work, but would have the effect of unconditionally all bibs in the import batch, regardless of matches.

As a note for testing, I suggest using the command-line staging import tools, as they reproduce the problem *and* give useful output in the case of a failure.  For example, if you follow the reproduction steps but use the command-line tools to commit the batch, you get the following error:

$ misc/commit_file.pl --batch-number 5
... importing MARC records -- please wait
Empty String at /usr/lib/perl5/XML/LibXML/SAX/Parser.pm line 42
Comment 3 Kyle M Hall (khall) 2013-02-07 13:35:19 UTC
You are correct. I'll post up a corrected patch when I can.

Kyle

(In reply to comment #2)
> Your patch changes the query to
> 
> SELECT candidate_match_id
> LEFT JOIN biblio ON ( candidate_match_id = biblionumber )
> FROM   import_record_matches
> WHERE  import_record_id = ?
> ORDER BY score DESC, candidate_match_id DESC;
> 
> This is invalid SQL ... JOIN clauses come after FROM clauses.  Even were the
> SQL statement constructed correctly, the left join by itself would make no
> difference to the query results, as the candidate_match_ids would get
> returned whether or not the matching bib was still attached.
> 
> The patch might *appear* to work, but would have the effect of
> unconditionally all bibs in the import batch, regardless of matches.
> 
> As a note for testing, I suggest using the command-line staging import
> tools, as they reproduce the problem *and* give useful output in the case of
> a failure.  For example, if you follow the reproduction steps but use the
> command-line tools to commit the batch, you get the following error:
> 
> $ misc/commit_file.pl --batch-number 5
> ... importing MARC records -- please wait
> Empty String at /usr/lib/perl5/XML/LibXML/SAX/Parser.pm line 42
Comment 4 Kyle M Hall (khall) 2013-02-07 14:56:56 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall (khall) 2013-02-07 14:58:23 UTC Comment hidden (obsolete)
Comment 6 Galen Charlton 2013-03-12 22:24:32 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-04-09 09:09:32 UTC
Kyle,
I cannot reproduce the issue.

My test plan:
1/ Choose a biblio (biblionumber=XXX)
2/ Save it as MARC unicode/UTF-8
3/ Stage my record file with a matching rule (ISBN) and add the incoming record if matching.
Screen is:
  1 records in file
  0 records not staged because of MARC error
  1 records staged
  1 records with at least one match in catalog per matching rule "ISBN"
  1 item records found and staged

4/ Delete items and biblio XXX
5/ Import into the catalog
Screen is:
  Number of records added	1
  Number of records updated	0
  Number of records ignored	0
  Number of items added	1
  Number of items ignored because of duplicate barcode	0

A new record with biblionumber=YYY is created.

What I missed?
Comment 8 Kyle M Hall 2013-04-09 13:47:48 UTC
Odd, I can still replicate this bug on master: http://screencast.com/t/M5qWorUpl

Kyle

(In reply to comment #7)
> Kyle,
> I cannot reproduce the issue.
> 
> My test plan:
> 1/ Choose a biblio (biblionumber=XXX)
> 2/ Save it as MARC unicode/UTF-8
> 3/ Stage my record file with a matching rule (ISBN) and add the incoming
> record if matching.
> Screen is:
>   1 records in file
>   0 records not staged because of MARC error
>   1 records staged
>   1 records with at least one match in catalog per matching rule "ISBN"
>   1 item records found and staged
> 
> 4/ Delete items and biblio XXX
> 5/ Import into the catalog
> Screen is:
>   Number of records added	1
>   Number of records updated	0
>   Number of records ignored	0
>   Number of items added	1
>   Number of items ignored because of duplicate barcode	0
> 
> A new record with biblionumber=YYY is created.
> 
> What I missed?
Comment 9 Jonathan Druart 2013-04-10 15:12:28 UTC
(In reply to comment #8)
> Odd, I can still replicate this bug on master:
> http://screencast.com/t/M5qWorUpl
> 
> Kyle

Ok thanks. I can reproduce if I follow your way.
I did not import twice the same record, but just one time an existing record.
I thought it was the same thing.

Marked as Passed QA.
Comment 10 Jonathan Druart 2013-04-10 15:12:45 UTC
Created attachment 17344 [details] [review]
Bug 9523: importing staged bib records hangs if a matched bib has been deleted

Test Plan:
1) Stage a MARC record file that will have matches with existing records
2) Delete the bib from Koha that was matched on
3) Attempt to import the records into Koha, the import will hang
4) Apply the patch
5) Reload manage-marc-import.pl and attempt to import again, this time it should succeed.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 11 Kyle M Hall 2013-04-10 15:58:06 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > Odd, I can still replicate this bug on master:
> > http://screencast.com/t/M5qWorUpl
> > 
> > Kyle
> 
> Ok thanks. I can reproduce if I follow your way.
> I did not import twice the same record, but just one time an existing record.
> I thought it was the same thing.
> 
> Marked as Passed QA.

I should have worded it more explicitly, my bad!

Kyle
Comment 12 Jared Camins-Esakov 2013-04-22 01:07:18 UTC
This patch has been pushed to master and 3.12.x.
Comment 13 Chris Cormack 2013-04-27 06:08:07 UTC
Pushed to 3.10.x will be in 3.10.6