User tests: Successful: Unsuccessful:
Pull Request for Issue #26351 (comment) .
This Pull Request (PR) does the same change as PR #26351 in postgresql/joomla.sql also for the update sql script "4.0.0-2018-07-29.sql" so there is no database error shown about missing index after a new installation of J4 on a PostgreSQL database.
Furthermore, the index name of the same index for MySQL is changed so it also fits to our naming scheme and is consistent with PostgreSQL (except that the latter has the table name as beginning of the index name), i.e. if someone searches for "idx_term_language" in sql files, he/she will find it in both MySQL and PostgreSQL joomla.sql and 4.0.0-2018-07-29.sql files.
Note on updating old 4.0 sql update scripts
Since we are not in Beta phase yet and so don't have to support updates from 4.0-Alpha-x to 4.0-Alpha-y or between nightly builds, we can change the existing 4.0 update scripts (but of course not pre-4.0 scripts). Later when in beta this will not be allowed anymore because we have to support updating from Beta-x to Beta-y (with y > x of course), so now is a good time to fix them.
For PostgreSQL
For MySQL
term
and language
of the #__finder_terms_common
table.On PostgreSQL
No database errors shown.
On MySQL
No database errors shown.
Index name is consistent with PostgreSQL (except that the latter has the table name as beginning of the index name).
On PostgreSQL
Database error shown, see #26351 (comment) .
On MySQL
No database errors shown.
Index name is not consistent with PostgreSQL and our naming scheme "idx_column1_column2" for an index over 2 columns.
None.
Status | New | ⇒ | Pending |
Category | ⇒ | SQL Administration com_admin Postgresql Installation |
I will repeat my objection to changing sql update files. They should be immutable no matter what stage of release we are in
@brianteeman That's your opinion, and you have been told several times that you are not right.
If they are not immutable at this stage then there was no point at all in creating multiple update sql and everything should have just gone in one file
Well, that could have been done, but there are other good reasons to have separate files for particular changes, last but not least execution times.
The thing is that the database schema checker can't handle when you e.g. change data type of a column in 1 sql update script, and then later change it again in a later update script. Sometimes such things happen. When the 1st sql is already in the wild, e.g. with beta, you have of course to make a 2nd script with a later date in the file name, which again modifies that column to another data type or varchar length or default value or whatever changes.
But this will not be enough, because when the column is fixed by the 2nd script and now you go again to the database checker, it shows an error that the column does not fit to what is in the 1st script. So you fix again, but then it will show that it does not fit to the 2nd script again, and so you end in a ping pong. We had such things in past, and the solution was that we commented out the change in the 1st script, leaving a ref to the corresponding PR in a comment. That was the only way to solve such stuff.
So immutable they were never. In some cases they had to be changed.
Now we are in the lucky situation that we don't have to support updates yet, so the period in which we can edit existing scripts is relative long. Normally we can edit an existing script only when it has not been released yes, e.g. I add a new script today, tomorrow someone finds mistake, and day after tomorrow is a release, then is is ok that it was touched.
This is why I think we should use this time now. If we would now act as if we would support updates from alpha x to alpha y, and we wanna change data structures, do things like nullable datetime columns and have the not nullable datetime columns valid default values, so we don't have any invalid datetime pronlemanymore which keeps us from MySQL strict mode, then we would have some 100 update scripts more, andf the first 50 would consist of comments that the statement has been moved to a later script due to this and that PR.
Sorry for my long text. I really don't want to argue. I only want to try again to explain you why I think we can do this now and why it is better. As soon as Beta is out, all will be like you and me are used to, the script will be nearly immutable except there is serious reason.
P.S.: The database schema checker is the only reason why we sometimes have to touch old update sql scripts, and if we did not have this checker, we could exactly proceed like Brian sais, once an update sql is created it should never be touched again. They could all run in a sequence from old to young, with all intermediate states, so if something to be changed again, just add a new file with younger date at the end. That would be perfect and maximum safe. I wish it would be like that.
Only because we have the checker we can't do that, and for the same reason I try to do as much as I can in the old files as long as possible, because otherwise the sequence of changes over the multiple files would be again not checker-compatible.
Restarted drone
Labels |
Added:
?
|
Agree with @richard67 - update files are never going to be totally immutable (for the reasons he stated - the upgrade checker isn't smart enough to understand - although this is edge cases) - but in my opinion there is no point in having bad data in the update files. For example here adding the bad index then removing it won't work with the schema checker - it will look for the index to exist and not exist at the same time (we've previously commented the files out for stable things)
Of course these are things that should be improved in the schema checker too so we can end up with immutable files - but we're not there yet. And to be honest for an alpha even if we get there i'd definitely think twice about whether it was needed to start creating the extra files
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-09-25 09:30:50 |
Closed_By | ⇒ | wilsonge |
@alikon Please test. @wilsonge Please test and/or review.