Created attachment 68914 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

The following issue should be resolved with this patch applied:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19485

But in addition to title-series, most of the other broken search links should also work.

Think I have missed a few places (facets for example) where field names are hard coded and needs to be changes. So no point in trying out patch in it's current state until these things are fixed.

Created attachment 69140 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Created attachment 69153 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Now most (hopefully all) issues (primarily with facets) should be sorted out.

Think I have been looking at the Zebra-mapping configuration in the wrong place (this old config, not biblio-koha-indexdefs.xml). I will go through the mapping changes one more time using this as reference instead.

*** Bug 19485 has been marked as a duplicate of this bug. ***

Created attachment 69306 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Wrote a script to extract mappings from biblio-koha-indexdefs.xml and convert to mappings.yaml, left the fields with existing mapping-targets as they are (even though discovered several discrepancies with the Zebra mappings) and mostly just added the missing fields. Will now do more thorough local testing so will probably adjust this patch some more before ready to be reviewed.

Created attachment 69314 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Is this ready for signoff?

Sorry, should have been more clear about that. Yes, this is ready for signoff. There is still some stuff I left out of this patch, like:

- New mappings for other marc-flavours than marc21 extracted from biblio-koha-indexdefs.xml
- Correction for some existing mappings (I think some offsets are off by one for example)
- New mappings and posssible corrections for authorities

But if this get reviewed and committed I can open new bug-reports for those once I have time to further polish the extraction-script.

(In reply to David Gustafsson from comment #13)
> But if this get reviewed and committed I can open new bug-reports for those
> once I have time to further polish the extraction-script.

Can you share the script?

Yes, I was thinking to do so, though it is only a quick and dirty script in Ruby (should perhaps be re-written in perl if to be included in Koha-repo), and though would clean it up just a little bit more, remove some hard-coded stuff, add some simple command line options etc. But will post a link to git-repo with said script once removed the worst ugliness.

https://github.com/ub-digit/koha-indexdefs-to-mappings-yaml

Patch fails to apply; conflict on file mappings.yaml.

Created attachment 69479 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Ok! Rebased against master. There was a commit in between setting the "sort" for serveral fields, those changes should now be merged in.

Reviewing the commit-diff once again, I think I made an error in the merge. Will upload again.

Created attachment 69480 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Think git auto-merged some stuff incorrectly, but this should now be sorted out.

Created attachment 69507 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates, and
sorting of search results in UI.

Field name 'Date/time-last-modified' does not work with elastic since "/" will create subfields, koha will also not tokanize this correctly, changed this (back) to 'Date-time-last-modified'.

(In reply to David Gustafsson from comment #22)
> Think git auto-merged some stuff incorrectly, but this should now be sorted
> out.

So, ready to be signed off?

Yes should be ready for sign off, have discovered no more issues since the most recent patch.

(In reply to David Gustafsson from comment #26)
> Yes should be ready for sign off, have discovered no more issues since the
> most recent patch.

The good way to tell other people that your patch is ready to be tested is to change the status of the bug report from "Patch doesn't apply" to "Needs Signoff"

Aha, I looked for a status drop down but manage to miss it since it wasn't a drop down. :) Will fix.

And, now I saw there was a dropdown, but below the comment area.

Just a caveat, just discovered that some field names hard coded in xsl-templates have names that differer in case from mappings.yaml. (control-number for example). Zebra has case-insensitive field names, but this will cause problems with elasticsearch. So this patch, while correcting some links, might also break some that worked before. I'm planning on writing a fix to make Elasticseach field names non case sensitive, but until that issue is resolved this patch might causee issues.

Created attachment 70494 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Case insensitive field names in searches is now fixed. A few changes I made could use some extra motivation/explanation:

# sub _convert_index_strings_freeform() (line 685):
changed from:
sub _convert_index_strings_freeform {
    my ( $self, $search ) = @_;
    while ( my ( $zeb, $es ) = each %index_field_convert ) {
        $search =~ s/\b$zeb(?:,[\w\-]*)?:/$es:/g;
    }
    return $search;
}
to:
my $field_name_pattern = '[\w\-]+';
my $multi_field_pattern = "(?:\\.$field_name_pattern)*";
...
sub _convert_index_strings_freeform {
    my ( $self, $search ) = @_;
    $search =~ s/($field_name_pattern)(?:,[\w-]*)?($multi_field_pattern):/\L$1\E$2:/og;
    $search =~ s/($field_name_pattern)($multi_field_pattern):/(exists $index_field_convert{$1} ? $index_field_convert{$1} : $1)."$2:"/oge;
    return $search;
}
(Excluding comments)
With new regexps for lower-casing field names etc there is a lot of duplication for things like field names, possible multiple/subfields etc, so created some variables for components that are often reused to prevent some pitfalls in possible future refactoring. I'm not sure it really matters performance wise, but also replaced the alias replacement code with a more efficient code since the %index_field_convert hash is now much larger than before.

Also, the regexp in sub _truncate_terms() has been changed to also use $field_name_pattern and $multi_field_pattern.

Created attachment 70495 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Found some more places where hard coded fields needed to be changed to lower-case.

Here is a github-diff that might be easier on the eyes: https://github.com/ub-digit/Koha/commit/8be850d2cdc93e5d121afa19c5eed1e1cc683f1c

Created attachment 70558 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Found one (hopefully) last place that needed case-adjustment. New diff: https://github.com/ub-digit/Koha/commit/a6fda51b59c65c2459d460fd374377b56b5f10cc

Created attachment 71090 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Found one more hard coded field value in authorities search that needed lower-casing.

Created attachment 71112 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Fixed hard coded field name in Koha/SearchEngine/ElasticSearch/Search.pm ('au' to 'authority-number')

*('an' to 'authority-number')

Created attachment 71966 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Removed incorrect alias/field name conversions (ctype => "content-type"), and some aliases with identical source and target values.

Created attachment 72673 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 75115 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Rebased against master.

Sorry David, but the patch does not apply on master. Small conflict on mappings.yaml. Could you rebase it ?
Also, could you provide a test plan ?

Will fix this. Test plan is a litte bit tricky since this corrects a lot of small stuff. But will try to backtrack to list most of what is fixed.

Created attachment 81297 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

To test:
1) Go to a biblio detail page
2) Click a link under "Publisher" or "Provider"
3) This link should yield no search results
4) Search for "ISBN:x" where x is a valid isbn.
5) The search should produce no results.
6) Apply patch
7) Go to a biblio detail page and click "Publisher" or "Provider" again
8) The link should no produce results
9) Search for "ISBN:x" again
10) The search should now produce results

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 81299 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

To test:
1) Go to a biblio detail page
2) Click a link under "Publisher" or "Provider"
3) This link should yield no search results
4) Search for "ISBN:x" where x is a valid isbn.
5) The search should yield no results.
6) Apply patch
7) Go to a biblio detail page and click "Publisher" or "Provider" again
8) The link should now yield search results
9) Search for "ISBN:x" again
10) The search should now yield search results

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 81303 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

To test:
1) Go to a biblio detail page
2) Click a link under "Publisher" or "Provider"
3) This link should yield no search results
4) Search for "ISBN:x" where x is a valid isbn.
5) The search should yield no results.
6) Apply patch
7) Reset mappings
8) Perform a full reindexing
9) Go to a biblio detail page and click "Publisher" or "Provider" again
10) The link should now yield search results
11) Search for "ISBN:x" again
12) The search should now yield search results

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Created attachment 81304 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

To test:
1) Go to a biblio detail page
2) Click a link under "Publisher" or "Provider"
3) This link should yield no search results
4) Search for "ISBN:x" where x is a valid isbn.
5) The search should yield no results.
6) Apply patch
7) Reset mappings
8) Perform a full reindexing
9) Go to a biblio detail page and click "Publisher" or "Provider" again
10) The link should now yield search results
11) Search for "ISBN:x" again
12) The search should now yield search results
13) Also try searching for "isbn:x" and "Isbn:x"
14) This should also work and yield search results

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Patch applies, resetting status.

Created attachment 82794 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Rebase against master.

This all looks sane to me.. but I'm going to raise it to our resident ES expert and ask for his SO as a PQA.. calling Ere

Created attachment 83704 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>

Awesome, thanks David. This was actually on my list of things to work on when time permits, and it had been bugging me.

*** Bug 21793 has been marked as a duplicate of this bug. ***

Created attachment 83710 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Fixed a single whitespace error that was introduced.

Passing QA

(In reply to Martin Renvoize from comment #62)
> Fixed a single whitespace error that was introduced.
> 
> Passing QA

Tests are failing with these patches:

prove -v t/db_dependent/SearchEnginge/ElasticSearch

David, will you be able to address the test issues?

Unfortunately I have have been a little bit too busy lately to have a look at this, but hope will have the time to do so some time next week.

Created attachment 85029 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Rebased and updated, I think in spirit of original set - set to Signed off, would like another QA run and Ere's okay

Extra note: I found that indexes like 'se' and 'an' were not searchable - no results were returned from faceting or searchign like "se:pengiun" changing to title-series (or even 'ser' in testing) seemed to work

(In reply to Nick Clemens from comment #69)
> Extra note: I found that indexes like 'se' and 'an' were not searchable - no
> results were returned from faceting or searchign like "se:pengiun" changing
> to title-series (or even 'ser' in testing) seemed to work

an is used in the XSLT templates, sounds like an issue?

(In reply to Nick Clemens from comment #67)
> Created attachment 85030 [details] [review] [review]
> Bug 19575: Rebase and fix tests
> 
> This patch lower cases the sort by fields to normalize checking them and
> adjusts
> some existing tests to meet the new expectations.
> 
> The regex for splitting terms has been moved into a subroutine so that
> adjustment was made

Thanks for adjusting the tests. The regexp was put outside of the subroutine on purpose though, to avoid recompilation on each call.

Created attachment 85062 [details] [review]
Bug 19575: Rebase and fix tests

This patch lower cases the sort by fields to normalize checking them and adjusts
some existing tests to meet the new expectations.

The regex for splitting terms has been moved into a subroutine so that adjustment was made

(In reply to David Gustafsson from comment #71)
> (In reply to Nick Clemens from comment #67)
> > Created attachment 85030 [details] [review] [review] [review]
> > Bug 19575: Rebase and fix tests
> > 
> > This patch lower cases the sort by fields to normalize checking them and
> > adjusts
> > some existing tests to meet the new expectations.
> > 
> > The regex for splitting terms has been moved into a subroutine so that
> > adjustment was made
> 
> Thanks for adjusting the tests. The regexp was put outside of the subroutine
> on purpose though, to avoid recompilation on each call.

Moved back outside, thanks

(In reply to Nick Clemens from comment #69)
> Extra note: I found that indexes like 'se' and 'an' were not searchable - no
> results were returned from faceting or searchign like "se:pengiun" changing
> to title-series (or even 'ser' in testing) seemed to work

About this issue and the "an" alias, this does not work with this patch applied? The alias seems to exist in the %index_field_convert map, and is expanded as far as I can tell. But could look into this if not working. I got a little bit confused since bug 20388 says "'se' is fixed in bug 19575". :)

Oops, bug 22325 I mean, perhaps you linked to the wrong bug number in the previous comment?

Could the cause be that the value of an: is unquoted in clause where UseAuthoritiesForTracings is set in the excerpt below (koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2intranetResults.xsl line 308):

<xsl:choose>
<xsl:when test="marc:datafield[@tag=100] or marc:datafield[@tag=110] or marc:datafield[@tag=111] or marc:datafield[@tag=700] or marc:datafield[@tag=710] or marc:datafield[@tag=711]">
    <p class="author">av
    <xsl:for-each select="marc:datafield[(@tag=100 or @tag=700) and @ind1!='z']">
    <a>
    <xsl:choose>
        <xsl:when test="marc:subfield[@code=9] and $UseAuthoritiesForTracings='1'">
            <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=an:<xsl:value-of select="str:encode-uri(marc:subfield[@code=9], true())"/></xsl:attribute>
        </xsl:when>
        <xsl:otherwise>
        <xsl:attribute name="href">/cgi-bin/koha/catalogue/search.pl?q=au:"<xsl:value-of select="str:encode-uri(marc:subfield[@code='a'], true())"/>"</xsl:attribute>
        </xsl:otherwise>
    </xsl:choose>

(And in many other places). Should perhaps not matter though if "an" never contains whitespace.

Created attachment 85105 [details] [review]
Bug 19575: Fix an search field

(In reply to David Gustafsson from comment #75)
> (In reply to Nick Clemens from comment #69)
> > Extra note: I found that indexes like 'se' and 'an' were not searchable - no
> > results were returned from faceting or searchign like "se:pengiun" changing
> > to title-series (or even 'ser' in testing) seemed to work
> 
> About this issue and the "an" alias, this does not work with this patch
> applied? The alias seems to exist in the %index_field_convert map, and is
> expanded as far as I can tell. But could look into this if not working. I
> got a little bit confused since bug 20388 says "'se' is fixed in bug 19575".
> :)

Aha, it turns out an should work, but an was mapped to authority-number and koha-auth-number was mapped to authority-number - but koha-auth-number was the field name :-)  Patch attached

Created attachment 85124 [details] [review]
Bug 19575: Keep authority-number as alias and fix query

Keep authority-number as alias and change field name
from alias to real field in hard coded Elasticsearch query

Ok! Since it was some time ago I don't really remember how to parse the Zebra field definitions anymore, but the authority-number entry looked a little bit strange and I might have made some a mistake regarding the format. I re-added authority-number as an alias, probably not needed but doesn't hurt. Also found an instance of authority-number being used in a hard coded Elasticsearch query.

Created attachment 85125 [details] [review]
Bug 19575: Use canonical field names and resolve aliased fields

Adjust elastic search mappings to more closely match Zebra equivalents
resolving serveral issues with coded Zebra searches in templates and
sorting of search results in UI. Also make field names in search strings
case insensitive to accept case variations in template links and user input.

Sponsored-by: Gothenburg University Library

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 85126 [details] [review]
Bug 19575: Rebase and fix tests

This patch lower cases the sort by fields to normalize checking them and adjusts
some existing tests to meet the new expectations.

The regex for splitting terms has been moved into a subroutine so that adjustment was made

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 85127 [details] [review]
Bug 19575: Fix an search field

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 85128 [details] [review]
Bug 19575: Keep authority-number as alias and fix query

Keep authority-number as alias and change field name
from alias to real field in hard coded Elasticsearch query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

*** Bug 22229 has been marked as a duplicate of this bug. ***

Created attachment 85178 [details] [review]
Bug 19575: (RM follow-up) Fix mapping typo

Found with QA tools, simple fix

I looked at this today, but the update statement is a little confusing.

Should we update a library's mappings in updates? or simply require that they reset mappings when upgrading?

I am okay with updating, but it also seems some of the lines are unnecessary with the generic statement of lower casing at the top?

Can you review and format the update using skeleton.perl? Or let me know if I missed something?

Yes, the lowercase update statement is pretty pointless since the mixed case field names will be restored on reset. Field names are lower-cased in _foreach_mapping, so the case of field names in mappings.yaml (or database) does not really matter. If lower casing all field names the corresponding field names in mappings.yaml should also be lower cased. The field name renaming is probably still a good idea, so that a reset is not required.

Created attachment 85247 [details] [review]
Bug 19575: Don't lower case fields in db and use update script instead of raw SQL

(In reply to David Gustafsson from comment #91)
> Created attachment 85247 [details] [review] [review]
> Bug 19575: Don't lower case fields in db and use update script instead of
> raw SQL

Hi David,

Thanks for that update, I still see some lines I wonder about, the ones like:
dbh->do( "UPDATE `search_field` SET `name` = 'isbn', `label` = 'isbn' WHERE `name` = 'isbn'" );

I don't see any change here - isbn is being renamed to isbn, there are a fair amount of lines like this in the update.

The other ones like se being renamed title-series do make sense

It seems we could trim this down a fair amount

(In reply to Nick Clemens from comment #92)
Talked with Ere, he verified these make sense

Awesome work all!

Pushed to master for 19.05

Pushed to 18.11.x for 18.11.04

 I had a brief discussion regarding this one in IRC and we reached the consensus that we are both earlier enough in the cycle and the lifetime of the Elasticsearch use that it was a sensible one to backport even though it has further reaching effects than we would normally permit in a stable branch release.

Why was su-geo facet field removed by this?

That might have been a mistake. The mappings where generated from this script from Zebra mapping definitions: https://github.com/ub-digit/koha-indexdefs-to-mappings-yaml/blob/master/mappings.rb but su-geo is in the source mappings as far as I can see and should not have been removed. I also manually rebased mappings.yaml a couple of times after upstream changes, don't know if made some mistake there.

Since I seems to have commented out the alias ('su-geo' => 'subject') in the patch, it seams like I expected the field to exist, so I probably did not remove it intentionally and it should be re-added.

I had a look at the script again and now remembered it does not migrate zebra facets declared in biblio-koha-indexdefs.xml, that might be the reason it got dropped. The other facet fields are present in mappings.yaml though.

I filed bug 22495 for the su-geo fix.

Looks to me like the original confusion might stem from the fact that queryparser.yaml defines su-geo as an alias for subject. I think that might be a mistake in queryparser.yaml. It was added in bug 9828 that improved UNIMARC support. I suspect there wasn't proper su-geo field in UNIMARC mappings, and the alias would make sense, but only for UNIMARC.

not backporting this to 18.05x

I see this line: 
$dbh->do( "UPDATE `search_field` SET `name` = 'date-of-publication', `label` = 'date-of-publication' WHERE `name` = 'pubdate'" );

The name is changed in database, but in YAML file, I always have pubdate, this has not been modified.

(In reply to claire.hernandez@biblibre.com from comment #102)
> I see this line: 
> $dbh->do( "UPDATE `search_field` SET `name` = 'date-of-publication', `label`
> = 'date-of-publication' WHERE `name` = 'pubdate'" );
> 
> The name is changed in database, but in YAML file, I always have pubdate,
> this has not been modified.

Hi Claire, 

can you please open a new bug report and link to this one? It's already in a released version, so we need a fix on a new bug.

Claire, the publication date mapping will be fixed in bug 22524.