Problem/Motivation

For PostgreSQL in Drupal 10 will the pg_trgm extension be a requirement. The extension needs to be installed and created. Both must be done by a database user that has at least superuser privileges. The database user that is used by Drupal should not have those privileges. That would be a security risk.

Proposed resolution

Add a hook_requirements() that will throw an error when the extension is not installed and/or not created. The hook should already exist and should be changed from a warning to an error.

Remaining tasks

TBD

User interface changes

None

API changes

New method called Drupal\Core\Database\Driver\pgsql\Schema::extensionExists(string $name). Which returns true when the given extension name is installed and created or return false when that is not the case. This method should already exists.

Data model changes

None

Release notes snippet

Drupal 10's PostrgreSQL database driver requires PostgreSQL 12 with the pg_trgm extension enabled. A requirements error is now displayed when installing or updating a PostgreSQL site without this extension enabled.

CommentFileSizeAuthor
#7 interdiff_5-7.txt2.84 KBmurilohp
#7 3214922-7.patch5.51 KBmurilohp
#5 3214922-5.patch5.15 KBmurilohp

Comments

daffie created an issue. See original summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Add a requirements error when PostgreSQL is used and the pg_trgm extension is not installed or created » Add a requirements error in Drupal 10 when PostgreSQL is used and the pg_trgm extension is not installed or created
Status: Active » Postponed
Parent issue: #3190516: [Policy] For PostgreSQL require that the pg_trgm extension is not only installed, but also created. » #3118149: [meta] Requirements for tagging Drupal 10.0.0-beta1
Related issues: +#3214921: Add a requirements warning in Drupal 9 when PostgreSQL is used and the pg_trgm extension is not created, +#3190516: [Policy] For PostgreSQL require that the pg_trgm extension is not only installed, but also created.

Make target version clear in title, since we want to make a slightly different change in Drupal 9: #3214921: Add a requirements warning in Drupal 9 when PostgreSQL is used and the pg_trgm extension is not created. Also marking postponed since Drupal 10 is not yet opened. Also re-parenting for the beta requirements.

daffie’s picture

daffie commented
Status: Postponed » Active

#3214921: Add a requirements warning in Drupal 9 when PostgreSQL is used and the pg_trgm extension is not created has landed and the 10.0.x branch is open.

murilohp’s picture

murilohp commented
Assigned: Unassigned » murilohp
murilohp’s picture

murilohp commented
Assigned: murilohp » Unassigned
Status: Active » Needs review
StatusFileSize
new 3214922-5.patch5.15 KB

Hey, I was taking a look at this and write a patch to handle this situation, I made a little refactor on core/modules/system/system.install, now the status page shows if the pg_trgm extension is enabled, this way I was able to write a test. I also created a new task function at core/modules/pgsql/src/Driver/Database/pgsql/Install/Tasks.php to handle extensions when installing a new site with postgres.

Thanks!

daffie’s picture

daffie commented
Status: Needs review » Needs work

The patch looks very good!

Just 1 remark:

+++ b/core/modules/system/system.install
@@ -552,17 +552,26 @@ function system_requirements($phase) {
+    $connection = Database::getConnection();
...
+    if (Database::isActiveConnection() && ($connection->driver() == 'pgsql')) {

You cannot add this here. You can only add this after Database::isActiveConnection(). There might not be a database connection and an exception will be thrown.

murilohp’s picture

murilohp commented
StatusFileSize
new 3214922-7.patch5.51 KB
new interdiff_5-7.txt2.84 KB

Fixing the coding standards and addressing #6 thanks @daffie!

murilohp’s picture

murilohp commented
Status: Needs work » Needs review
daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
I do not see how we can add more testing for the changes.
For me it is RTBC.

@murilohp: Thank you for working on this.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed b74bb8f and pushed to 10.0.x. Thanks!

  • catch committed b74bb8f on 10.0.x 
    Issue #3214922 by murilohp, daffie: Add a requirements error in Drupal...
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +10.0.0 release notes

This warning is new since alpha1, and is enforcing the platform requirement that will be documented in the 10.0.0 notes as well.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes

Added release note.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes

Linked the existing CR from the release note.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.