Skip to content

Conversation

@twister65
Copy link
Contributor

@twister65 twister65 commented Jul 10, 2019

Pull Request for Issue #25467.

Summary of Changes

Use right data type of template styles home column in db.
Synchronize PR #25477 with PR #25484.

Testing Instructions

Install a copy of Joomla 3.9.10 on postgres.
Apply this PR to your joomla source directory.
Build an update package to your joomla-4.0-dev source directory:
git tag 4.0.0-alpha11-dev
php build/build.php
Update Joomla 3.9.10 with the new update package.
Check if Joomla upgrades successfully

Expected result

Actual result

Documentation Changes Required

@twister65
Copy link
Contributor Author

@richard67 and @alikon , please test.

@richard67
Copy link
Member

I have tested this item ✅ successfully on a37567f

Code review. Is obvious.


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

@richard67
Copy link
Member

@twister65 Maybe these sql statements should be extended in both mysql and postgresql to handle also template styles where the value for column home is not '0' or '1' but a language code, e.g. 'de-DE', 'en-GB', ...
I think that could be subject of another, future PR, so I gave this one here a good test.
But if you wanna do it in this one I can text again tomorrow evening.

@richard67
Copy link
Member

Hmm, I just see that could become too complicated maybe.

@richard67
Copy link
Member

@twister65 Maybe you should change title and description of this PR, because you don't change data type of a database column, you only use right data type in the schema update for inserting the data into that table. Other stuff (data type) is still correct in 4.0-dev because mistake from staging was not merged to 40.0-dev yet.

@twister65 twister65 changed the title [4.0] Upgrade - Change back data type of template styles home column in postgres [4.0] Fix Upgrade with postgresql- From Joomla v3.9.x to v4.0 Jul 11, 2019
@twister65
Copy link
Contributor Author

Sorry for the mistake, I did not anticipate your PR @richard67 .

@twister65
Copy link
Contributor Author

There is also another issue. When you select PostgreSQL (native) in Joomla 3.9.x and upgrade to Joomla 4.x, the database type is not recognized because Joomla 4.x only supports PostgreSQL (PDO).
In Global Configuration -> Server -> Database Type, MySQL (PDO) is displayed instead of PostgreSQL (PDO). And in the configuration file, $dbtype is 'postgresql', instead of 'pgsql'.
We must force the database type to PGSQL during the upgrade.

@richard67
Copy link
Member

richard67 commented Jul 22, 2019

We must force the database type to PGSQL during the upgrade.

Ping @wilsonge

@wilsonge wilsonge merged commit 8ae0a43 into joomla:4.0-dev Jul 22, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jul 22, 2019
@richard67
Copy link
Member

@wilsonge Any idea how we will handle switching from native PostgreSQL driver in 3.9.x or 3.10.x to the PostgreSQL PDO driver when updating to 4.0.x?

@wilsonge
Copy link
Contributor

Sorry I thought I'd already merged this!

I can't really remember now but i think the intention is to make people do this in Joomla 3.x (obviously we have the upgraded Joomla Upgrade Component in the 3.10 branch). Because obviously we have the same issue for the people currently on the mysql driver. That's the reason we backported the postgresql pdo driver into 3.x

@wilsonge
Copy link
Contributor

Thanks for testing guys and for the PR!

@twister65
Copy link
Contributor Author

Perhaps, we can add a test here:

public function update($installer)

@twister65
Copy link
Contributor Author

This is done here:

/*
* Joomla! 4.0 removes support for the `ext/pgsql` PHP extension. To help with the migration, we will migrate the configuration
* to the PDO PostgreSQL driver regardless of if the environment supports it. Instead of getting a "driver not found" type of
* error, this will instead force the API to report that the driver is not supported.
*/
if (strtolower($dbtype) === 'postgresql')
{
$dbtype = 'pgsql';
}

@richard67
Copy link
Member

Now as you posted, I remember having seen that, too, recently. But why has that not worked when you tested? Or has it worked, and I missunderstood your comment above?

@twister65
Copy link
Contributor Author

But I don't think it is called during the upgrade.
@wilsonge can enlighten us.

@richard67
Copy link
Member

Maybe the same thing should be added to the postflight method of administrator/components/com_admin/script.php?

@richard67
Copy link
Member

Or is postflight too late?

@wilsonge
Copy link
Contributor

What do you mean by during the upgrade :)

@twister65
Copy link
Contributor Author

Upgrade: from Joomla v3.9.x to v4.x.

@richard67
Copy link
Member

And during? ;-)

@wilsonge
Copy link
Contributor

I mean when we download the previous patch then we're still on the old db driver. As soon as you've unzipped the files and done the file cleanup (all part of the 0-100 status bar) and the page reloads then you should be using that database.php file from the DIC. As JFactory will have been overwritten with the new file and we'll be using the Joomla Framework DB Driver v2

@twister65 twister65 deleted the home-revert branch July 27, 2019 14:53
@twister65
Copy link
Contributor Author

twister65 commented Jul 28, 2019

As JFactory will have been overwritten with the new file and we'll be using the Joomla Framework DB Driver v2

Yes, otherwise the upgrade would have failed.
But $dbtype is not correct in the configuration file. I think we need to solve this problem.

@richard67
Copy link
Member

Agree.

@twister65
Copy link
Contributor Author

Please, review / test this PR #25729 .

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.

4 participants