Currently in Koha/OAI/Server/ListBase.pm two different criteria are used for querying dates and displaying dates. When displaying the "Last Modified" date, this is used: SELECT MAX(timestamp) FROM ( SELECT timestamp FROM biblio_metadata WHERE biblionumber = ? UNION SELECT timestamp FROM deleteditems WHERE biblionumber = ? UNION SELECT timestamp FROM items WHERE biblionumber = ? ) bi But when querying, only biblio_metadata.timestamp is used. This should be made consistent. We could use a query like this (using a join, but removing the need to do a separate query for each record, so it might actually be faster than what we have now): select bm.biblionumber, greatest(bm.timestamp, i.timestamp) as t from biblio_metadata bm inner join items i on bm.biblionumber=i.biblionumber having t >= '2020-07-14' and t <= '2020-07-15' limit 10; Furthermore, we should only use items.timestamp and deleteditems.timestamp when the user has the "include_items" option set in their OAI-PMH config. It does not make sense to query item timestamps if you are only sending data about the records, and not the items. Therefore, if "include_items" is disabled, we can do a simpler query: select biblionumber, timestamp from biblio_metadata where timestamp >= '2020-07-14' and timestamp <= '2020-07-15' limit 10; See full discussion on IRC: http://irc.koha-community.org/koha/2021-01-18#i_2324076 Definition of "datestamp" for reference: https://www.openarchives.org/OAI/openarchivesprotocol.html#Datestamp
This could be an opportunity to fix the timezone handling too since I think we're using local time rather than UTC time...
Good point, David. I did not notice that, since our library only operates in one time zone. Please also note that I forgot to join on the deleteditems table in the "select ... greatest( ... from ..." example query above. It should probably be something like this (untested): SELECT bm.biblionumber, GREATEST(bm.timestamp, i.timestamp, di.timestamp) AS t FROM biblio_metadata bm INNER JOIN items i ON bm.biblionumber=i.biblionumber INNER JOIN deleteditems di ON bm.biblionumber=di.biblionumber HAVING t >= '2020-07-14' AND t <= '2020-07-15' LIMIT 10; One more thing I would like to mention: There was some Koha update in July 2020 (See discussion http://irc.koha-community.org/koha/2021-01-18#i_2324154 ) which modified thousands of items to have the exact same timestamp. This is problematic for users with the "include_items" option set in OAI-PMH. This is not something we can fix for people who already applied that update, but it's tangentially relevant to this bug. We should be careful when mass-updating items or records, because when all of them have the same date, not matter how small your OAI-PMH resolution, you will have to harvest all of them at once at some point.
(In reply to David Cook from comment #1) > This could be an opportunity to fix the timezone handling too since I think > we're using local time rather than UTC time... That shouldn't be the case. We set the timezone to UTF in Repository.pm.
(In reply to Rudolf Byker from comment #0) > But when querying, only biblio_metadata.timestamp is used. That shouldn't be the case either. In the record loop in ListBase.pm the timestamp is retrieved for each record in turn. You're right about that we could, or probably should, ignore item timestamps when include_items is not enabled.
(In reply to Ere Maijala from comment #3) > (In reply to David Cook from comment #1) > > This could be an opportunity to fix the timezone handling too since I think > > we're using local time rather than UTC time... > > That shouldn't be the case. We set the timezone to UTF in Repository.pm. Ahh, that's because you're a champion, Ere! I'm thinking too far into the past before your contribution ;). ere++
Haha, thanks. So, is there really an issue that I'm failing to see here?
> Haha, thanks. So, is there really an issue that I'm failing to see here? Indeed. I re-read my original post, and can't find a way to say it any clearer than I did. Please also read the linked IRC chat. :)(In reply to Ere Maijala from comment #6)
(In reply to Rudolf Byker from comment #7) > Indeed. I re-read my original post, and can't find a way to say it any > clearer than I did. Please also read the linked IRC chat. :)(In reply to Ere > Maijala from comment #6) Oh, there's a bug indeed. Sorry, not sure how I missed it then and now. It all works fine when you have `include_items: 1` in the conf file specified by preference OAI-PMH:ConfFile, but the item timestamps should be ignored when that's not the case. I'll post a patch. The thing with joins is that they are indeed quite terrible. If you only consider a range of one day, it all looks ok, but the way OAI-PMH harvesting usually works is that the initial harvest is done without date limits to get all the records, and then incremental harvesting is done with just a 'from' date (since the beginning of the previous harvesting). That means the date limits may include millions of records, and joins on such large sets are better avoided. I'll contact you off-bug for further discussion on the topic. You can check the history for these files to see how things have evolved in an attempt to improve functionality and performance. Also, I've just posted bug 27584 with a further refinement trying to address performance issues when there are millions of biblios.
(In reply to Ere Maijala from comment #3) > (In reply to David Cook from comment #1) > > This could be an opportunity to fix the timezone handling too since I think > > we're using local time rather than UTC time... > > That shouldn't be the case. We set the timezone to UTF in Repository.pm. Looking at this one again and I think our approach is still a bit flawed. "MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval. (This does not occur for other types such as DATETIME.) By default, the current time zone for each connection is the server's time. The time zone can be set on a per-connection basis. As long as the time zone setting remains constant, you get back the same value you store. If you store a TIMESTAMP value, and then change the time zone and retrieve the value, the retrieved value is different from the value you stored." (https://dev.mysql.com/doc/refman/5.6/en/datetime.html) That works fine if your library system only uses 1 timezone, but it would be problematic if you used multiple timezones. That said, if you were to use multiple timezones, chances are they would be close together, and time discrepancies in the OAI-PMH might pass relatively unnoticed. That's where storing the data as UTC in the first place is really crucial...
(In reply to David Cook from comment #9) > That works fine if your library system only uses 1 timezone, but it would be > problematic if you used multiple timezones. That said, if you were to use > multiple timezones, chances are they would be close together, and time > discrepancies in the OAI-PMH might pass relatively unnoticed. > > That's where storing the data as UTC in the first place is really crucial... Sorry, I'm not following. Since timestamp is stored in UTC and we set the database connection for OAI-PMH to UTC, I can't really an issue. If another client using another TZ retrieves the record, the timestamp shows a different value, sure, but that's just as it should be, no?
(In reply to Ere Maijala from comment #10) > Sorry, I'm not following. Since timestamp is stored in UTC and we set the > database connection for OAI-PMH to UTC, I can't really an issue. If another > client using another TZ retrieves the record, the timestamp shows a > different value, sure, but that's just as it should be, no? In hindsight, I have no idea what I was talking about. I think that my brain may have been stuck on PostgreSQL's "timestamp without time zone" data type which doesn't store UTC internally and uses local time instead, which has caused me problems in the past with legacy systems. I withdraw my earlier comment as today it doesn't make sense to me.
Bug 27548 contains changes that should fix the misreported timestamp.
(In reply to David Cook from comment #11) > In hindsight, I have no idea what I was talking about. I think that my brain > may have been stuck on PostgreSQL's "timestamp without time zone" data type > which doesn't store UTC internally and uses local time instead, which has > caused me problems in the past with legacy systems. That would, indeed, be problematic. Luckily, that's not an issue with MySQL/MariaDB. :)
Ere, I don't see how Bug 27463 is related to this one? The patch is one line, and does not affect the OAI code. Did you mistype the bug number? :)
Sorry, now I copy-pasted the wrong code. Let met try again ... I don't see how Bug 27548 is related to this one? The patch is one line, and does not affect the OAI code. Did you mistype the bug number? :)
Oops, mistyped, it's 27584 (linked in "Depends on" field).
Revisiting this today. There are a lot of confusing tangents in the discussion above, but I think the gist of it is, from comment #8, comment #12 and comment #16: > ... It > all works fine when you have `include_items: 1` in the conf file specified > by preference OAI-PMH:ConfFile, but the item timestamps should be ignored > when that's not the case. I'll post a patch. and > Bug 27584 contains changes that should fix the misreported timestamp. (typo corrected) So, what still needs to be done here?
I believe the actual issues are now, finally, fixed in bug 29135. I'd appreciate verification, though!
I tested as follows: - Checked out Koha master branch. - Ran Koha using docker composer from the koha-testing-docker repo. - Enabled OAI-PMH (no YAML file, so default should be to exclude items) - Visited cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml and took note of last modified date of KOHA-OAI-TEST:7 . It is shown in UTC. Good. - Added an item for record 7. - Checked KOHA-OAI-TEST:7 again: No change. Good. - Edited record 7. - Checked KOHA-OAI-TEST:7 again: Time updated. Still UTC. Good. - Edited an item for record 7. - Checked KOHA-OAI-TEST:7 again: No change. Good. - Deleted two items for record 7. - Checked KOHA-OAI-TEST:7 again: No change. Good. - Deleted the last item for record 7. - Checked KOHA-OAI-TEST:7 again: No change. Good. I think we can close this issue now :)
(In reply to Rudolf Byker from comment #19) > I tested as follows: > > - Checked out Koha master branch. > - Ran Koha using docker composer from the koha-testing-docker repo. > - Enabled OAI-PMH (no YAML file, so default should be to exclude items) > - Visited cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml and > took note of last modified date of KOHA-OAI-TEST:7 . It is shown in UTC. > Good. > - Added an item for record 7. > - Checked KOHA-OAI-TEST:7 again: No change. Good. > - Edited record 7. > - Checked KOHA-OAI-TEST:7 again: Time updated. Still UTC. Good. > - Edited an item for record 7. > - Checked KOHA-OAI-TEST:7 again: No change. Good. > - Deleted two items for record 7. > - Checked KOHA-OAI-TEST:7 again: No change. Good. > - Deleted the last item for record 7. > - Checked KOHA-OAI-TEST:7 again: No change. Good. > > I think we can close this issue now :) \o/ Y'all rock :-D
Looks like this one can be closed