? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
8 Apr 2017

As title says: when creating the new fields_categories table, the query does not contain IF NOT EXISTS

Summary of Changes

Can be merged on review

Note: I did not change anyhting for postgres or azure as I have no idea about the equivalent, if there is one.

@rdeutz @wilsonge @alikon

avatar infograf768 infograf768 - open - 8 Apr 2017
avatar infograf768 infograf768 - change - 8 Apr 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Apr 2017
Category SQL Administration com_admin Installation
avatar alikon alikon - test_item - 8 Apr 2017 - Tested successfully
avatar alikon
alikon - comment - 8 Apr 2017

I have tested this item successfully on e4e0903


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15171.

avatar richard67 richard67 - test_item - 8 Apr 2017 - Tested successfully
avatar richard67
richard67 - comment - 8 Apr 2017

I have tested this item successfully on e4e0903

Tested by review.

For postgres and azure nothing to be done, since their joomla.sql scripts up to now do not use any equivalent in their statements for any of the tables.

In postgres there is an equivalent since version 9.1, see http://stackoverflow.com/questions/1766046/postgresql-create-table-if-not-exists . But as long as Joomla has to support older versions, it cannot be used.
In case if Joomla 4.0 will drop support for older versions of postgres, it would be worth to do another PR just for 4.0 adding that clause to ALL create table statements in joomla.sql and schema update scripts for postgres.

In mssql (azure) there is no equivalent statement, but it can be wrapped into a condition, see http://stackoverflow.com/questions/6520999/create-table-if-not-exists-equivalent-in-sql-server . This does not really look maintainable to me and so better forget about that.

But in my opinion it is a mistake anyway if the install script runs on a database where the tables already exist, and so the "IF EXISTS" clause should be dropped for ALL mysql create table statements, but this would also be another PR for 4.0 maybe.

This PR here in my opinion makes the statements consistent and so is good.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15171.

avatar franz-wohlkoenig franz-wohlkoenig - change - 8 Apr 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 8 Apr 2017

RTC after two successful tests.

avatar infograf768
infograf768 - comment - 8 Apr 2017

@richard67

But in my opinion it is a mistake anyway if the install script runs on a database where the tables already exist

Well, this can be done when using the Reinstall button in joomlaupdate. 😃

avatar richard67
richard67 - comment - 8 Apr 2017

@infograf768 Well, as far as I know that will run the schema updates after having copied the files and folders, but it does not run the joomla.sql, or am I wrong? If I am right, then for the schema updates of course the "IF EXIST" could still make sense in case or previously broken and so only half executed updates. So in case Joomla 4.0 will not support postgres versions prior to 9.1, it would make sense to add that to the schema updates for postgres in the 4.0 branch, and in the joomla.sql it would not do much harm and also be more consistent to mysql if added there, too, in that branch.

avatar rdeutz rdeutz - change - 8 Apr 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-04-08 10:10:52
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 8 Apr 2017
avatar rdeutz rdeutz - merge - 8 Apr 2017
avatar infograf768
infograf768 - comment - 8 Apr 2017

@richard67
I think Reinstall re-uses the update.sql. Not sure though. Indeed we have to find out.

avatar richard67
richard67 - comment - 8 Apr 2017

@infograf768 I am not sure either.

avatar wilsonge
wilsonge - comment - 20 Apr 2017

Current plan for J4 is for Postgres 9.2 and higher :) So PR's welcome for that change in the J4 branch!

avatar richard67
richard67 - comment - 21 Apr 2017

@wilsonge I assume the DB fix still will exist in J4, right? If so, then such PR for J4 as mentioned above has to include changes for the database schema checker aka DB fix so it can deal with "CREATE TABLE IF NOT EXISTS" for Postgresql. Not a big thing but has to be done in case if schema updates will use that kind of statement in future. Another thing is that in my opinion beside the joomla.sql also ALL schema updates (including old ones) should be changed so they all use "CREATE TABLE IF NOT EXISTS" and so are consistent with mysql in all cases. Or would you say we should not touch the old schema updates but only such created in future?

@infograf768 Will you do that PR? If not, I could see if I find time, just let me know so we won't work in parallel.

avatar infograf768
infograf768 - comment - 21 Apr 2017

Please do :)

avatar richard67
richard67 - comment - 21 Apr 2017

@wilsonge I just see that the schema manager already is able to handle that kind of statement for postgresql, see https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/CMS/Schema/ChangeItem/PostgresqlChangeItem.php#L194 . And it is not clear to me what will be done with the schema manager in 4.0. Can you explain me and check my questions to you in my comment above?

avatar richard67
richard67 - comment - 21 Apr 2017
avatar wilsonge
wilsonge - comment - 21 Apr 2017

Thanks for the PR. The schema manager is probably going to remain unchanged. If someone wants to volunteer for it then go ahead. But right this second it's not high up the j4 priority list - i'm trying to focus on getting extensions ready for webservices

Add a Comment

Login with GitHub to post a comment