User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Category | ⇒ | SQL Administration com_admin Installation |
I have tested this item
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.
Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
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.
@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.
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:
?
|
@richard67
I think Reinstall re-uses the update.sql. Not sure though. Indeed we have to find out.
@infograf768 I am not sure either.
Current plan for J4 is for Postgres 9.2 and higher :) So PR's welcome for that change in the J4 branch!
@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.
Please do :)
@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?
@infograf768 @wilsonge Done, see #15446.
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
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.