Skip to content

Conversation

@OctavianC
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

Checks if the string utf8mb4 exists in the query - no need to do a regex replacement otherwise.
How I came across this: had a client that was attempting to install any extension and received the following errors:

JInstaller: :Install: Error SQL SQL=
Extension Install: SQL error processing query: DB function reports no errors.

The SQL is empty because the database did not support utf8mb4 AND the PCRE library the PHP was using was outdated and didn't understand (*SKIP)(*FAIL). Thus, preg_replace() returned false instead of the original $query.

It won't make much of a difference in the long run, but I've seen plenty of times this error on the Joomla! forum and perhaps this will solve it.

Testing Instructions

Install any extension that creates tables in the database. Make sure it installs correctly and the tables are created. Make sure utf8mb4 tables are downgraded to utf8 if the database does not support the character set (MySQL < 5.5 if I recall correctly).

Documentation Changes Required

None.

No need to do preg_replace() if utf8mb4 doesn't exist in the $query string.
@mehranfazili
Copy link

Worked fine here.

@ghost
Copy link

ghost commented Aug 26, 2018

@mehranfazili please mark your Test as successfully:

  • open Issue Tracker
  • Login with your github-Account
  • Click on blue "Test this"-Button above Authors-Picture
  • mark your Test as successfully
  • hit "submit test result"

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
@richard67
Copy link
Member

I have tested this item ✅ successfully on 4cd6dd8

Hacked the db driver so it returns always false for utf8mb4 support.
Both with and without this PR, a new installation of the CMS resulted in tables having utf8 character set and collations, so SQL statements are correctly downgraded with and without this PR.
Finally a code review tells me this change makes sense.
We are on the safer side in cased descibed in this PR, and in there might be also a performance gain according to PHP manuals.


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

@richard67
Copy link
Member

@franz-wohlkoenig I think @mehranfazili will not mark his successful test result in the issue anymore after more than 2 years. But theroetically it is still valid, and as the guy who worked much on that utf8mb4 stuff in past I can say this change is good and still valid, plus I've just tested as described in my test result.
=> Please set RTC.

@richard67
Copy link
Member

Pity I did not notice this PR before, but end of 2016 I had a very passive phase here on GitHub because of new job.

@Quy
Copy link
Contributor

Quy commented Jun 30, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 30, 2019
@HLeithner HLeithner merged commit 631d862 into joomla:staging Jun 30, 2019
@HLeithner
Copy link
Member

thx

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 30, 2019
@HLeithner HLeithner added this to the Joomla 3.9.9 milestone Jun 30, 2019
@richard67
Copy link
Member

@mbabker Does this need re-integration back to the framework database package, and if so for which versions, master and 2.0-dev or only one of both? If I know what to do I will make the necessary PR(s) to the framework package.

@mbabker
Copy link
Contributor

mbabker commented Jun 30, 2019

Any changes made in this repo should go to the Framework repo, in whatever branch(es) the change impacts. There should not be selective porting of changes, that's how things get inconsistent.

@richard67
Copy link
Member

@mbabker I checked and saw in the framework database package it is different anyway, and the change from this PR here is not really needed there. So nothing to do, it seems to me. Thanks for advising.

wilsonge added a commit to joomla-framework/database that referenced this pull request Aug 31, 2019
@OctavianC OctavianC deleted the patch-5 branch November 23, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants