[Bug 7793] New: redefine the field message_id as PRIMARY KEY of message_queue
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Priority: P5 - low Change sponsored?: --- Bug ID: 7793 Assignee: gmcharlt@gmail.com Summary: redefine the field message_id as PRIMARY KEY of message_queue QA Contact: koha.sekjal@gmail.com Severity: enhancement Classification: Unclassified OS: All Reporter: stephane.delaune@biblibre.com Hardware: All Status: NEW Version: unspecified Component: Database Product: Koha table message_queue has a NOT NULL AUTO_INCREMENT key `message_id`, we should move to PRIMARY KEY -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #1 from stephane.delaune@biblibre.com --- Created attachment 8489 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8489&action=edit proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |stephane.delaune@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #2 from Chris Cormack <chris@bigballofwax.co.nz> --- Hmmm + PRIMARY KEY (`message_id`), KEY `message_id` (`message_id`), Maybe we should remove the second Key? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #3 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- I'm concerned about this patch because there is nothing to prevent two rows having duplicate message_ids. The existing index is KEY and not UNIQUE. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 11743 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11743&action=edit Bug 7793: Followup: Remove the message_queue.message_id KEY -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |jonathan.druart@biblibre.co | |m --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- Last patch removes the useless message_id KEY. I hope it was you want :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |In Discussion -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #6 from Jonathan Druart <jonathan.druart@biblibre.com> --- Chris, Why do you change the status to "In discussion" ? The last patch removes the second key on message_id. It is not what you wanted ? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> --- Yep, it is, but i switched it to in discussion as Jared's concern has not been addressed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gmcharlt@gmail.com --- Comment #8 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Jared Camins-Esakov from comment #3)
I'm concerned about this patch because there is nothing to prevent two rows having duplicate message_ids. The existing index is KEY and not UNIQUE.
Since the column is AUTO_INCREMENT, I question to what extent this would be a problem in practice unless folks have been in the habit of munging message_id. Regardless, a follow-up to the schema update should be straightforward: check to see if there are duplicates, and if so, renumber the affected rows. I don't think that there is any code that cares about any particular values in that column. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #9 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 36145 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36145&action=edit Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |olli-antti.kivilahti@jns.fi Assignee|stephane.delaune@biblibre.c |olli-antti.kivilahti@jns.fi |om | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #8489|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=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11743|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=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff --- Comment #10 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Sorry to steal your bug. Here is everything inside one commit + the rare case of duplicate PKs. No discussion for this one. It is a no-brainer to have a PRIMARY KEY, especially since DBIx-operations fail without it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- Olli, Could you - move the updatedb entry at the end of the updatedatabase.pl file. - make the change more clear, it's not easily readable And I think there is a problem: Before: 2 id=42 After: 1 id=43 and 1 id=44 I would expect: 1 id=42 and 1 id=43. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36145|0 |1 is obsolete| | --- Comment #12 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 36945 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36945&action=edit Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #13 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Fixed Jonathan's good bug. Thanks for it. Won't move the updatedatabase-subroutine "down" because it will just conflict so badly with all future modifications. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 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=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36945|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 36987 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36987&action=edit Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. 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=7793 --- Comment #15 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Thanks for the sign off Joubu! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- I got this error when I tried to update my db: DBD::mysql::db do failed: ALTER TABLE causes auto_increment resequencing, resulting in duplicate entry '9' for key 'PRIMARY' at installer/data/mysql/updatedatabase.pl line 6750. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36987|0 |1 is obsolete| | --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 37027 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37027&action=edit [SIGNED-OFF] Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.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=7793 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 37028 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37028&action=edit Bug 7793 - Simplify db update Since message_id is not linked to anything else in the database, we can ensure the message_id's are unique by simply dropping the column and recreating it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Bernardo Gonzalez Kriegel <bgkriegel@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=7793 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37028|0 |1 is obsolete| | --- Comment #19 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 39143 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39143&action=edit [SIGNED-OFF] Bug 7793: Simplify db update Since message_id is not linked to anything else in the database, we can ensure the message_id's are unique by simply dropping the column and recreating it. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Moved updatedatabase to atomicupdate First patch signed. No errors -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Bernardo Gonzalez Kriegel from comment #19)
Created attachment 39143 [details] [review] [SIGNED-OFF] Bug 7793: Simplify db update
Since message_id is not linked to anything else in the database, we can ensure the message_id's are unique by simply dropping the column and recreating it.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Moved updatedatabase to atomicupdate
atomicupdate only works for sql queries (and 1 per file). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39143|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=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37028|1 |0 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.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=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37027|0 |1 is obsolete| | Attachment #37028|0 |1 is obsolete| | --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 39525 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39525&action=edit Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #22 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 39526 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39526&action=edit Bug 7793 - Simplify db update Since message_id is not linked to anything else in the database, we can ensure the message_id's are unique by simply dropping the column and recreating it. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #23 from Jonathan Druart <jonathan.druart@biblibre.com> --- I amended the 2 patches to reorder the DB entry. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|koha.sekjal@gmail.com |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Guys, can u verify this? I ran the dbix upgrade script just in case on top of this, and found that the generated Koha/Schema/Result/MessageQueue.pm differs from the one shipped with this patch set. I always regenerate them, because the hash on them usually is invalid and makes the upgrade script fail. I mark this Failed-QA until you tell me which is wrong: the kohastructure.sql change or the Koha/Schema/Result/MessageQueue.pm change. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #25 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #24)
Guys, can u verify this? I ran the dbix upgrade script just in case on top of this, and found that the generated Koha/Schema/Result/MessageQueue.pm differs from the one shipped with this patch set.
I always regenerate them, because the hash on them usually is invalid and makes the upgrade script fail.
I mark this Failed-QA until you tell me which is wrong: the kohastructure.sql change or the Koha/Schema/Result/MessageQueue.pm change.
Yes, the file provided in the first patch is wrong. I think the changes to Koha/Schema should not be provided unless it is needed to make the dev/fix work. It always should be provided in a separate patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- So these patches need to be redone? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39525|0 |1 is obsolete| | Attachment #39526|0 |1 is obsolete| | --- Comment #27 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 40172 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40172&action=edit Bug 7793 - redefine the field message_id as PRIMARY KEY of message_queue Making message_queue work nicely with DBIx. DROP the existing KEY and replace with an PRIMARY KEY. If somebody has managed to get duplicate message_ids in his/her message_queue-table, this patch takes care of them as well, but givin all duplicate message_id's a new id. TEST INSTRUCTIONS: in the patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> Amended patch: Remove change to Koha/Schema/Result/MessageQueue.pm, will be done by the RM -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #28 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 40173 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40173&action=edit Bug 7793 - Simplify db update Since message_id is not linked to anything else in the database, we can ensure the message_id's are unique by simply dropping the column and recreating it. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 --- Comment #29 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #24)
Guys, can u verify this? I ran the dbix upgrade script just in case on top of this, and found that the generated Koha/Schema/Result/MessageQueue.pm differs from the one shipped with this patch set.
I always regenerate them, because the hash on them usually is invalid and makes the upgrade script fail.
I mark this Failed-QA until you tell me which is wrong: the kohastructure.sql change or the Koha/Schema/Result/MessageQueue.pm change.
I have amended the first patch to remove the changes to this file. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7793 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Olli et al! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org