Bug 10740 - date created and modified indexes in ccl.properties
Summary: date created and modified indexes in ccl.properties
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 8252
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-16 10:28 UTC by Fridolin Somers
Modified: 2020-11-30 21:45 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:


Attachments
Bug 10740 - date created and modified indexes in ccl.properties (2.11 KB, patch)
2013-08-16 10:36 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10740 - date created and modified indexes in ccl.properties (2.11 KB, patch)
2013-10-22 07:30 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10740 - date created and modified indexes in ccl.properties and QP (6.40 KB, patch)
2013-10-25 13:04 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-08-16 10:28:46 UTC
date created and modified indexes are defined in bib1.att as : 
 1011 date-entered-on-file
 1012 Date/time-last-modified

This should be used in ccl.properties.
Comment 1 Fridolin Somers 2013-08-16 10:36:04 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2013-08-22 14:27:52 UTC
Is it really a good idea to put a slash in the index name? What we usually do is replace all punctuation with hyphens.
Comment 3 Fridolin Somers 2013-08-22 15:02:07 UTC
(In reply to Jared Camins-Esakov from comment #2)
> Is it really a good idea to put a slash in the index name? What we usually
> do is replace all punctuation with hyphens.
The index is actually like this.
But I agree to modify. I'd say : Datetime-last-modified
What do you say ?
Comment 4 Jared Camins-Esakov 2013-08-22 15:04:26 UTC
(In reply to Fridolyn SOMERS from comment #3)
> (In reply to Jared Camins-Esakov from comment #2)
> > Is it really a good idea to put a slash in the index name? What we usually
> > do is replace all punctuation with hyphens.
> The index is actually like this.
> But I agree to modify. I'd say : Datetime-last-modified
> What do you say ?

I think that's the best idea.
Comment 5 Mathieu Saby 2013-08-22 15:41:11 UTC
Hello Fridolyn
I wanted to reword this index in bug 8252 (needs signoff), but changed my mind after Paul's comment.

Mathieu
Comment 6 Fridolin Somers 2013-08-23 08:20:57 UTC
(In reply to mathieu saby from comment #5)
> Hello Fridolyn
> I wanted to reword this index in bug 8252 (needs signoff), but changed my
> mind after Paul's comment.
> 
> Mathieu

Oh, yes.
@Jared : What do you think about bug 8252 ?
I'd say it is dangerous to use a "/" in indexes.
Comment 7 Mathieu Saby 2013-08-23 08:23:03 UTC
I thought it was dangerous too ;-)
But bug 8252 is already quite complex to test, so I won't replace the / in it...

Mathieu
Comment 8 I'm just a bot 2013-09-29 05:39:09 UTC
Patch applied cleanly, go forth and signoff
Comment 9 Mathieu Saby 2013-09-29 08:39:39 UTC
Bug 8252 has been signed off by Bernardo Gonzales Kriegel and Frédéric Demians.
I'm waiting for its QA. When it is pushed we could do other improvements/corrections, like the one proposed by Fridolyn.

M. Saby
Comment 10 David Noe 2013-10-21 23:36:11 UTC
This patch does not apply.  The change appear to have been made already.  Both these line are in ccl.properties:

att 1011    date-entered-on-file
att 1012    Date/time-last-modified
Comment 11 Fridolin Somers 2013-10-22 07:30:59 UTC Comment hidden (obsolete)
Comment 12 Mathieu Saby 2013-10-22 07:42:32 UTC
(In reply to Fridolyn SOMERS from comment #11)
> Created attachment 22262 [details] [review] [review]
> Bug 10740 - date created and modified indexes in ccl.properties
> 
> Patch rebased (Conflict with Bug 8252)

Hi Fridolyn
As 8252 is pushed to master, can we say that it "block" 10740 ?

Mathieu
Comment 13 Mathieu Saby 2013-10-22 08:14:47 UTC
I will test your patch in a DOM UNIMARC this evening.

Mathieu
Comment 14 Mathieu Saby 2013-10-22 08:19:31 UTC
Out of the scope of your bug : 
Each time we add an new index in the list of indexes in Search.pm, my blood is freezing :(
I know it's needed now, but would not it be more flexible (if a library wants to add custom index) with a list stored out of Perl code, in some YAML file for ex?

Mathieu
Comment 15 Jared Camins-Esakov 2013-10-22 12:17:40 UTC
(In reply to mathieu saby from comment #14)
> Out of the scope of your bug : 
> Each time we add an new index in the list of indexes in Search.pm, my blood
> is freezing :(
> I know it's needed now, but would not it be more flexible (if a library
> wants to add custom index) with a list stored out of Perl code, in some YAML
> file for ex?

This is how QueryParser works. :)
Comment 16 Mathieu Saby 2013-10-22 12:25:35 UTC
(In reply to Jared Camins-Esakov from comment #15)
> (In reply to mathieu saby from comment #14)
> > Out of the scope of your bug : 
> > Each time we add an new index in the list of indexes in Search.pm, my blood
> > is freezing :(
> > I know it's needed now, but would not it be more flexible (if a library
> > wants to add custom index) with a list stored out of Perl code, in some YAML
> > file for ex?
> 
> This is how QueryParser works. :)

Great! So when we decide that pre-QP search could be deprecated, this piece of code could be delete I imagine?
But, is there a similar operation to do in QP conf to declare the new index, or is it automatic?

Mathieu
Comment 17 Jared Camins-Esakov 2013-10-22 12:35:52 UTC
(In reply to mathieu saby from comment #16)
> Great! So when we decide that pre-QP search could be deprecated, this piece
> of code could be delete I imagine?

Yup. We talked about that at the hackfest on Sunday.

> But, is there a similar operation to do in QP conf to declare the new index,
> or is it automatic?

There is a similar configuration required: you have to add your new index to etc/searchengine/queryparser.yaml.
Comment 18 Mathieu Saby 2013-10-22 12:50:37 UTC
(In reply to Jared Camins-Esakov from comment #17)
> (In reply to mathieu saby from comment #16)
> > Great! So when we decide that pre-QP search could be deprecated, this piece
> > of code could be delete I imagine?
> 
> Yup. We talked about that at the hackfest on Sunday.
> 
> > But, is there a similar operation to do in QP conf to declare the new index,
> > or is it automatic?
> 
> There is a similar configuration required: you have to add your new index to
> etc/searchengine/queryparser.yaml.

I did not know that... Is there a page on the wiki or elsewere explaining that, or other points about QP?
I found http://wiki.koha-community.org/wiki/C_%26_P_Search_Rewrite_RFC but it is a RFC,  not a page on "What you absolutely need to know about QP now that it is used by Koha".
For example, with QP, is it possible to search for an exact expression using Google syntax (double quotes), like "bla bla" ?

Mathieu
Comment 19 Mathieu Saby 2013-10-24 20:42:47 UTC
Jared, I suppose I need to add a followup to this bug I wrote some time ago and which does not take into account the QP : http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9828 and added some fields to Search.pm.
I don't know at all how to do this, and in which cases (when Search.pm is altered? when bib1.att is altered? when ccl.properties is altered?).

Mathieu
Comment 20 Fridolin Somers 2013-10-25 12:50:00 UTC
(In reply to mathieu saby from comment #19)
> I don't know at all how to do this
I will provide a QP follow-up for this bug 10740, to add "date-time-last-modified".
I noticed that in QP all indexes are lowercase.
So in the end I wonder why not use everywhere : "date-time-last-modified" and not "Date/time-last-modified".
Comment 21 Fridolin Somers 2013-10-25 13:04:08 UTC
Created attachment 22391 [details] [review]
Bug 10740 - date created and modified indexes in ccl.properties and QP

Revised patch.
Date/time-last-modified converted in date-time-last-modified for QP.
I think using / in an index would have provoke bugs is future.
Comment 22 Mathieu Saby 2013-11-04 08:35:52 UTC
(In reply to Fridolyn SOMERS from comment #20)
> (In reply to mathieu saby from comment #19)
> > I don't know at all how to do this
> I will provide a QP follow-up for this bug 10740, to add
> "date-time-last-modified".
> I noticed that in QP all indexes are lowercase.
> So in the end I wonder why not use everywhere : "date-time-last-modified"
> and not "Date/time-last-modified".

In ccl.properties, there are a lot of aliases differing from the main index only in terms of capitalisation (Notes notes etc).
I always wondered if it was necessary or not...

Mathieu
Comment 23 Marcel de Rooy 2014-04-14 08:09:44 UTC
This patch does not deal with DOM indexing.
In the biblio-koha-indexdefs.xml/biblio-zebra-indexdefs.xsl files that govern dom indexing (compare them with record.abs for grs1), I see Date/time-last-modified. I think it would be consistent to use the same fieldname here too.

Failed QA
Comment 24 Marc Véron 2016-10-04 16:16:59 UTC
Still valid?
Comment 25 Fridolin Somers 2020-03-27 13:13:20 UTC
(In reply to Marc Véron from comment #24)
> Still valid?

In ES the search field is without slash, see Bug 22524

But with Zebra the index is "1012    Date/time-last-modified" and Date-time-last-modified is an alias.

Since it works I close.