[#12847] - Small performance check for downgrading utf8mb4
- Closed
- 30 Jun 2019
- Medium
- Build: staging
- # 12847
- Diff
- OctavianC:patch-5
- Success Hound No violations found. Woof! Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
- Failure continuous-integration/drone/pr Build is failing Details
- Pending JTracker/HumanTestResults Human Test Results: 1 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
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.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
@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"
I have tested this item
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.
@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.
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.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12847.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-06-30 17:17:47 |
| Closed_By | ⇒ | HLeithner |
thx
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.
Worked fine here.