[Bug 10606] New: query in GetUpcomingDueIssues
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Bug ID: 10606 Summary: query in GetUpcomingDueIssues Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: gmcharlt@gmail.com QA Contact: testopia@bugs.koha-community.org The patch for bug 9362 (which I did accept into master) introduces a MySQLism: Using HAVING without GROUP BY is a MySQLism, so at some point the query will have to become: SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail FROM issues LEFT JOIN items USING (itemnumber) LEFT OUTER JOIN branches USING (branchcode) WHERE returndate is NULL AND (TO_DAYS( date_due )-TO_DAYS( NOW() )) > 0 AND (TO_DAYS( date_due )-TO_DAYS( NOW() )) < ?; This is a pity, as it would be nice to be able to use "days_until_due" as an alias to make the query more readable, but it simply isn't portable. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Summary|query in |MySQLism in |GetUpcomingDueIssues |GetUpcomingDueIssues -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 20311 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20311&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues Removing this mysqlism as Galen proposed. Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10719 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20311|0 |1 is obsolete| | --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 20333 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20333&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Tomás, It seems that TO_DAYS is a mysqlism. It is not in the postgres documentation http://www.postgresql.org/docs/9.2/static/functions-datetime.html -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20333|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20403 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20403&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, did you seen my comment ? postgres=# select TO_DAYS( NOW() ); ERREUR: la fonction to_days(timestamp with time zone) n'existe pas LIGNE 1 : select TO_DAYS( NOW() ); In english: The do_days function does not exist. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20403|0 |1 is obsolete| | --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 20410 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20410&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- New version passes tests form Bug 10719 too. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20410|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20422 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20422&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20422|0 |1 is obsolete| | --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 20610 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20610&action=edit [PASSED QA] Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and QA script pass. Also tested with unit tests from bug 10719. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #10 from Galen Charlton <gmcharlt@gmail.com> --- The bug 10719 tests do not passed when using MariaDB: # Failed test 'Bug 9362: Only one item due in more than 2 days (3 days in advance)' # at t/db_dependent/Circulation.t line 321. # got: '0' # expected: '1' not ok 35 - Bug 9362: Only one item due in more than 2 days (4 days in advance) # Failed test 'Bug 9362: Only one item due in more than 2 days (4 days in advance)' # at t/db_dependent/Circulation.t line 321. # got: '0' # expected: '1' not ok 36 - Bug 9362: Only one item due in more than 2 days (5 days in advance) # Failed test 'Bug 9362: Only one item due in more than 2 days (5 days in advance)' # at t/db_dependent/Circulation.t line 321. # got: '0' # expected: '1' # Looks like you failed 3 tests of 36. The DATE() function returns a date, but it looks like DATEDIFF(DATE( date_due ),DATE( NOW() )) works but " DATE( date_due )-DATE( NOW() )" does not. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Galen Charlton from comment #10)
The DATE() function returns a date, but it looks like DATEDIFF(DATE( date_due ),DATE( NOW() )) works but " DATE( date_due )-DATE( NOW() )" does not.
I confirm they pass with MySQL and they don't pass with MariaDB. Using MariaDB: MariaDB [koha]> select to_days("2013-08-21 23:59:00")-to_days(NOW()); +-----------------------------------------------+ | to_days("2013-08-21 23:59:00")-to_days(NOW()) | +-----------------------------------------------+ | -6 | +-----------------------------------------------+ MariaDB [koha]> select date("2013-08-21 23:59:00")-date(NOW()); +-----------------------------------------+ | date("2013-08-21 23:59:00")-date(NOW()) | +-----------------------------------------+ | -6 | +-----------------------------------------+ Really odd... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- Not sure what was wrong, but I have just retested to apply this patch and prove Circulation.t, and it returns green. Needs QA. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #13 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 35154 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35154&action=edit Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and QA script pass. Also tested with unit tests from bug 10719. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20610|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #35154|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 35332 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=35332&action=edit [PASSED QA] Bug 10606: Remove MySQLism in GetUpcomingDueIssues To test: [1] Arrange to have at least one loan in your test database due one day from now. [2] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 and note the number of loans reported. [3] Apply the patch. [4] Run misc/cronjobs/advance_notices.pl -c -n -v -m=2 again and verify that the number of loans reported remains the same. Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and QA script pass. Also tested with unit tests from bug 10719. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, did you test with MariaDB? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10606 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I reverted the patch. There's no point breaking functionalities for something that should be handled in the ORM. Even having an IF switch for each engine... -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org