Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

postgresql 16 release #295

Merged
merged 3 commits into from
Aug 19, 2024
Merged

postgresql 16 release #295

merged 3 commits into from
Aug 19, 2024

Conversation

alikon
Copy link
Contributor

@alikon alikon commented Dec 25, 2023

Pull Request for Issue (joomla/joomla-cms#42335)

Summary of Changes

as per https://www.postgresql.org/docs/current/release-16.html
Remove read-only server variables lc_collate and lc_ctype

Testing Instructions

with Postgres 16
Go to admin area.
Try to show system info

Documentation Changes Required

@HLeithner
Copy link
Contributor

Can we get it for the current database instead of the global DB?

@teoberi
Copy link

teoberi commented Jan 10, 2024

Why is this not in 5.0.2?

@richard67
Copy link
Contributor

Why is this not in 5.0.2?

@teoberi It would help maybe if you would test the changes and leave a comment here about the result.

@teoberi
Copy link

teoberi commented Aug 18, 2024

Why is this not in 5.0.2?

@teoberi It would help maybe if you would test the changes and leave a comment here about the result.

I thought it was quite clear when I asked why it is not in 5.0.2 because since that version I use the solution here and everything is OK. The only problem is that I have to apply the changes manually after each version update.
PHP 8.3.10
Postgresql 16.4
both compiled from sources.

@richard67
Copy link
Contributor

Can we get it for the current database instead of the global DB?

@alikon What Harald means is if there is a way go get the current database default collation instead of the global variable which was removed. Do you know if there is a way to get that? Or shall I try to find out?

@teoberi
Copy link

teoberi commented Aug 19, 2024

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b0f6c437160db640d4ea3e49398ebc3ba39d1982
https://www.postgresql.org/docs/current/catalog-pg-database.html

datlocprovider char
Locale provider for this database: c = libc, i = icu
datcollate text
LC_COLLATE for this database
datctype text
LC_CTYPE for this database
daticulocale text
ICU locale ID for this database

@richard67
Copy link
Contributor

richard67 commented Aug 19, 2024

@alikon I've just tried the following SQL statement from the page linked in @teoberi 's comment on a PostgreSQL 14.12 where SHOW LC_COLLATE still works, too:

SELECT "datcollate" FROM "pg_database" WHERE "datname" = current_database();

The result de_DE.UTF-8 from that SQL statement is the same as the result of SHOW LC_COLLATE.

Could you change this PR here so it uses that statement on PostgreSQL >= 16 instead of returning false?

P.S.: When using the following statement you get the same column name in the result:

SELECT datcollate AS lc_collate FROM pg_database WHERE datname = current_database();

So the following code should work:

        if (version_compare($this->getVersion(), '16.0', '>=')) {
            $this->setQuery('SELECT datcollate AS lc_collate FROM pg_database WHERE datname = current_database()');
        } else {
            $this->setQuery('SHOW LC_COLLATE');
        }

        $array = $this->loadAssocList();

        return $array[0]['lc_collate'];

@alikon
Copy link
Contributor Author

alikon commented Aug 19, 2024

thanks @teoberi & @richard67

@teoberi
Copy link

teoberi commented Aug 19, 2024

It works now.

@teoberi
Copy link

teoberi commented Aug 19, 2024

https://ci.joomla.org/joomla-framework/database/1616/1/3
The ifs have an extra space, i.e. 9 instead of 8.

@richard67
Copy link
Contributor

https://ci.joomla.org/joomla-framework/database/1616/1/3 The ifs have an extra space, i.e. 9 instead of 8.

@teoberi Has been fixed by @alikon with commit 2a32f4c .

@teoberi
Copy link

teoberi commented Aug 19, 2024

I hope that the Pull request will be accepted now and I will get rid of the manual changes.

@richard67
Copy link
Contributor

I hope that the Pull request will be accepted now and I will get rid of the manual changes.

@teoberi Unfortunately it will be a bit too late for the upcoming 4.4.7 and 5.1.3 releases, which will be released tomorrow, but it will very likely be in the Joomla 5 release after that, which will be 5.2.0 in October.

@richard67
Copy link
Contributor

I have tested this and it works as expected

@rdeutz rdeutz merged commit 3927dea into joomla-framework:3.x-dev Aug 19, 2024
1 of 2 checks passed
@alikon alikon deleted the patch-3 branch August 19, 2024 16:26
@teoberi
Copy link

teoberi commented Oct 15, 2024

I hope that the Pull request will be accepted now and I will get rid of the manual changes.

@teoberi Unfortunately it will be a bit too late for the upcoming 4.4.7 and 5.1.3 releases, which will be released tomorrow, but it will very likely be in the Joomla 5 release after that, which will be 5.2.0 in October.

No, not even in 5.2.0!

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.

5 participants