[3.10] Backport database changes from PR #30192 to fix broken updates from 3.10 to 4

[richard67]

Pull Request for Issue # .

Summary of Changes

This fixes update from 3.10 to 4 failing with SQL error due to not existing database columns by creating these columns already in 3.10.

It seems there is not really a way to fix it in J4, because first the PHP is updated and then the database, and the getTemplate call in the AdminApplication uses the new columns added by PR #30192 .

Testing Instructions

This PR should be tested with both kinds of databases, MySQL (or MariaDB) and PostgreSQL. If you have only one of these types, report back with your test result what kind of database you have used.

1. Update a clean 3.10-dev or latest 3.10 nightly to Joomla 4, using a Joomla 4 update package which already contains the changes from PRs [4.0] Child templates  #30192 , [4.0] Fix broken installation with SQL error on PostgreSQL databases #30327 and [4.0] Fix broken update with SQL error on PostgreSQL databases #30330 .
   Because there is no nightly update package yet which contains these changes, you can use following custom update URL or zip package, depending on which method you prefer, Live Update or Upload & Update:

• Custom URL https://test5.richard-fath.de/pr_list_pr-30330.xml
• Update package https://test5.richard-fath.de/Joomla_4.0.0-beta4-dev+pr.30330-Development-Update_Package.zip

Result: The update fails with an SQL error, see section "Actual result BEFORE applying this Pull Request" below.

2. Start again with a clean 3.10-dev or latest 3.10 nightly.

3. Run the SQL statements in the update SQL script added by this PR and suitable for your database type, replacing the #__ by your database prefix.

4. Do again an update with the custom update URL or zip package mentioned in step 1.

Result: The update suceeds when starting from a new installation of J 3.10 which has been updated before so that it contains the patch of this PR.

5. Start again with a clean 3.10-dev or latest 3.10 nightly.

6. Apply the patch of this PR.

7. If you have already made an installation, delete configuration.php.

8. Make a new installation.

9. Do again an update with the custom update URL or zip package mentioned in step 1.

Result: The update suceeds when starting from a new installation of J 3.10 which already included the patch of this PR.

Actual result BEFORE applying this Pull Request

Update to J4 fails:

The screnshot has been taken when testing with PostgreSQL. With MySQL or MariaDB it might look different but have the same result.

Expected result AFTER applying this Pull Request

Update to J4 succeeds.

Documentation Changes Required

None.

[zero-24]

I'm not sure whether this is a good solution tbh maybe we can apply them via script.php at the upgrade time? Or implement fall backs when that colums does not exists.

[richard67]

I'm not sure whether this is a good solution tbh maybe we can apply them via script.php at the upgrade time? Or implement fall backs when that colums does not exists.

@zero-24 I'm not sure either, but it could be the only way. The stuff in script.php runs as far as I remember after the update sql scripts have been applied, but the error happens before the sql update are applied. A fallback would be executed every time the getTemplate is called during the whole life time of J4, and this just to catch one case during the update. That looks not really reasonable to me.

[richard67]

@wilsonge Any idea(s)?

[alikon]

why we need those new j4 fields in 3.10 install script ?

[richard67]

why we need those new j4 fields in 3.10 install script ?

@alikon Can you read PR descriptions? It says it fixes failed update of J 3.10 to J4, right?

Does our update to J4 make any difference between a newly installed J 3.10 or a J 3.10 updated from a previous version?

Answer is: No, it doesn't.

So that's why we need it in both the installation and the update SQL script.

[zero-24]

Well we could try to catch the SQL errror and than fallback to the old query right?

[richard67]

Well we could try to catch the SQL errror and than fallback to the old query right?

Sure we can do that, but as I said, this means we do it for every time this query is tried, during the whole life time of J4. That seemed to be silly to me.

The same applies to doing a "show columns" to check if the columns are there before chosing the right query (with or without these columns).

In the latter case the number of useless checks could be at least reduced by using some static variable somewhere, keeping the result of that check.

If one of the above (exception or check the table structure) is a way to go, and someone provides a PR for J4, I will be happy to close this one here because I also don't like this PR here very much.

But on the other hand I don't want to establish a more silly solution in J4.

[richard67]

Any way it has to be fixed, because currently updating 3.10 to 4 is simply impossible.

[HLeithner]

3.10 is a combat release for 4.0 it makes sense to do this database change in this version, if we are unable to do it earlier in j4 upgrade process

[zero-24]

Well we could try to catch the SQL errror and than fallback to the old query right?

Sure we can do that, but as I said, this means we do it for every time this query is tried, during the whole life time of J4. That seemed to be silly to me.

Well a try/catch block means the catch is only executed when there is a error in the database query. There is no additional code executed on normal sites.

[zero-24]

3.10 is a combat release for 4.0 it makes sense to do this database change in this version, if we are unable to do it earlier in j4 upgrade process

Hmm as very last resort maybe but i don't think this is a good solution as this could happen inside future releases too and there we would not have 3.10 or similiar to hide such issue.

[richard67]

Well a try/catch block means the catch is only executed when there is a error in the database query. There is no additional code executed on normal sites.

Ok that's right, except maybe the small "try" part of it.

But an exception might have other reasons, e.g. database server down, and then we would also execute the other fallback query.

[zero-24]

Well a try/catch block means the catch is only executed when there is a error in the database query. There is no additional code executed on normal sites.

Ok that's right, except maybe the small "try" part of it.

But an exception might have other reasons, e.g. database server down, and then we would also execute the other fallback query.

Yes and when that seccond query fails too we can still show the error page right?

When the Database server is down we should not reach this part of the code anyway right?

[richard67]

Yes and when that seccond query fails too we can still show the error page right?

When the Database server is down we should not reach this part of the code anyway right?

We never know in which microsecond adb server decides to go down so theoretically we might get to that place of code while the db is up and then try to run the queries while it is down. But that's a very theoretical thing, I must admitt.

[HLeithner]

trying to catch a query because we failed to update is the wrong approach, if we introduce another column in the template and fail to update then this feature can't be added in the minor version or we find a better solution for this but at this time we have a solution. Every Joomla 4 upgrade has to be on Joomla 3.10 first.

[dgrammatiko]

@richard67 thanks for catching this and fixing it. I think the 3.10 is the easier way out here. One question though, as I think this isn't the first time that this is happening, why the order of Joomla update is a. overwrite the files b. apply db migration? I think reversing this order is a more logical procedure (of course it means extracting to a random folder in the /tmp fetching the sqls and then moving the files)

[richard67]

One question though, as I think this isn't the first time that this is happening, why the order of Joomla update is a. overwrite the files b. apply db migration? I think reversing this order is a more logical procedure (of course it means extracting to a random folder in the /tmp fetching the sqls and then moving the files)

@dgrammatiko Thanks for the feedback. I don't really know all the history why the order of processing is like it is, but I remember from the times when we implemented the utf8mb4 conversion that we needed the new, updated database driver to handle the update sql and conversion in the right way, e.g. with a downgrade to utf8 if necessary. So for that the order of processing was good.

Let's wait for more feedback, maybe someone has an idea how to handle it in J4.

Maybe I should open a J4 issue in parallel to this PR so we can track the J4 discussion better?

[richard67]

I mean we are discussing to fix the symptoms here. The true problem is how updating to J4 works. Why does it have to load the menu structure before running the SQL updates? Why do we need to log in some 150 times for that? (ok, it's only 2 times but still annoying). This login thing might be the reason why it loads the admin menu too early. If we would fix that, we would not need any other fix for the issue handled here.

[Fedik]

One question though, as I think this isn't the first time that this is happening, why the order of Joomla update is a. overwrite the files b. apply db migration?

It the same if you do update via FTP:
upload files => run DB fix

Doing opposite make no sense, and have more risk to crash the site and corrupt all user data

[HLeithner]

It the same if you do update via FTP:
upload files => run DB fix

if we don't do it in 3.10 the user can't fix the database with the DB fixer

[Fedik]

another "fix" would be change:

->select($db->quoteName(['s.template', 's.params', 's.inheritable', 's.parent']))

to

->select($db->quoteName(['s.*']))

[dgrammatiko]

Out of curiosity:

joomla-cms/libraries/src/Application/AdministratorApplication.php

Line 170 in 7897918

$this->route();

What's the point of routing for the admin (since there's no sef URLs in the backend? (this is what loads the menu)

[richard67]

@Fedik The db fix topic is a different thing: The fixer only runs structural changes, i.e. DDL (data definition language), e.g. CREATE TABLE, ALTER TABLE and so on, but it does not run content changes, i.e. DML (data manipulation language), i.e. INSERT, UPDATE, DELETE. That is the main reason why the old "copy the files and run the db fixer" method makes problems, not the order of processing, if you run the unpacked sql in a temporary folder before you apply the new PHP.

[richard67]

About the routing I have no idea.

About the SELECT * ..: That could work if we later check for each field in the result if it is set.

But it's a dangerous thing because one may make a PR to "optimize" db stuff and replaces the "*" by a dedicated columns list again ;-)

[HLeithner]

Guys I think we are wasting time here, please merge this as soon as we have 2 tests for it or come with a solution thats pretty and not a hack.

[richard67]

Well the testing instructions are complete and contain all what is needed ;-)

[dgrammatiko]

I have tested this item ✅ successfully on 8ad69db

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30333.}

[alikon]

I have tested this item ✅ successfully on 8ad69db

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30333.}

[alikon]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30333.}

[HLeithner]

Thanks

[wilsonge]

@richard67 we presumably now need a PR to J4 to remove the column creation scripts

[richard67]

@wilsonge I knew there was something I've forgotten ... will do soon.

[richard67]

@wilsonge Houston, we have a problem: In J4 I have to remove it from the update SQL (or comment it our) in order not to break update from 3.10 now as this PR is merged. But this will break updates from J4 Beta 1 to 3 to the next J4 Beta 4.

[Fedik]

Maybe we can go to SELECT * idea, how bad is it? of course leave a comment "danger: do not change because explosion!!!"

Or another idea is to check the version:

...
$query->select($db->quoteName(['s.template', 's.params']));

if ($version >= 4)
{
  $query->select($db->quoteName(['s.inheritable', 's.parent']));
}
...

Both not nice, but may work.

[dgrammatiko]

But this will break updates from J4 Beta 1 to 3 to the next J4 Beta 4.

Isn't there a conditional on sql, eg if column doesn't exist create it?

something like

ALTER TABLE `#__template_styles` ADD COLUMN IF NOT EXISTS `inheritable` tinyint(1) NOT NULL DEFAULT 0;

[dgrammatiko]

@Fedik let's not add unnecessary conditionals to the runtime if this can be handled in the sql

[richard67]

@dgrammatiko No I think there's nothing like that for alter table add column, but I will check later.

[dgrammatiko]

@richard67 I just realised this code is specific to MariaDB 😞

[richard67]

PR for removing the SQL for adding the columns from J4 as requested by @wilsonge in his comment above is #30363 . I suggest we continue the discussion in that PR, if necessary (whyt I believe).