Make PDO connection options configurable

Setting to CNW. The patch removes some important documentation regarding WHY our default mode is what it is.

Also, I think the default.settings.php documentation needs to have a better warning that normal users should not monkey with the defaults.

-    // Force MySQL's behavior to conform more closely to SQL standards.
-    // This allows Drupal to run almost seamlessly on many different
-    // kinds of database systems. These settings force MySQL to behave
-    // the same as postgresql, or sqlite in regards to syntax interpretation
-    // and invalid data handling. See http://drupal.org/node/344575 for
-    // further discussion. Also, as MySQL 5.5 changed the meaning of
-    // TRADITIONAL we need to spell out the modes one by one.

#8: configurable_sql_mode-1309278-8.patch queued for re-testing.

Touching the SQL MODE is fine.

But before touching charsets or collations, you need to read all nitty-gritty details in #772678: Database default collation is not respected

Update description to reflect bug in PDO

Closed #1308874: Unable to override sql_mode as a duplicate. First get this done, then backport (maybe).

As discussed in IRC I am in favor of having init_commands instead of extra properties for every connection setting I'll ever need. SQL_BIG_SELECTS just to name one other.

However I don't think we should include charsets, collations and what not there. Drupal uses UTF-8.

I would much rather go minimalist here. We spent probably 80 hours or more between a half dozen people trying to figure out the right magic incantation of configuration options to make MySQL behave like a standard SQL database rather than a 1997-era card catalog that tries to be smarter than you (aka MySQL 3). I am extremely reluctant to let people break that who don't know what they're doing (which is pretty much everyone, including me).

Changing the character encoding is asking for trouble, as you can't change the encoding of the HTTP traffic. Try to write to the database in a different character set than the data coming from the browser and you're in for a world of pain.

There is an "out" now for really edge cases: Subclass the MySQL driver to your own custom driver and change the settings there. Works great. :-)

So I'm not entirely convinced the benefit here is worth the risk of toppling the house of cards that is MySQL.

I am more comfortable with #22, although still skeptical. I'd like to get David Strauss to weigh in here.

@CrellI I think there are N ways to configure Drupal from settings.php in a way that it breaks rather subtly. Also in the light of #1314214: MySQL driver does not support full UTF-8 (emojis, asian symbols, mathematical symbols) allowing overriding SET NAMES is a good idea, isn't it?

I have a couple questions about the use cases supporting the inclusion of this patch:

* Is it possible to achieve the same changes by updating my.cnf? Because this patch is to support a <1% use case, a solution that involves re-configuring MySQL is acceptable.
* Is it possible to just do this by sub-classing the MySQL driver? Again, for <1% use cases, heavy solutions are okay.

Most of the changelog excerpts and examples are not terribly convincing for why Drupal should offer yet another configuration option, especially in the form of advertising it in default.settings.php. Pretty much all of the excepts and examples either don't substantially affect Drupal projects of any size (like issues around stored procedures), are about accommodating bugs in fairly obscure systems (like tungsten-replicator), or are only encountered outside Drupal's DB interface (like running INFILE from outside Drupal).

I'm with basic here: it is really useful in the special cases to be able to modify connection options and initialization commands. Of course, you can *always* subclass the driver, but doing so will break a lot of things that actually rely on the driver name (especially: Drush).

I think we should extend this concept a little bit:

• Extend it to all the database engines in Core
• Allow PDO driver options to be configurable (this is especially useful for PDO::ATTR_TIMEOUT which controls the amount of time the driver will wait for a connection to be established, and is awfully long by default - 30s)

@Damien Tournoud's suggestions make sense to me.

However, let's keep the charset issue separated in #1314214: MySQL driver does not support full UTF-8 (emojis, asian symbols, mathematical symbols), please.

I was thinking both init commands and PDO driver options. Of course, both of those have different meanings on different database engines, but both are useful.

I'm more okay with this change if equivalent functionality is available for other engines.

On the issue of not being able to use a subclassed driver, that sounds like a bug. Code should be checking whether something is an instance of the type they need, not whether the class names matches a string.

Approved here. This gives some much needed flexibility in tuning those parameters, which is especially important in large-scale deployments.

And I would like to see this backported to 7.x as well.

Looks good to me as well, and safe for backporting.

Assigning this to Crell for a final glance over if he's got time. I'm happy committing this myself though.

Fixing tag.

Also confirmed with Crell on IRC that he'd like one last look at this before committing. Bumping back down to needs review.

Ugh. Sorry for taking so long to respond here, everyone. I'm more comfortable with the patch in #41 (for both D7 and D8), with the following caveat:

+++ b/sites/default/default.settings.php
@@ -153,6 +153,21 @@
+ * Advanced users can add/override arbitrary init commands and pdo settings.
+ * @see DatabaseConnection_mysql::__construct
+ * @see DatabaseConnection_pgsql::__construct
+ * @see DatabaseConnection_sqlite::__construct
+ * @code
+ * $databases['default']['default'] = array(
+ *   'init_commands' => array(
+ *     'big_selects' => 'SET SQL_BIG_SELECTS=1',
+ *   ),
+ *   'pdo' => array(
+ *     PDO::ATTR_TIMEOUT => 5,
+ *   ),
+ * );
+ * @endcode

While this is true, I believe it should have a big warning on it that the default settings are designed for database portability and changing them may cause unexpected behavior, including potentially data loss. This is NOT something that anyone should be playing with without really knowing what they're doing.

Let's improve the documentation there to warn people away. After that, I'm comfortable committing this.

#47 looks fine to me. Thanks!

The last sentence in the warning is pure nanny talk IMO. Otherwise, nice patch.

Here's a reroll removing the last sentence and moving to the new directory structure.

OK, let's do. I'm OK with backporting this as well as there is no API change.

And the D7 port. Only the paths differ.

Looks good to me. Committed/pushed to 8.x.

Moving to 7.x - it'll need a patch with no suffix for the testbot to have a crack at it.

Yay! The 7.x patch again.

Lol, no, it was pretty straight forward - looks like I must have accidantely started from your D7 backport :)

EXCELLENT. This type of backwards-compatible capability enhancement is super exciting to see, and I know Rudy's been working on it for quite awhile. Seems to have buy-in from the DBTNG maintainers, and shouldn't break anything in existing installations. In case it does, we have a good month or so to find out.

Committed and pushed to 7.x! Thanks!!

Hi Folks,

Forgive me if I'm wrong, but one of the goals of this change is to allow the charset to be configurable in order to allow the following bugs to be fixed:
#1314214: MySQL driver does not support full UTF-8 (emojis, asian symbols, mathematical symbols)
#1199736: Error on pasting in different language

We need to able able to make the "SET NAMES" command configurable between "utf8" and "utf8mb4"

I think we decided to leave the charset out of this, see #15, #19, #30. Not sure, though ...