MDL-55124 database: Allow option to not send options.

pgbouncer doesn't accept the PostgreSQL options command as it can't process it. If you are using pgBouncer you need to make sure your database is configured to set client encoding and standard_conforming_strings correctly on each connection. pgBouncer can do this, as can ALTER commands. Also using sockets without a filename did not allow different ports to work. This is because you either specify a filename which includes a port or you just specify a port and libpq works out the correct socket location.
mpetrowi · Sep 20, 2016 · f834a8f · f834a8f
1 parent 1f27448
commit f834a8f
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 12 deletions.
diff --git a/config-dist.php b/config-dist.php
@@ -59,6 +59,10 @@
     'dbport'    => '',          // the TCP port number to use when connecting
                                 //  to the server. keep empty string for the
                                 //  default port
+    'dbhandlesoptions' => false,// On PostgreSQL poolers like pgbouncer don't
+                                // support advanced options on connection.
+                                // If you set those in the database then
+                                // the advanced settings will not be sent.
 );
 
 

diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php
@@ -134,11 +134,12 @@ public function connect($dbhost, $dbuser, $dbpass, $dbname, $prefix, array $dbop
         if (!empty($this->dboptions['dbsocket']) and ($this->dbhost === 'localhost' or $this->dbhost === '127.0.0.1')) {
             $connection = "user='$this->dbuser' password='$pass' dbname='$this->dbname'";
             if (strpos($this->dboptions['dbsocket'], '/') !== false) {
-                $connection = $connection." host='".$this->dboptions['dbsocket']."'";
-                if (!empty($this->dboptions['dbport'])) {
-                    // Somehow non-standard port is important for sockets - see MDL-44862.
-                    $connection = $connection." port ='".$this->dboptions['dbport']."'";
-                }
+                // A directory was specified as the socket location.
+                $connection .= " host='".$this->dboptions['dbsocket']."'";
+            }
+            if (!empty($this->dboptions['dbport'])) {
+                // A port as specified, add it to the connection as it's used as part of the socket path.
+                $connection .= " port ='".$this->dboptions['dbport']."'";
             }
         } else {
             $this->dboptions['dbsocket'] = '';
@@ -153,14 +154,29 @@ public function connect($dbhost, $dbuser, $dbpass, $dbname, $prefix, array $dbop
             $connection = "host='$this->dbhost' $port user='$this->dbuser' password='$pass' dbname='$this->dbname'";
         }
 
-        // ALTER USER and ALTER DATABASE are overridden by these settings.
-        $options = array('--client_encoding=utf8', '--standard_conforming_strings=on');
-        // Select schema if specified, otherwise the first one wins.
-        if (!empty($this->dboptions['dbschema'])) {
-            $options[] = "-c search_path=" . addcslashes($this->dboptions['dbschema'], "'\\");
-        }
+        if (empty($this->dboptions['dbhandlesoptions'])) {
+            // ALTER USER and ALTER DATABASE are overridden by these settings.
+            $options = array('--client_encoding=utf8', '--standard_conforming_strings=on');
+            // Select schema if specified, otherwise the first one wins.
+            if (!empty($this->dboptions['dbschema'])) {
+                $options[] = "-c search_path=" . addcslashes($this->dboptions['dbschema'], "'\\");
+            }
 
-        $connection .= " options='".implode(' ', $options)."'";
+            $connection .= " options='" . implode(' ', $options) . "'";
+        } else {
+            /* We don't trust people who just set the dbhandlesoptions, this code checks up on them.
+             * These functions do not talk to the server, they use the client library knowledge to determine state.
+             */
+            if (!empty($this->dboptions['dbschema'])) {
+                throw new dml_connection_exception('You cannot specify a schema with dbhandlesoptions, use the database to set it.');
+            }
+            if (pg_client_encoding($this->pgsql) != 'UTF8') {
+                throw new dml_connection_exception('client_encoding = UTF8 not set, it is: ' . pg_client_encoding($this->pgsql));
+            }
+            if (pg_escape_string($this->pgsql, '\\') != '\\') {
+                throw new dml_connection_exception('standard_conforming_string = on, must be set at the database.');
+            }
+        }
 
         ob_start();
         if (empty($this->dboptions['dbpersist'])) {

diff --git a/lib/upgrade.txt b/lib/upgrade.txt
@@ -78,6 +78,13 @@ information provided here is intended especially for developers.
   - $CFG->wwwroot: http://example.com/moodle
   - $CFG->alternateloginurl : /my/super/login.php
   - Login url will be: http://example.com/moodle/my/super/login.php (moodle root based)
+* PostgreSQL connections now use advanced options to reduce connection overhead.  These options are not compatible
+  with some connection poolers.  The dbhandlesoptions parameter has been added to allow the database to configure the
+  required defaults. The parameters that are required in the database are;
+    ALTER DATABASE moodle SET client_encoding = UTF8;
+    ALTER DATABASE moodle SET standard_conforming_strings = on;
+    ALTER DATABASE moodle SET search_path = 'moodle,public';  -- If you wish to use a default schema.
+  You can set these options against the database or the moodle user who connects.
 
 === 3.1 ===