create sql update file, eg:
ALTER TABLE #__my_table
ADD IF NOT EXISTS new_column
text;
Do NOT update component for reproducing this error; just keep query in update/sql/mysql folder
Backend, com_installer, view "database": check returns missing column "new column" of type "text"
Database check returns missing column "IF" of type "NOT"
Joomla 4.2.5 on PHP 8.1
Database check in libraries/src/Schema/ChangeSet.php does not ignore phrases like "if not exists" or "if exists" (eg for drop queries). Without these phrases joomla update fails if query could not be executed, eg. if column "new_column" still exists. So to prevent update errors I add phrases "if not exists"... to make sure update will perform.
A quick and dirty solution could be modify "libraries/src/Schema/ChangeSet.php" on line 313 by changing
$fileQueries->updateQuery = $query;
to
$fileQueries->updateQuery = preg_replace("/(IF EXISTS|IF NOT EXISTS)/", "", $query);
After that, everything works like a charm...
Labels |
Added:
No Code Attached Yet
|
See https://dev.mysql.com/doc/refman/8.0/en/alter-table.html for MySQL and https://mariadb.com/kb/en/alter-table/ for MariaDB.
For adding and removing columns, extensions devs can use the „CAN FAIL“ installer hint which was added with #36506 for their „ALTER TABLE … ADD COLUMN“ statements in their update SQL scripts.
in my opinion this issue should be closed as expected behavior. I can’t do that right now because not at my desktop.
Status | New | ⇒ | Expected Behaviour |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-11-30 17:29:17 |
Closed_By | ⇒ | richard67 |
Closing as expected behaviour for the reason stated above.
If we will have separate database drivers for MySQL and MariaDB in version 5 (or 6?), we can implement such MariaDB-specific features.
Furthermore, extension developers who use specific MariaDB syntax not supported by MySQL should simply be punished, and their extensions should be wiped out from JED.
I did not know MySQL doesn't support such statements; I'm using; MariaDB since years and did not take care of differences - my fault. I will change all statements to statements that are compatible with both databases. If you want to punish me for that: feel free...
Thanks for your answer and helping me make my extensions more compatible.
No prob... I'm really happy about your help with "CAN FAIL"... Thanks!!!
@richard67 I modified my update script:
ALTER TABLE #__my_table ADD new_column text /** CAN FAIL **/;
But it still throws an error: "Duplicate column name 'new_column'". I checked installer.php script and the $canFail var is set to true for this query in parseSchemaUpdates() method, but it seems to exit the script execution after this first query. The next lines of update script were ignored. If I log the steps I can see that no code after executing the first query ($db->setQuery($query)->execute();) will be executed. Even catch-cases wasn't called. What am I doing wrong?
@richard67 I modified my update script:
ALTER TABLE #__my_table ADD new_column text /** CAN FAIL **/;
But it still throws an error: "Duplicate column name 'new_column'". I checked installer.php script and the $canFail var is set to true for this query in parseSchemaUpdates() method, but it seems to exit the script execution after this first query. The next lines of update script were ignored. If I log the steps I can see that no code after executing the first query ($db->setQuery($query)->execute();) will be executed. Even catch-cases wasn't called. What am I doing wrong?
@dawe78 When does it throw that error? When you use the database fixer to apply your change? Then it is expected. The hint is only for the installer when it runs the update SQL. If you create an update package for your installation, the statement should not throw an error.
@richard67 This error appears if I try to update my component. Component update failed, which should not be the expected behaviour, I think?! And I think the queries after the failed query should be executed too on component update?!
@richard67 This error appears if I try to update my component. Component update failed, which should not be the expected behaviour, I think?! And I think the queries after the failed query should be executed too on component update?!
Yes, the queries after that should be executed, too. Maybe it fails for another reason? You should check the logs of your MariaDB server, and if you don’t have access because on a hosting company, use a local copy with a local db server where you can access the logs.
@richard67 I tried to figure out what happened and I think I got it:
libraries/vendor/joomla/database/src/MySqli/MySqliStatement.php, line 434:
if (!$this->statement->execute())
{
throw new ExecutionFailureException($this->query, $this->statement->error, $this->statement->errno);
}
Calling $this->statement->execute() throws an exception (mysqli_sql_exception) and code execution is interrupted. So I changed the above lines to catch mysqli_sql_exception and put it in ExecutionFailureException:
try {
$this->statement->execute();
} catch (\Exception $e) {
throw new ExecutionFailureException($this->query, $this->statement->error, $this->statement->errno);
}
Now update works as expected without errors. What do you think about this solution, @richard67 ?
@dawe78 That seems to me just changing the exception type.
I think it is expected that the execute throws an exception when it fails, and we catch that in the callong function or somewhere more above in the call tree. But it might be that we only catch RuntimeException types and not ExecutionFailureException types, or vice versa, and the kind of exception might depend on the database client (mysqli or pdo) and version (there were some changes in PDO with PHP 8.0 or 8.1 about that). We had a similar case elsewhere, see https://github.com/joomla/joomla-cms/pull/35951/files#diff-64a03c6e98e4fa15e90e706da227307887df2b68ecc691e7daa7b56183873eeeR254 .
That doesn't mean that I think your fix is wrong. maybe it's right, but then it should be made in the framework for the 2.d-ev branch here https://github.com/joomla-framework/database , and possibly also for other database drivers.
I will see when I can find the time to check that deeper. But it would help if you had a call stack so I know where to start.
That seems to me just changing the exception type.
Yes, thats what it is. But installer.php::parseSchemaUpdates() method catches in query-check only exceptions of type ExecutionFailureException or PrepareStatementFailureException, but correct exception handling is necessary for $canFail option. If it comes with the wrong exception type, script execution is interrupted at this point and no other query will be executed.
Can I do anything to help you with this possible fix?
Stack trace:
Joomla\Component\Installer\Administrator\Controller\UpdateController::update() called in [basePath]/libraries/src/MVC/Controller/BaseController.php on line 672
Joomla\Component\Installer\Administrator\Model\UpdateModel::update(0) called in [basePath]/administrator/components/com_installer/src/Controller/UpdateController.php on line 58
Joomla\Component\Installer\Administrator\Model\UpdateModel::install() called in [basePath]/administrator/components/com_installer/src/Model/UpdateModel.php on line 354
Joomla\CMS\Installer\Installer::update([basePath]/tmp/install_638f4b614fa4d) called in [basePath]/administrator/components/com_installer/src/Model/UpdateModel.php on line 454
Joomla\CMS\Installer\InstallerAdapter::update() called in [basePath]/libraries/src/Installer/Installer.php on line 835
Joomla\CMS\Installer\InstallerAdapter::install() called in [basePath]/libraries/src/Installer/InstallerAdapter.php on line 1255
Joomla\CMS\Installer\Adapter\ComponentAdapter::parseQueries() called in [basePath]/libraries/src/Installer/InstallerAdapter.php on line 766
Joomla\CMS\Installer\InstallerAdapter::parseQueries() called in [basePath]/libraries/src/Installer/Adapter/ComponentAdapter.php on line 622
Joomla\CMS\Installer\Installer::parseSchemaUpdates(10010) called in [basePath]/libraries/src/Installer/InstallerAdapter.php on line 835
Joomla\Database\DatabaseDriver::execute() called in [basePath]/libraries/src/Installer/Installer.php on line 1286
(at this point, exceptions of type ExecutionFailureException or PrepareStatementFailureException are expected, but mysqli returns mysqli_sql_exception)
Joomla\Database\Mysqli\MysqliStatement::execute() called in [basePath]/libraries/vendor/joomla/database/src/DatabaseDriver.php on line 675
(here I changed exception type to return expected exception type to Installer)
I hope this helps. Let me know if I can do more than that!
@dawe78 mysqli_sql_exception extends from RuntimeException, see https://www.php.net/manual/en/class.mysqli-sql-exception.php . So I think we just should extend line 1281 in the current 4.2-dev branch(https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Installer/Installer.php#L1281)) in Installer.php to check for RuntimeException, too:
} catch (ExecutionFailureException | PrepareStatementFailureException | \RuntimeException $e) {
I think this should work.
@dawe78 How does it come that your stack trace reports line 1286 and not 1281 as the one with the Joomla\Database\DatabaseDriver::execute()
call? Have you added some debug stuff to file libraries/src/Installer/Installer.php
? Or is your Joomla installation not up to date?
How does it come that your stack trace reports line 1286 and not 1281 as the one with the
Joomla\Database\DatabaseDriver::execute()
call? Have you added some debug stuff to filelibraries/src/Installer/Installer.php
? Or is your Joomla installation not up to date?
Oh, I'm sorry, I put some lines for logging... my Joomla installation is up to date...
} catch (ExecutionFailureException | PrepareStatementFailureException | RuntimeException $e) {
Works for me; just tested with original MySqliStatement.php...
@dawe78 I can make a pull request with that fix, but not before on weekend. The most work with it will be to see how the error can be reproduced, which might depend on database driver and version and possibly also on the kind of SQL statement, and possibly to create some test package for other testers because each PR will need 2 successful tests by humans (I hope you will be one of them).
@richard67 I could create a testing component package with single database table and an update package with test statements if you want me to. And of course I will do the test with MariaDB.
@richard67 Okay, where can I upload the packages?
@dawe78 Pull request is ready. Please test #39391 . When having tested, go to the pull request in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/39391 , use the blue "Test this" button at the to left corner, then select your test result (hopefully "Tested successfully") and then submit. Thanks in advance.
@richard67 I have tested this fix and posted test result...Thank your for your help!!!
The if exists and if not exists for adding and removing columns is MariaDB only, MySQL doesn’t understand it. Because we have to support both kinds of databases we cannot use it. Furthermore, extension developers who use specific MariaDB syntax not supported by MySQL should simply be punished, and their extensions should be wiped out from JED. Another thing are if exists and if not exists for adding and removing tables. This is supported by both kinds of databases and so also by our db checker.