? ? Pending

User tests: Successful: Unsuccessful:

avatar nikosdion
nikosdion
31 Dec 2021

Pull Request for Issue # .

Summary of Changes

This PR addresses the incidental issues discovered in the conversation surrounding gh-36492. See #36492 (comment)

There are three changes made and explained in details below:

  • Simplified nested if–blocks in Joomla\CMS\Installer\Installer::parseSchemaUpdates
  • Update the schema version after fully executing an entire SQL update file instead of after executing all update files
  • Allow marking queries with /** CAN FAIL **/ before their final semicolon to signify that their failure should NOT halt the application of updates

Testing Instructions

I have put together a dummy component.

BEFORE applying the PR patch

Preparation*

If the dummy component is already installed, uninstall it.

Install version 1.0.0.

Result: success.

Check System, Database. The Dummy Component schema version is 1.0.0-202101011234

Test 1: cannot install updates once an older version with an SQL version is installed

Try to install version 1.0.1.

Result: update failure with message:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'LONTEXT, PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT COLLATE utf8mb4_unicode_c' at line 3

This is expected, version 1.0.1 is rigged with an invalid SQL update.

Check System, Database. The Dummy Component schema version is 1.0.0-202101011234. This is a problem and we'll see why in just a few moments.

Try to install version 1.1.0.

Result: update failure with message:

JInstaller: :Install: Error SQL Unknown column 'dummy_id' in 'test4_dummy_test'

That's because the first 1.0.1 update file (1.0.1-202102210753.sql) tries to execute again!

Check System, Database. The Dummy Component schema version is still 1.0.0-202101011234. Whomp whomp.

Test 2: SQL queries which we reasonably expect to fail cause the update to display an error

Uninstall the dummy component.

Install version 1.0.1.

Result: success. The clean installation does not go through the update files.

Check System, Database. The Dummy Component schema version is 1.0.1-202103142143.

Try to install version 1.1.0.

Result: update failure with message:

JInstaller: :Install: Error SQL Can't DROP 'test4_dummy_test_created'; check that column/key exists

This is expected. We have a SQL query in the 1.1.0-202112311820.sql file which tries to drop a key that does not exist.

Check System, Database. The Dummy Component schema version is still 1.0.1-202103142143. Whomp whomp.

After applying the PR patch

Preparation*

If the dummy component is already installed, uninstall it.

Install version 1.0.0.

Result: success.

Check System, Database. The Dummy Component schema version is 1.0.0-202101011234

Test 1: [FIXED] cannot install updates once an older version with an SQL version is installed

Try to install version 1.0.1.

Result: update failure with message:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'LONTEXT, PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT COLLATE utf8mb4_unicode_c' at line 3

This is expected, version 1.0.1 is rigged with an invalid SQL update.

Check System, Database. The Dummy Component schema version is 1.0.1-202102210753. This is great! It means that Joomla is aware that only the last SQL file has failed to execute. Neat!

Try to install version 1.1.0.

Result: SUCCESS!

Even though the previous update failed, we have correctly marked the first update file (1.0.1-202102210753.sql) as already executed. Updating to the next version up does not retry executing the file we have already run!

Check System, Database. The Dummy Component schema version is 1.1.0-202112311820.sql. Awesome sauce!

Test 2: [FIXED] SQL queries which we reasonably expect to fail cause the update to display an error

Uninstall the dummy component.

Install version 1.0.1.

Check System, Database. The Dummy Component schema version is 1.0.1-202103142143 as expected.

Result: success.

Try to install version 1.1.0.

Result: SUCCESS!

The last query of 1.1.0-202112311820.sql is marked as “CAN FAIL”. So when it fails no error is raised, as requested.

Check System, Database. The Dummy Component schema version is 1.1.0-202112311820.sql. Awesome sauce!

Documentation Changes Required

None

Additional information on the changes made

Simplified nested if–blocks in Joomla\CMS\Installer\Installer::parseSchemaUpdates

There were if–blocks nested three levels deep, making it very hard to read the code. As per modern PHP coding standards they have been replaced with early returns without functional changes to the code.

This has no functional change to the code. It just makes it more logical and easier to read.

Update the schema version after fully executing an entire SQL update file instead of after executing all update files

Until now, the #__schemas table was only updated with the name of the last SQL update file executed if and only if all SQL update files had been executed. This was problematic in case a failure occurred in the middle of executing multiple update files.

For example, an update from schema version 1.0.0-202101011200 to version 1.1.0-202112011200 might involve executing the following update files:

  • 1.0.1-202102070834
  • 1.0.1-202102121845
  • 1.0.2-202105221607
  • 1.1.0-202110151130
  • 1.1.0-202112011200

Each of these files may be modifying the schema and/or data in the database in a way that trying to re–apply each of these update SQL files would either fail or cause irreparable damage to the data.

If a SQL error occurred at the first SQL statement of the very last (1.1.0-202112011200) file, Joomla would still show the schema version as 1.0.0-202101011200. Trying to use the Database Fix page may or may not work — not all of the 1.1.0-202112011200 file would be executed, just the lines with DDL statements recognised by Joomla. Furthermore, trying to reinstall version 1.1.0 or any later version without using Database Fix would result in the files 1.0.1-202102070834 through 1.1.0-202110151130 to be executed again which would cause the update to fail or irreparably damage database data. This situation is extremely suboptimal.

With the change made in the parseSchemaUpdates() method we are now changing the recorded schema version right after applying each and every SQL update file. In our hypothetical scenario the recorded schema version would be 1.1.0-202110151130 instead of 1.0.0-202101011200. Even if Database Fix is not used it would still be possible to reinstall version 1.1.0 or a future update successfully, assuming that the root cause of the original SQL update failure is rectified in the database.

Allow marking queries with /** CAN FAIL **/ before their final semicolon to signify that their failure should NOT halt the application of updates

MySQL — unlike PostgreSQL — does not allow conditional column and index rename / modification / drop. That is to say, you can't tell MySQL to rename a column if and only if there is a column named such and such, or drop an index only if it exists.

For example, this DOES NOT work in MySQL:

ALTER TABLE `#__test` DROP KEY IF EXISTS `#__test_unique`;

Up to this point the only viable workaround was to use prepared statements like this:

SET @myQuery=if((
  SELECT true FROM `INFORMATION_SCHEMA`.`STATISTICS` WHERE
          `INDEX_SCHEMA` = DATABASE() AND
          `TABLE_NAME`        = '#__test' AND
          `INDEX_NAME` = '#__test_unique'
) = true,'ALTER TABLE `#__test` DROP KEY `#__test_unique`;','SELECT 1');

PREPARE stmt FROM @myQuery;
EXECUTE stmt;
DEALLOCATE PREPARE stmt;

There are two major downsides with these workarounds. First and foremost, they are difficult to write and comprehend. Second, they are not picked up by the Database Fix page.

Why do we need conditional execution? Let's say that you accidentally publish a broken update, like we did with version 1.0.1 of our dummy component. Future versions need to install cleanly even when this broken release was installed. This puts developers in a bit of a pickle. Sure, you can tell users to ignore the installation error, use the Database Fix page and then retry installing the update. If you had any UPDATE queries which had to execute in that SQL files, though, you're stuffed. They will never execute. You will have to figure out how to include them in the next update file in a way that they will execute only if they had not executed before.

Moreover, given the messy way real world sites are maintained (e.g. a database table from a newer schema specification may be restored on a site with an older schema specification recorded in the #__schemas table or vice versa) this leads to some updates being impossible to install or tables becoming inconsistent. This makes users think that the software is broken when, in fact, it's their horrible site maintenance to blame and the fact that Joomla does not allow for error recovery.

If you look into the core of this issue, none of these problems would exist if the developer could mark the DDL queries as acceptable to fail in a future update. The future updates would install cleanly. If the user has a borked schema they can still use the Database Fix page because the actual DDL query can still be parsed by Joomla.

So, how can you tell Joomla to ignore failures in DDL update queries?

I implemented this by allowed developers to add the /** CAN FAIL **/ special comment marker right before the final semicolon in a query. When this comment is present (exactly as presented above) the Installer code will ignore any errors coming from that query.

TO make that happen I created Installer::splitSql() — a slightly modified fork of DatabaseDriver::splitSql() — which does not suppress this comment, unlike DatabaseDriver::splitSql. Code is added in the Installer methods to remove the marker from the query being executed against the database. Since the database fixer uses DatabaseDriver::splitSql which removes all comments, including this marker, we do not affect Joomla's ability to fix a broken schema. Yay!

After this change, the aforementioned SQL could be written in a much more readable, concise format that can also be picked up correctly by the Database Fix page as follows:

ALTER TABLE `#__test` DROP KEY `#__test_unique` /** CAN FAIL **/;

This will greatly simplify database updates for non–trivial extensions and Joomla itself.

Note: Why not update DatabaseDriver::splitSql() itself? That would require an upstream change to the Joomla! Framework, however the change is something only required in the CMS' Installer class. Considering this and the fact that splitSql has remained largely unchanged in a decade it made more sense to fork it locally in the Installer class than ask the Framework to make a change they might very reasonably object to.

? Happy New Year!

I wish you all and your families a happy and prosperous New Year 2021. May it be a great year for Joomla!

avatar nikosdion nikosdion - open - 31 Dec 2021
avatar nikosdion nikosdion - change - 31 Dec 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Dec 2021
Category Libraries Front End Plugins
avatar richard67
richard67 - comment - 31 Dec 2021

Why delete and then insert the record with the schema version in database instead of just updating it?

avatar richard67
richard67 - comment - 31 Dec 2021

Drone failures are not related to this PR (I think).

avatar brianteeman
brianteeman - comment - 31 Dec 2021

codemirror update is because your branch is out of date.

avatar brianteeman
brianteeman - comment - 31 Dec 2021

as this pr completely breaks my pr (merge conflicts) you might as well merge mine into this

avatar nikosdion
nikosdion - comment - 31 Dec 2021

@richard67 Delete + insert is what Joomla does. In fact, we are not guaranteed to have a record there. I could optimise that a bit but it'd make the code more opaque. Of course not today — it's 10pm here — and not tomorrow. If you need to say the word.

@brianteeman If yours is merged before mine I'm willing to rebase mine. In the opposite case I can help you rebase yours. The idea being that even though we both touch the same code we do unrelated things and we've all agreed to keep PRs strictly to one issue at a time to prevent any SNAFUs like what we had in the past.

Regarding codemirror: I had updated to 4.1-dev before working. However, I think I may have typed npm update instead of npm install. I'll revert that change on Sunday. Thanks for the heads up.

avatar richard67
richard67 - comment - 31 Dec 2021

Have a happy new year.

avatar nikosdion nikosdion - change - 1 Jan 2022
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 1 Jan 2022
Category Libraries Front End Plugins Libraries
avatar nikosdion
nikosdion - comment - 1 Jan 2022

I have implemented the suggestion by @richard67 about update vs insert but I did keep the old code as a fallback, wrapped in a transaction, because I figured out there is a case where this might happen: if the SQL update file messes with the #__schemas table OR there is another process on the server doing that we might end up with the #__schemas record for the extension magically appearing or disappearing on us when we do not expect it. Instead of failing the installation we will fall back to the less efficient code, wrapped in a transaction which isolates the effects of another process upon our database data.

Finally, I have reverted the accidental CodeMirror change.

This is ready for testing.

avatar richard67
richard67 - comment - 15 Jan 2022

@nikosdion Now as @brianteeman 's PR #36492 has been merged into 4.0-dev and this has been merged up into 4.1-dev, this PR here has conflicts now (as ecpected). If you don't have the time to solve the conflicts, I can do that for you. But if you can do it, I should mention a change which we found out that it was necessary to get the new logs: We had to change the catch here to catch also a PrepareStatementFailureException. In your PR that change would have to be done here: https://github.com/joomla/joomla-cms/pull/36506/files#diff-b99bfdb7da0a1b88f83ba167cfd61b61b3ee3f3875ba5e252258cb761b9bbfb1R1178 and possibly other places, too.

Let me know if I shall do it.

avatar richard67
richard67 - comment - 15 Jan 2022

@nikosdion See nikosdion#2 for branch update and conflict resolution.

avatar nikosdion
nikosdion - comment - 15 Jan 2022

I merged your PR into my branch. I see that still only the libraries/src/Installer/Installer.php file appears changed (good). However, GitHub complains that “This branch is out-of-date with the base branch”. I am not sure why. Do you see the same message or is GitHub hating me because I don't have commit rights to the main Joomla repo?

avatar richard67
richard67 - comment - 15 Jan 2022

@nikosdion I see the same message. I think it's just because there has been another commit in the 4.10-dev branch meanwhile.

But there is something else which you should check: Have I added the Log::add(Text::_('JLIB_INSTALLER_SQL_END'), Log::INFO, 'Update'); at the right place?

avatar nikosdion
nikosdion - comment - 16 Jan 2022

@richard67 Yep, just checked it, it looks to all be in the right place.

avatar nikosdion
nikosdion - comment - 26 Feb 2022

@bembelimen Is this still something that's going to be merged? I am hitting a lot of weird real world cases where updates don't run because the schema version recorded by Joomla DOES NOT match what is really going in the database, typically because of a previous, failed update or because someone did something silly on their database manually. The only realistic way I can work around this in my extensions using Joomla's database schema management without breaking the Fix Database feature in Joomla is the “can fail” feature introduced in this PR.

avatar richard67
richard67 - comment - 26 Feb 2022

@nikosdion I'm not @bembelimen and so can't make any RL decisions. I had this PR here on my list for testing and had already reviewed the code changes, but then I had to focus on other, private stuff.

But there is something in this PR which I do not feel comfortable with. The PR does mainly 2 things:

  1. It fixes the issue that the schema version was not updated in database to the latest successfully run SQL update script and so with the next update attempt all the previously succeeded scripts would be run again.

  2. It allows statements to fail with the magic comment.

The first thing we need so or so, regardless if we take the second thing as it is or chose a different solution for the second thing.

About the second thing I am not sure if it is safe just to allow the statement to fail with any kind of failure or if we need some kind of failure type or condition, e.g. in case of an ALTER TABLE ADD COLUMN we could say /* CAN FAIL COLUMN_EXISTS /* or something like that, and then in case of any failure we check if the column already exists (i.e. allowed failure) or not (i.e. other failure) and act accordingly.

As said I am not sure yet if we would need something like that or if the simple /* CAN FAIL COLUMN_EXISTS /* is sufficient, but we had some discussion on Glip about it recently, and some people think we might need such conditions.

avatar richard67
richard67 - comment - 26 Feb 2022

P.S.: I think such conditions could be useful for DDL where we cannot use "IF NOT EXISTS" because not all databases support that for that kind of statement, e.g. for the example with adding the column and it exists already where it is MySQL not supporting it, while MariaDB and PostgreSQL do.

avatar nikosdion
nikosdion - comment - 26 Feb 2022

@richard67 I have actually given it a LOT of thought, I didn't just go cowboy coding here. Here are real world issues I have seen.

DDL statements where we try to rename a column can fail if the last update ran partially (meaning this query already ran) but the schema version has not been updated. Therefore they should be able to fail.

DDL statements where we drop a column or an index are not critical and should be able to fail.

DDL statements which change the table engine or encoding may fail if that engine or encoding is not supported on the server.

The only way we currently have to work around them is use a complex SELECT statement to assign a statement to a MySQL variable, followed by PREPARE, EXECUTE and DEALLOCATE. However, by doing that we break the Database Fix feature in Joomla which can no longer detect the schema changes, as expected.

INSERT statements with a hardcoded ID (e.g. what is used to populate a new table) may also fail in the main (not even the update!) SQL if the previous installation failed before Joomla could record a schema version. We can't use REPLACE INTO or INSERT IGNORE in Joomla because it breaks the Database Fix feature in Joomla.

The only statements which don't currently have a problem are CREATE TABLE and DROP TABLE, the two DDL statements MySQL supports IF EXISTS / IF NOT EXISTS. Everything else is basically hanging on a prayer and a lie, that the #__schemas table is reliable.

The #__schemas table is unreliable.

For starters, as things are right now it only gets updated when all SQL files execute. If they haven't? Not updated, therefore the next attempt to install or update the extension will fail on any of the DDL or DML statements.

Even after fixing that to execute after each SQL file is executed we still have the problem that it won't be updated if ANY of the queries in the SQL file fails, e.g. because the site has a complex history and something doesn't match our expectations based on the schema version. Again, any further attempts to update the install or update the extension will fail.

Even if I put each individual statement in its own file, making it impossible for me as a developer to manage SQL updates mind you, this can STILL fail: a DDL or DML SQL statement takes too long to complete, PHP times out and the schemas table is not updated. Any further attempt at installing / updating the extension failed with a SQL error.

Even if none of these are an issue for some magical reason, I still have to deal with sites which somehow have leftover tables from an earlier schema version of my extension but also appear to not have my extension installed (either because Joomla borked when running the DROP statements on uninstallation, or the server timed out on uninstallation or the user did something stupid with their database outside of Joomla). In this case my CREATE TABLE ... IF NOT EXISTS does not run and my DB tables have the wrong schema compared to what is recorded to the schemas table. My extension will fail to work and fail to update.

All of these are REAL WORLD issues I have seen on REAL WORLD sites. For each of these issues I have at least half a dozen support tickets in mind.

What am I supposed to do as a developer?

Tell my users "yeah, Joomla's schema management sucks balls, your only option is to mess with the database to drop tables and edit the schemas table, possibly resulting in data loss"? They don't care, they blame me, even though I'm as much a victim as they are!

Tell my users that they can't use my software? Suicidal, it's not even an option.

Ask the users to ask OSM for a refund for the purchase they made from me? I mean, sure, but it doesn't solve any problem.

Not use Joomla's DB installer at all? That's what I was doing with FOF and, frankly, I am seriously considering going back to if this problem isn't fixed. The downside is that my software no longer works with Joomla's Database Fix and I have to create my own feature to auto-repair the schema.

If you don't like my solution then fix Joomla's chronically broken database installation and upgrade code for real world sites. You can't just shove your head in the sand and pretend that if it doesn't work it's the developer's fault and responsibility because, no, dammit, it's not! 99% of the problems we have seen are caused by Joomla or the server and Joomla does not give us and our users the option to actually fix ANY of those problems! In the end of the day asking us 3PDs to suffer either by losing business or having to fix what Joomla broke on our clients' sites because the Joomla project doesn't want to fix its problems for over 15 years is unacceptable. This is not a high school weekend project, it's supposed to be "the CMS trusted by millions". I don't care how you fix it, just fix it already!

avatar richard67
richard67 - comment - 26 Feb 2022

I have actually given it a LOT of thought, I didn't just go cowboy coding here.

@nikosdion I know, and I've expected nothing else. And I agree with what you wrote. Personally I tend to say the PR is good as it is, and will try to find the time for testing it. But it's not on me to decide anything.

avatar richard67
richard67 - comment - 26 Feb 2022

We can't use REPLACE INTO or INSERT IGNORE in Joomla because it breaks the Database Fix feature in Joomla.

@nikosdion The database checker and fixer doesn't care about DML and so ignores these statements. So these would not break it. Update: Ah, I missed the thing with the variable, so I misunderstood. But still the below applies regarding the fixer.

The problem with the database fixer is that it doesn't fix the database, it fixes only the structure, DDL.

In past when we had maintained additional, extension-specific installation SQL scripts, the fixer was useful together with doing a discovery installation for new extensions which have been added with the update, because the extension's installation SQL did all the inserts and updates with the discovery installation.

But we have stopped to maintain such installation SQL scripts for the core extensions a while ago. For a few like com_finder there are some left, but we have none for those extensions which are added by 4.x.

And so the database fixer has become some kind of useless for fixing broken updates because all the DML doesn't run and will not be run with a discovery installation, like it was in past when we had that.

We could fix that either by introducing back own installation SQL for all extensions added from 3.10 to 4 so discovery installation can be used again after a broken update and a database fix.

Or we extend the database checker so it detects by the schema version that an update was incomplete and provides a button to complete it, i.e. run the missing update SQL and the rest of the finalization step. This would then work when your PR here is implemented and the update SQL scripts are provided with the /* CAN FAIL */ at the right places, because that would cater for previously only partly executed SQL scripts.

What do you think is the better way?

Or is that all bullshit?

avatar nikosdion
nikosdion - comment - 26 Feb 2022

Yeah, sorry man. It’s not your fault, I’m just frustrated.

I’ve been having these issues with Joomla’s extensions since 2008, when the upgrade method was first introduced in 1.5.3. In 2011 I ended up writing my own framework to isolate my extensions as much as possible from the Joomla issues as possible. Still, I’ve been affected by several installer issues I can’t work around on penalty of being delisted forever from the JED. Ever since I’ve been hearing the same lie from core maintainers and several bozos that it’s all my fault.

I said okay, now that core MVC in J4 doesn’t suck I’ll move to using nothing but core and I did exactly that. Predictably, I am affected by even more Joomla issues, including those my framework isolated me from and those nothing could isolate me from. Nobody can now tell me it’s my code; there’s none of it running on installation and update!

Even better, I can point my finger to core code and submit PRs to say hey this is broken because of this and that, here’s what has happened to me, here’s how you can address it. Nobody can accuse me of making it up, the code is here to prove me right all along.

The best part is another issue I’ve been reporting since 2008, that Joomla sometimes forgets to copy files, and I was told I’m making it up is now confirmed by Joomla leadership. Dunno man, did it really have to take 15 years of being insulted? Why not listen to me the first time around? I’ve been part of this community since it was called Mambo, I have a track record of knowing WTAF I’m doing!

So, yeah, I’m really frustrated. At this point I don’t care how these issues are fixed as long as they’re finally fixed, even if it took 15 years to get here. If they’re not fixed I want the project to admit it publicly. I’m sick and tired of being slandered and threatened for trying to help and I care too much to cease contributing like almost every other major 3PD has already done. You feel me?

Sorry for the rant.

avatar nikosdion
nikosdion - comment - 26 Feb 2022

This would then work when your PR here is implemented and the update SQL scripts are provided with the /* CAN FAIL */ at the right places, because that would cater for previously only partly executed SQL scripts.

That’s the idea, exactly!

avatar brianteeman
brianteeman - comment - 26 Feb 2022

Just a reminder to observers that the database fix was renamed to be structure fix as thats all it does. Unfortunately people on the forum think it will fix all sorts of weird things

avatar richard67
richard67 - comment - 26 Feb 2022

Just a reminder to observers that the database fix was renamed to be structure fix as thats all it does. Unfortunately people on the forum think it will fix all sorts of weird things

@brianteeman Yes, because our documentation nowhere mentions that it fixes the structure only. So for a broken small update with a structure change it is sufficient, but for fixing bigger updates where much data is changed or added, it is useless if there is no way to fix the data in addition.

avatar nikosdion
nikosdion - comment - 26 Feb 2022

Yeah, I know how it works and what it does.

In fact, it should be called “a simplistic and attempt to apply schema changes if the buggy SQL parser in Joomla does not choke on perfectly valid SQL because there’s a space or newline or because the developer used a more modern version of ALTER TABLE than Joomla supports, making it extremely unlikely it will ever work” but I guess that’s a mouthful…

Of course writing a SQL lexer and parser in PHP is kinda batshit crazy so we can’t complain that we have doesn’t work. We can only complain that it’s used despite us knowing it doesn’t actually work.

That’s reason number 13963 I think Joomla’s schema management is obsolete and unfit for purpose. What I was using in FOF (run SQL statements based on conditions which could even be SQL statements themselves!) was far more flexible in fixing ALL KINDS of database issues. And, to be fair, that code was just something I threw together in an afternoon! I can’t fathom that we couldn’t come up with something even better if we actually tried… The only problem is that it’s break b/c. Ah well, a man can dream…

avatar richard67 richard67 - test_item - 26 Feb 2022 - Tested successfully
avatar richard67
richard67 - comment - 26 Feb 2022

I have tested this item successfully on 3972a2e

I've successfully tested as described.

In addition, I've tested with both MySQL and PostgreSQL my changes on existing update SQL scripts for an upcoming PR which uses the new /** CAN FAIL **/ feature added by this PR here and a few other changes so the following worked:

  1. I've made a new installation of current 4.1-dev.
  2. I've patched the schema version for the core in the database of that installation so it is a 3.10 schema and all the 4.x update SQL scripts will run again in the next step.
  3. I've updated that installation using "Upload & Update" of the update component and a big update package for updating from 3.10 patched with the changes from this PR here plus the change from my PR #37154 plus my changes in the update SQL scripts.
  4. I've verified in the update log that all SQL statements from all update SQL scripts have run.

At the end there might be duplicate records here and there for this and that, which might be fixed with some other PR, but there was no SQL error caused by DDL (data definition language) where there is no IF EXISTS or IF NOT EXISTS supported by the particular database type, and there were no erros about violated primary or unique keys.

My branch with all changes together can be seen here: 4.1-dev...richard67:test-pr-36506 .

My PR with the changes on the SQL update scripts will come later.

Both together, this PR here and my upcoming one, will fix issues #35664 and #37141 .


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

avatar richard67
richard67 - comment - 26 Feb 2022

Additional test, also successful: Updated a 3.10 to a patched 4.1-dev package which has an SQL error at the beginning of one of the update SQL scripts. Result: The schema version is correctly set by this PR. Then updated again to the package without any SQL error: Result: The scripts which have not run yet are run, and at the end the schema version is updated right.

avatar richard67
richard67 - comment - 28 Feb 2022

Please test #37156 . Is uses the changes from this PR here, so at the end when having tested that PR you can mark a test result also for this one here (if you are not the author of this one here of course).

avatar nikosdion
nikosdion - comment - 28 Feb 2022

I like #37156. You have my implicit vote for it — I just can't provide the test since I am the author of this PR as per your comment.

I'm really happy to see an issue that's been nagging me for... oof... well over a decade head towards its demise (insert gleeful laughter).

avatar richard67
richard67 - comment - 28 Feb 2022

@nikosdion Well for my PR you can do a test, you only can't mark a test result for your own one.

avatar richard67
richard67 - comment - 28 Feb 2022

Ah, I see my tests in my PR do not cover all functionality from this PR here, so it still needs to test this one. My PR doesn't really use the schema version update fix.

avatar nikosdion
nikosdion - comment - 28 Feb 2022

Yeah, I thought that this PR was meant to be a prerequisite for your PR which prevents me from testing yours as I have authored this PR here :)

avatar alikon alikon - test_item - 1 Mar 2022 - Tested successfully
avatar alikon
alikon - comment - 1 Mar 2022

I have tested this item successfully on 3972a2e


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

avatar alikon alikon - change - 1 Mar 2022
Status Pending Ready to Commit
avatar alikon
alikon - comment - 1 Mar 2022

RTC


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

avatar bembelimen bembelimen - change - 17 Mar 2022
Labels Added: ?
avatar bembelimen
bembelimen - comment - 17 Mar 2022

I thing this PR is awesome but I think (also with @richard67 follow up PR) I think this should go in 4.2.

avatar richard67
richard67 - comment - 17 Mar 2022

@bembelimen Does it need new human tests now after the rebase?

avatar nikosdion
nikosdion - comment - 17 Mar 2022

@bembelimen I agree that it needs to go into a dot-zero release. I had rushed to finish it a month before the 4.1 release date to make it into the beta freeze but somehow it got overlooked. I have no problem if it goes into 4.2 as long as it actually does go in it!

avatar bembelimen
bembelimen - comment - 17 Mar 2022

@bembelimen Does it need new human tests now after the rebase?

I don't think so, probably the RL should run it once before merging :)

avatar roland-d roland-d - change - 24 Mar 2022
Labels Added: ?
Removed: ?
avatar roland-d roland-d - change - 24 Mar 2022
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2022-03-24 07:42:21
Closed_By roland-d
avatar roland-d roland-d - close - 24 Mar 2022
avatar roland-d roland-d - merge - 24 Mar 2022
avatar roland-d
roland-d - comment - 24 Mar 2022

Thank you everybody for making the installer better.

Add a Comment

Login with GitHub to post a comment