Add a requirements warning in Drupal 9 when PostgreSQL is used and the pg_trgm extension is not created

Make target version clear in title, since we want to make a slightly different change in Drupal 10: #3214922: Add a requirements error in Drupal 10 when PostgreSQL is used and the pg_trgm extension is not installed or created.

Interlinking the Drupal 10 issue.

Looks great except naming to make it more extendable in future

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -1060,6 +1060,21 @@ protected function hashBase64($data) {
+   * Determines whether the PostgreSQL extension is created.
...
+  public function extensionExists($name) {

I'd make it more broader - requirementsValid(): bool

Also would be great to mark method @internal

Looks great, I'd just added link to official docs about creation

hey @daffie,

A few comments:

1. Issue description has this:

The database user that is used by Drupal should not have those privileges. That would be a security risk.

But the proposed patch does not do anything about that. I guess issue description is misleading a bit

2.

Add a hook_requirements() that will throw an warning when the extension is not installed and/or not created. In D10 the warning will be replaced by an error.

The proposed patch checks if the extension is installed (afaik), but says it's not created. I believe it also could be misleading. Either update the message to include all the options (not created, not installed or dropped) OR add more sophisticated checks (if possible) to clearly state what's wrong.

3. Test coverage is missing, but not sure what can be done here. Maybe unit with mocking the DB queries?

@daffie:

#12.2 As far as I can tell, there is also https://www.postgresql.org/docs/current/view-pg-available-extensions.html, so it should be possible to detect whether the extension is available for loading or not => giving us the ability to set a very detailed warning/error message. What do you think?

+++ b/core/modules/system/system.install
@@ -460,6 +460,28 @@ function system_requirements($phase) {
+      'title' => t('The pg_trgm extension'),

I'd rather say t('PostgreSQL pg_trgm database extension')

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -1060,6 +1060,40 @@ protected function hashBase64($data) {
+  /**
+   * Determines whether the PostgreSQL extension is created.
+   *
+   * @param string $name
+   *   The name of the extension.
+   *
+   * @return bool
+   *   Return TRUE when the extension is created, FALSE otherwise.
+   *
+   * @internal
+   */
+  public function extensionExists($name): bool {
+    return (bool) $this->connection->query('SELECT installed_version FROM pg_available_extensions WHERE name = :name', [
+      ':name' => $name,
+    ])->fetchField();
+  }
+
+  /**
+   * Determines whether the PostgreSQL extension is available.
+   *
+   * @param string $name
+   *   The name of the extension.
+   *
+   * @return bool
+   *   Return TRUE when the extension is available, FALSE otherwise.
+   *
+   * @internal
+   */
+  public function extensionAvailable($name): bool {
+    return (bool) $this->connection->query('SELECT name FROM pg_available_extensions WHERE name = :name', [
+      ':name' => $name,
+    ])->fetchField();

two methods here seem a bit overkill, I think it'd be better have one select and return an array of [name, installed_version], then do the processing in calling code.

There's new major release is out so would be great to check support for the extension in 14.0

Seems sensible to me, we warn the user something is wrong, point to the solution, but not fall in trying to provide all steps to solve, which would be overdoing IMO. +1 for RTBC, but needs someone to confirm the message works by testing manually.

Patch looks good and works, identifying that the pg_trgm is not enabled. I feel the wording could be simplified and clarified to direct the user to why pg_trgm is recommended (and will be required).

Current wording for the warning description is:

The pg_trgm extension is not created. This extension is only needed on a PostgreSQL database. Extensions on PostgreSQL can only be created by a database user with superuser privileges. The current warning will be replaced by an error before the release of Drupal 10.0.0.

I propose:

The pg_trgm PostgreSQL extension is not present, which may impact Drupal's performance when using PostgreSQL DB. pg_trgm will be required by Drupal 10. See Drupal database server requirements for more information.

Reasoning behind this rewriting:

• Tell the user why pg_trgm is important enough to recommend/require: performance.
• Simplify the version string from 10.0.0 to 10 - the two extra zeroes don't add anything (maths joke?)
• Don't try to document PostgreSQL details here; link to Drupal's official docs instead; that page is the correct place to point people to PostgreSQL guidance on creating extensions.
• Two links to "create" felt repetitive, and the first one I felt like the word "not" should be part of the link?

The linked DB docs don't currently include the linkage to creating extensions under PostgreSQL, but I feel that's the right place to move those links to.

Testing

I created a fresh Drupal 9.3.x environment with the patch from #18 and a PostgreSQL DB (10.12) on Lando.

~/Projects/drupal-core/.lando.yml:

name: drupal-core
recipe: drupal9
env:
  DRUSH_OPTIONS_URI: http://drupal-core.lndo.site
config:
  database: postgres
  webroot: .

I also required drush/drush.

I installed Drupal on PostgreSQL.

lando drush site-install standard --db-url=pgsql://postgres@database/drupal9

I checked the requirements warning in CLI, where it appears correctly. (screenshot of CLI output, abbreviated below)

$ lando drush core:requirements
+------------------------------+----------+---------------+
| Title                        | Severity | Summary       |
+------------------------------+----------+---------------+
| Database system              | Info     | PostgreSQL    |
|                              |          |               |
| Database system version      | Info     | 10.12         |
|                              |          |               |
| Drupal                       | Info     | 9.3.0-dev     |
|                              |          |               |
| PostgreSQL pg_trgm extension | Warning  | Not created   |
+------------------------------+----------+---------------+

I checked the web UI for the warning also (screenshot of warning in browser). NB: I've removed an unrelated Drupal warning from this image for clarity.

I created the extension:

$ lando drush sqlc
drupal9=# create extension pg_trgm;

The warning was no longer displayed once the extension was created. I then dropped the extension, and saw that the warning was once more visible.

The functionality is great, setting NW for proposed wording changes. Thanks everyone!

Tested with PostgreSQL 13 also, success also.

My recollection from #2988018: [PP-1] Performance issues with path alias generated queries on PostgreSQL was that there was index configuration / regeneration required after the pg_trgm extension was enabled. Is the scope of this issue solely to ensure that required extension is available to the DB engine?

We will not be using the extension in D9

Is there any reason why, IFF the extension is detected as enabled, that an option to create the path_alias indexes wouldn't be provided for Drupal 9 ?

If the requirements test for "is the pg_trm extension enabled?" succeeds, then the additional test "are the pg_trm indexes present on the path_alias table?" could be executed, with a warning and a link to generate them if the answer is "no".

Understood. My suggestions could be follow-ups in any case -- there's no reason why they should delay the current issue.

Just a nit on language:

+++ b/core/modules/system/system.install
@@ -472,6 +472,19 @@ function system_requirements($phase) {
+      'description' => t('The <a href=":pg_trgm">pg_trgm</a> PostgreSQL extension is not present. We will be using the extension to improve Drupal\'s performance when using PostgreSQL in Drupal 10. The extension be required by Drupal 10. See <a href=":requirements">Drupal database server requirements</a> for more information.', [

We will be using the extension to improve Drupal\'s performance when using PostgreSQL in Drupal 10. The extension be required by Drupal 10.

seems duplicate, I suggest

The extension will be required by Drupal 10 to improve Drupal performance when using PostgreSQL.

Manually tested in #21.

+++ b/core/modules/system/system.install
@@ -472,6 +472,19 @@ function system_requirements($phase) {
+  // Test with PostgreSQL databases for the status of the pg_trgm extension.
+  if (Database::isActiveConnection() && (Database::getConnection()->driver() == 'pgsql') && !Database::getConnection()->schema()->extensionExists('pg_trgm')) {
+    $requirements['pgsql_extension_pg_trgm'] = [
+      'severity' => REQUIREMENT_WARNING,
+      'title' => t('PostgreSQL pg_trgm extension'),
+      'value' => t('Not created'),
+      'description' => t('The <a href=":pg_trgm">pg_trgm</a> PostgreSQL extension is not present. The extension will be required by Drupal 10 to improve Drupal performance when using PostgreSQL. See <a href=":requirements">Drupal database server requirements</a> for more information.', [
+        ':pg_trgm' => 'https://www.postgresql.org/docs/current/pgtrgm.html',
+        ':requirements' => 'https://www.drupal.org/docs/system-requirements/database-server-requirements',
+      ]),
+    ];
+  }
+

This can have tests on postgres ie. we can test for an extension we know to exist and one we are sure does not and test the return values.

Also I think this warning should only occur on new installs and at runtime. I.e. I don't think we should issue this warning on an update at the moment because if someone is doing a security update I don't think that they need to be thinking about this warning.

Creditting @Mixologic as per issue summary.

Reading the postgres docs are we sure about the whole super user requirement thing?

The docs say:

This module is considered “trusted”, that is, it can be installed by non-superusers who have CREATE privilege on the current database.

The docs changed for postrges 13 and 14 to add that line. I'm pretty sure for postgres 13 and 14 we can install the extension automatically so I think this change should account for that.

+++ b/core/modules/system/system.install
@@ -472,6 +472,19 @@ function system_requirements($phase) {
+      'description' => t('The <a href=":pg_trgm">pg_trgm</a> PostgreSQL extension is not present. The extension will be required by Drupal 10 to improve Drupal performance when using PostgreSQL. See <a href=":requirements">Drupal database server requirements</a> for more information.', [

A tiny nit but there are 3 "Drupal"s in this paragraph, I think we can change the middle sentence to just say "The extension will be required by Drupal 10 to improve performance when using PostgreSQL."

@daffie re tests...

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -1060,6 +1060,23 @@ protected function hashBase64($data) {
+  public function extensionExists($name): bool {
+    return (bool) $this->connection->query('SELECT installed_version FROM pg_available_extensions WHERE name = :name', [
+      ':name' => $name,
+    ])->fetchField();
+  }

This must be testable on the CI supported version of PostgreSQL.

+++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Schema.php
@@ -1060,6 +1060,23 @@ protected function hashBase64($data) {
+    return (bool) $this->connection->query('SELECT installed_version FROM pg_available_extensions WHERE name = :name', [

+++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
@@ -1347,4 +1347,28 @@ public function testDefaultAfterAlter() {
+    // Make sure that the extension is created.
+    $this->connection->query('CREATE EXTENSION IF NOT EXISTS "pg_trgm"');
+    $this->assertTrue($this->schema->extensionExists('pg_trgm'));
+
+    // Drop the extension.
+    $this->connection->query('DROP EXTENSION "pg_trgm"');
+    $this->assertFalse($this->schema->extensionExists('pg_trgm'));
+
+    // Make sure that the extension is created as other tests may rely on the
+    // extension.
+    $this->connection->query('CREATE EXTENSION IF NOT EXISTS "pg_trgm"');

pg_trgm is installed on DrupalCI right? So we only need to test that it is installed. Going by my local psql you could probably test that plpgsql is installed.

Imo this test shouldn't be installing an extensions but it is perfectly acceptable to assume that an extension is installed.

+++ b/core/modules/system/system.install
@@ -472,6 +472,19 @@ function system_requirements($phase) {
+  // Test with PostgreSQL databases for the status of the pg_trgm extension.
+  if (Database::isActiveConnection() && (Database::getConnection()->driver() == 'pgsql') && !Database::getConnection()->schema()->extensionExists('pg_trgm')) {
+    $requirements['pgsql_extension_pg_trgm'] = [
+      'severity' => REQUIREMENT_WARNING,
+      'title' => t('PostgreSQL pg_trgm extension'),
+      'value' => t('Not created'),
+      'description' => t('The <a href=":pg_trgm">pg_trgm</a> PostgreSQL extension is not present. The extension will be required by Drupal 10 to improve performance when using PostgreSQL. See <a href=":requirements">Drupal database server requirements</a> for more information.', [
+        ':pg_trgm' => 'https://www.postgresql.org/docs/current/pgtrgm.html',
+        ':requirements' => 'https://www.drupal.org/docs/system-requirements/database-server-requirements',
+      ]),
+    ];
+  }

So the prefix deprecation is also being checked on update. I think this is wrong. We should only be making deprecation checks at runtime. They shouldn't interrupt an install or an update at this point. So I think this should be in a if ($phase === 'runtime') { and we should open an issue to make the prefix deprecation only occur at runtime.

Agreed with #42 - this should be a runtime warning in Drupal 9, and an upgrade error for Drupal 10.

+++ b/core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php
@@ -1347,4 +1347,20 @@ public function testDefaultAfterAlter() {
+    $this->connection->query('CREATE EXTENSION IF NOT EXISTS "pg_trgm"');

I do not think we should be doing this. The effect of this would go beyond the scope of the test if the extension was not created previously. Per #40, let's just test the extension is there. In DrupalCI is OK, and running the tests locally from Drupal 9.3 onwards will simply point the testers to create the extension in their environment.

AFAICS, all feedback addressed. Back to RTBC.

Committed 84e8b97 and pushed to 9.4.x. Thanks! Also cherry-picked to 9.3.x.

? Status change did not work :(

Opened #3249178: Add a requirements warning in Drupal 9 when PostgreSQL is used and the pg_trgm extension is not created for upgrade status too.

For using Lando, it will resolve issues with Drupal 10 and postgreSQL

lando psql

\c drupal10

create extension pg_trgm;