? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
9 Jul 2019

Pull Request for Issue #25482 .

Summary of Changes

Change back data type of column home to string (type depending on db type) in table #__template_styles, because it can not only hold values '0' or '1' but also language code, e.g. 'de-DE', if the template style is a default for a particular language.

This is an undo of PR #24595 .

Testing Instructions

4 different tests.

  1. Update Joomla 3.9.8 site to a patched 3.9.9 package with this PR and the version number patched to 3.10. PLEASE DO THIS ONLY ON TESTING SITE, NOT ON LIFE SITE.
    You can find such a package here:
    https://test5.richard-fath.de/Joomla_3.9.10-dev-Update_Package_pr-25484.zip.

  2. Do the same with a 3.9.9: Update it to my patched package.

  3. On a 3.9.8, apply this PR e.g. with patch tester, then go to "Extensions -> Manage -> Database". Use the "Fix" button to apply the changes in the new schema updates from this PR.

  4. Do the same as in step 3 but on a 3.9.9

Expected result

In all cases the update or schema fix succeeds, and issue #25482 is solved.

Actual result

See issue #25482 .

Documentation Changes Required

None.

avatar richard67 richard67 - open - 9 Jul 2019
avatar richard67 richard67 - change - 9 Jul 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Jul 2019
Category SQL Administration com_admin Postgresql MS SQL
avatar richard67 richard67 - change - 9 Jul 2019
Labels Added: ?
avatar mbabker
mbabker - comment - 9 Jul 2019

For the record, the fact you have to rename the index is exactly why the bug in the schema parser needs to be fixed. You should not have to version suffix table indexes.

avatar richard67
richard67 - comment - 9 Jul 2019

@mbabker Yes, I know. But it is not a bug, it is missing by design. That's why I did not fix it yet, it is a bigger thing.

avatar richard67
richard67 - comment - 9 Jul 2019

Attention, PR is not ready yet, changes in joomla.sql are missing. But you can already apply the PR and run the DB fixer for testing the schema updates are working.

avatar mbabker
mbabker - comment - 9 Jul 2019

It's a bug if it's missing this critical of a feature 😉

(And yeah, I realize fixing the database code is a lot more effort than it's sometimes worth, but still have to make it known I hate hacky fixes to work around a deeper underlying problem)

avatar HLeithner
HLeithner - comment - 9 Jul 2019

ALTER TABLE #__template_styles DROP INDEX idx_client_id_home; may fail because the index doesn't exists on direct update

avatar joomla-cms-bot joomla-cms-bot - change - 9 Jul 2019
Category SQL Administration com_admin Postgresql MS SQL SQL Administration com_admin Postgresql MS SQL Installation
avatar richard67
richard67 - comment - 9 Jul 2019

@mbabker I absolutely agree with you about the issue, but it is not just a bug fix. It needs a redesign of the schema checker. That's why we now have to use the ugly hacks, we don't have the time now to fix the schema checker.

avatar richard67
richard67 - comment - 9 Jul 2019

@HLeithner Not sure. For postgres I have used "IF EXISTS", that should be ok. For the other DB types we don't have that yet. I have to check that.

avatar richard67 richard67 - change - 9 Jul 2019
The description was changed
avatar richard67 richard67 - edited - 9 Jul 2019
avatar richard67 richard67 - change - 9 Jul 2019
The description was changed
avatar richard67 richard67 - edited - 9 Jul 2019
avatar richard67 richard67 - change - 9 Jul 2019
The description was changed
avatar richard67 richard67 - edited - 9 Jul 2019
avatar richard67
richard67 - comment - 9 Jul 2019

Hint for testers: Link to modified update package in description has changed, package has been updated by latest changes on this PR.

avatar simbus82
simbus82 - comment - 9 Jul 2019

@HLeithner Not sure. For postgres I have used "IF EXISTS", that should be ok. For the other DB types we don't have that yet. I have to check that.

Something like this for MySQL can help?
(only some new versions of mysql like MariaDB supports IF EXIST for indexes)

SET @idxexist := (SELECT COUNT(*) FROM information_schema.statistics WHERE table_name = '#__template_styles' AND index_name = 'idx_client_id_home' AND table_schema = database());
SET @sqlrmidx:= if( @idxexist > 0, 'SELECT ''INFO: Index already exists.''', 'ALTER TABLE #__template_styles DROP INDEX idx_client_id_home');
PREPARE rmidx FROM @sqlrmidx;
EXECUTE rmidx;
avatar richard67
richard67 - comment - 9 Jul 2019

@simbus82 That would work for the installer when doing the update, but the database schema checker would be confused. we found other way. pr is ready for being tested.

avatar richard67
richard67 - comment - 9 Jul 2019

@alikon @twister65 Can you test this PR for postgres? I gotta go sleep now.

avatar richard67 richard67 - change - 9 Jul 2019
The description was changed
avatar richard67 richard67 - edited - 9 Jul 2019
avatar wilsonge
wilsonge - comment - 10 Jul 2019

I have tested this item ✅ successfully on 71252b7

On a mariadb setup this worked for me on the downloads.joomla.org backup I have (which experienced this issue) going from both 3.9.8 backup to this proposed 3.9.10 directly and also going from 3.9.8 to 3.9.9 and then to the proposed 3.9.10. In both cases the schema checker also didn't show any issues.


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

avatar wilsonge wilsonge - test_item - 10 Jul 2019 - Tested successfully
avatar alikon
alikon - comment - 10 Jul 2019

not able to do a full test on postgresql ... just a quick test on SQL Fiddle
Cattura1

avatar infograf768
infograf768 - comment - 10 Jul 2019

I have tested this item ✅ successfully on 71252b7

This corrects the issue here with mysqli


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

avatar infograf768
infograf768 - comment - 10 Jul 2019

I have tested this item ✅ successfully on 71252b7

This corrects the issue here with mysqli


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

avatar infograf768 infograf768 - test_item - 10 Jul 2019 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 10 Jul 2019
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Jul 2019

Status "Ready To Commit".

avatar wilsonge
wilsonge - comment - 10 Jul 2019

@alikon not good enough for a postgresql test for this I'm afraid. We need to be sure our index's are being preserved. Anyone else here able to do a postgresql test before this is merged

avatar franz-wohlkoenig franz-wohlkoenig - change - 10 Jul 2019
Status Ready to Commit Pending
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Jul 2019

Set back on pending by comment of @wilsonge above.

avatar alikon
alikon - comment - 10 Jul 2019

i know, but cannot help today more than this ...

avatar wilsonge
wilsonge - comment - 10 Jul 2019

https://docs.joomla.org/Tables/template_styles i've updated this page to state it's a varchar and noted the type of the database

avatar wilsonge
wilsonge - comment - 10 Jul 2019

@alikon are you able to at least test it with the index's applied to that table rather than just with the table structure? I installed postgres with brew last night onto my macbook but didn't get time to set up a full joomla install - and won't be able to until tonight

avatar alikon
alikon - comment - 10 Jul 2019

sorry @wilsonge & all, i'm able to do only some spot SQL Fiddle

Cattura2

fd7d092 10 Jul 2019 avatar HLeithner pgsql
avatar HLeithner
HLeithner - comment - 10 Jul 2019

I tested all postgresql scenarios with postgresql 9.6 and worked now.

thx richard

avatar HLeithner HLeithner - close - 10 Jul 2019
avatar HLeithner HLeithner - merge - 10 Jul 2019
avatar HLeithner HLeithner - change - 10 Jul 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-07-10 14:47:25
Closed_By HLeithner

Add a Comment

Login with GitHub to post a comment