Bug 24994 - TableExists should be used instead of IF NOT EXISTS in updatedatabase
Summary: TableExists should be used instead of IF NOT EXISTS in updatedatabase
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-26 16:01 UTC by Martin Renvoize
Modified: 2020-11-30 21:46 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.06


Attachments
Bug 24994: Correct DB Updates (13.63 KB, patch)
2020-03-26 16:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24994: Correct DB Updates (13.70 KB, patch)
2020-05-13 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24994: Correct DB Updates (13.34 KB, patch)
2020-05-14 18:18 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2020-03-26 16:01:10 UTC
Ooops.. I introduced a series of these during my tenure as RM..
Comment 1 Martin Renvoize 2020-03-26 16:02:06 UTC
Created attachment 101887 [details] [review]
Bug 24994: Correct DB Updates
Comment 2 Martin Renvoize 2020-03-26 16:02:29 UTC
I corrected the cases I introduced in this patch..
Comment 3 Jonathan Druart 2020-03-27 09:36:18 UTC
Tomas, why did you suggest that?
I cannot remember why we should not use "DROP IF EXISTS"

If we go that way, we need:
 * a guideline (and explain why)
 * a check to the QA script

See also https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7902#c3
Comment 4 Martin Renvoize 2020-03-27 09:42:23 UTC
Even if we choose to stick to IF EXISTS it may be nice to add a QA script check for both ADD TABLE (IF NOT EXISTS) and DROP TABE (IF EXISTS).. I know I've missed those on occasions myself.. I don't remember there being a QA check for it as yet (probably because we had the TableExists function as an option).
Comment 5 Tomás Cohen Arazi 2020-03-27 11:06:52 UTC
(In reply to Jonathan Druart from comment #3)
> Tomas, why did you suggest that?
> I cannot remember why we should not use "DROP IF EXISTS"
> 
> If we go that way, we need:
>  * a guideline (and explain why)
>  * a check to the QA script
> 
> See also https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7902#c3

I pointed a case of table creation. Maybe my comment was too generic. I have no opinion on the drop case.
Comment 6 Katrin Fischer 2020-03-27 13:54:46 UTC
DROP IF EXISTS could be bad for features we backported that include tables. The data would be lost.
Comment 7 Jonathan Druart 2020-05-13 13:47:27 UTC
Created attachment 104846 [details] [review]
Bug 24994: Correct DB Updates

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Tomás Cohen Arazi 2020-05-14 18:18:51 UTC
Created attachment 104895 [details] [review]
Bug 24994: Correct DB Updates

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Joy Nelson 2020-05-19 22:25:26 UTC
backported to 19.11.06