Bug 8665 - DOM indexing fails to index some bib records
Summary: DOM indexing fails to index some bib records
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low critical (vote)
Target Milestone: Oldversions
Assignee: Jared Camins-Esakov
QA Contact: Paul Poulain
URL:
Whiteboard:
Keywords:
Depends on: 7818
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-22 08:16 UTC by Colin Campbell
Modified: 2018-12-16 19:34 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8665: DOM indexing fails to index some bib records (9.41 KB, patch)
2012-10-04 12:53 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8665: DOM indexing fails to index some bib records (9.67 KB, patch)
2012-10-08 14:40 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 8665: DOM indexing fails to index some bib records (9.83 KB, patch)
2012-10-17 07:09 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 8665 follow-up: add missing line to XSLT (2.41 KB, patch)
2012-10-25 18:04 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8665: DOM indexing fails to index some bib records (9.80 KB, patch)
2012-10-26 19:08 UTC, Elliott Davis
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8665 follow-up: add missing line to XSLT (2.48 KB, patch)
2012-10-26 19:14 UTC, Elliott Davis
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2012-08-22 08:16:02 UTC
Switched on dom indexing and some records became unfindable. e.g. of a 5 volume set only one volume could be retrieved. The problem is reproduceable thus:

Copy a bib record. Change what you like in the new record but keep the 001 the same. Rebuild zebra on bibs. Only one of the records will appear when searching.

Problem appears to be etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl  which contains the following

  <xslo:attribute name="z:id">
        <xslo:value-of select="$controlField001"/>
      </xslo:attribute>

which appears to set the record id to the contents of 001, with the result that only one record for each distinct 001 is indexed. Replacing the controlField001 with another value resolves the problem.
Comment 1 Paul A 2012-10-03 16:09:54 UTC
Although not specifically defined, we assume that 001 (Control Number) should be unique in Koha. Duplicates (for eventual correction) can be found by running a report:

SELECT GROUP_CONCAT(biblionumber SEPARATOR ', ') AS biblionumbers, 
EXTRACTVALUE(marcxml,'//controlfield[@tag="001"]') AS id FROM biblioitems  
GROUP BY id  HAVING count(id) > 1
Comment 2 D Ruth Bavousett 2012-10-04 06:32:11 UTC
It's a nice assumption, Paul, and in a perfect world, it would be so, but the assumption is a bit rash in the real world. In the GRS model of indexing, only 999$c is *required* to be unique, and (IMO) it should be the same way in DOM.

Until this is fixed, I've got libraries that can't use DOM indexing, period.
Comment 3 D Ruth Bavousett 2012-10-04 06:56:16 UTC
Colin, any chance you could describe what to change controlField001 *to* that can workaround this problem?  Pleeeeeease?  :)
Comment 4 Jared Camins-Esakov 2012-10-04 12:53:29 UTC Comment hidden (obsolete)
Comment 5 Colin Campbell 2012-10-05 07:44:46 UTC
(In reply to comment #3)
> Colin, any chance you could describe what to change controlField001 *to*
> that can workaround this problem?  Pleeeeeease?  :)

My kludge was to substitute this:

      <xslo:attribute name="z:id">
        <xslo:value-of select="biblioitemnumber"/>
      </xslo:attribute>

Not a thought out solution but assuming anything in the MARC record will be unique (even if theoretically so) is groundless.
Comment 6 Jared Camins-Esakov 2012-10-08 14:40:16 UTC Comment hidden (obsolete)
Comment 7 Chris Cormack 2012-10-17 07:09:22 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2012-10-25 16:34:17 UTC
QA comment:

 * are the following lines related to this fix ?:
-  <xslo:template mode="index_heading_conditional" match="marc:datafield[@tag='450']">
+  <xslo:template mode="index_heading_conditional">
...
+  <xslo:template match="text()" mode="index_heading_conditional"/>
...
+      <xslo:apply-templates mode="index_heading_conditional"/>


 * OK for
-    <target_index>materials-specified :w</target_index>
+    <target_index>materials-specified:w</target_index>

 * not sure I understand what does:
-    <xslo:variable name="controlField001" select="normalize-space(marc:controlfield[@tag='001'])"/>
+    <xslo:variable name="idfield" select="normalize-space(marc:controlfield[@tag='001'])"/>
     <z:record type="update">
       <xslo:attribute name="z:id">
-        <xslo:value-of select="$controlField001"/>
+        <xslo:value-of select="$idfield"/>

(in authority-zebra-indexdefs) : it does "nothing" except changing the variable to be consistent with biblio-zebra-indexdefs, right ?
(for biblio, the change is in the variable name AND in what it is filled with:
-    <xslo:variable name="controlField001" select="normalize-space(marc:controlfield[@tag='001'])"/>
+    <xslo:variable name="idfield" select="normalize-space(marc:datafield[@tag='999']/marc:subfield[@code='c'])"/>
)
Comment 9 Jared Camins-Esakov 2012-10-25 18:04:11 UTC Comment hidden (obsolete)
Comment 10 Jared Camins-Esakov 2012-10-25 18:08:55 UTC
(In reply to comment #8)
> QA comment:
> 
>  * are the following lines related to this fix ?:
> -  <xslo:template mode="index_heading_conditional"
> match="marc:datafield[@tag='450']">
> +  <xslo:template mode="index_heading_conditional">'

A line was missing in the transformer. I fixed that in the follow-up.

> +  <xslo:template match="text()" mode="index_heading_conditional"/>
> ...
> +      <xslo:apply-templates mode="index_heading_conditional"/>

These others are no-ops, caused by the fact that I didn't retransform the bib stylesheets after adding index_heading_conditional.

>  * OK for
> -    <target_index>materials-specified :w</target_index>
> +    <target_index>materials-specified:w</target_index>
> 
>  * not sure I understand what does:
> -    <xslo:variable name="controlField001"
> select="normalize-space(marc:controlfield[@tag='001'])"/>
> +    <xslo:variable name="idfield"
> select="normalize-space(marc:controlfield[@tag='001'])"/>
>      <z:record type="update">
>        <xslo:attribute name="z:id">
> -        <xslo:value-of select="$controlField001"/>
> +        <xslo:value-of select="$idfield"/>
> 
> (in authority-zebra-indexdefs) : it does "nothing" except changing the
> variable to be consistent with biblio-zebra-indexdefs, right ?

Exactly. That's the point of the fix.

> (for biblio, the change is in the variable name AND in what it is filled
> with:
> -    <xslo:variable name="controlField001"
> select="normalize-space(marc:controlfield[@tag='001'])"/>
> +    <xslo:variable name="idfield"
> select="normalize-space(marc:datafield[@tag='999']/marc:
> subfield[@code='c'])"/>
> )

Remember that the *-zebra-indexdefs.xsl files are all automatically generated. Any time the transformer is changed in a way that could cause side effects, all the *-zebra-indexdefs.xsl files must be regenerated.
Comment 11 Elliott Davis 2012-10-26 19:08:56 UTC
Created attachment 13085 [details] [review]
[SIGNED-OFF] Bug 8665: DOM indexing fails to index some bib records

Use a user-specified field for z:id.

This patch also fixes an excess space before the index in the MARC21
biblio index definitions, which someone fixed in the generated file
but not in the source file it should have been fixed in.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>
Comment 12 Elliott Davis 2012-10-26 19:14:26 UTC
Created attachment 13086 [details] [review]
[SIGNED-OFF] Bug 8665 follow-up: add missing line to XSLT

The DOM transformer was missing a line from a previous development,
resulting in the MARC21 authorities DOM indexing stylesheet being
regenerated with a missing line. This patch readds the missing line
to the transformer, and provides the corrected authority-zebra-indexdefs.

Signed-off-by: Elliott Davis <elliott@bywatersolutions.com>
Comment 13 Paul Poulain 2012-10-29 18:12:14 UTC
QA comment:
 * small patch, xsl only, but a core one
 * fixes identifier for zebra by switching from 001 to where biblionumber is stored, because biblionumber is guaranteed to be unique ++
 * fixes other minor mistakes -- to fix in the same patch, but not worth failing QA, it's a needed fix

passed QA
Comment 14 Paul Poulain 2012-10-29 18:37:00 UTC
Patch pushed to master

(checked that it work in UNIMARC)
Comment 15 Chris Cormack 2012-10-30 22:07:11 UTC
Pushed to 3.8.x will be in 3.8.7
Comment 16 Martin Renvoize 2013-02-08 11:44:49 UTC
So i've been running through some customisations and patches that require rebuilding these dom config files...

I'm using the script make_zebra_dom_cfg_from_record_abs as directed on http://wiki.koha-community.org/wiki/Switching_to_dom_indexing to create my new marc21 biblio-koha-indexdefs.xml config file:

~/kohaclone/misc/maintenance/make_zebra_dom_cfg_from_record_abs --input ~/kohaclone/etc/zebradb/marc_defs/marc21/biblios/record.abs --output ~/kohaclone/etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xml

However each time I run the script the ID line is removed from the resulting file and so this bug reasserts itself..

Am I missing something?
Comment 17 Jared Camins-Esakov 2013-02-08 13:05:04 UTC
(In reply to comment #16)
> So i've been running through some customisations and patches that require
> rebuilding these dom config files...
> 
> I'm using the script make_zebra_dom_cfg_from_record_abs as directed on
> http://wiki.koha-community.org/wiki/Switching_to_dom_indexing to create my
> new marc21 biblio-koha-indexdefs.xml config file:
> 
> ~/kohaclone/misc/maintenance/make_zebra_dom_cfg_from_record_abs --input
> ~/kohaclone/etc/zebradb/marc_defs/marc21/biblios/record.abs --output
> ~/kohaclone/etc/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xml
> 
> However each time I run the script the ID line is removed from the resulting
> file and so this bug reasserts itself..
> 
> Am I missing something?

Yes. You should not use the make_zebra_dom_cfg_from_record_abs script. I will update the wiki page to clarify that the script should not be used.