Force connection with PDO::CASE_LOWER

I'm not entirely sure I follow. SQL is supposed to be case-insensitive, per the spec. MySQL does it a bit wrong and is case-sensitive for table names if and only if it's running on a case-sensitive file system. By just always using lowercase table and field names in all code, everything just works fine.

So what do we need here? Remember we could be dealing with foreign databases as well, which may not be as controlled as Drupal database.

I think this looks reasonable. Not Crell?

If I'm understanding the issue correctly and it just does case-folding on fetch, the only place we'd still have an issue is if we run into a 3rd party database table with columns foo and Foo, which would then get corrupted when loading. If we're OK with that limitation (I am, since making a DB like that is a moronic idea to begin with) then I'm OK with this patch.

We should make sure to document the case handling in the handbook, too.

OK, here's my statement on it. +1 to forcing lower-case, but let's do it in a way that allows it to be over-ridden by a specific driver. That way, if someone really needs to connect to a 3rd party database where they do care about case they can clone their own custom driver and override the case handling, then connect to the external database with that driver instead.

That should be an easy change here and give us the most flexibility.

Committed to CVS HEAD.

This broke sqlite support. Note that the $connection_options array is no longer passed to parent::__construct().

-    parent::__construct('sqlite:'. $connection_options['database'], '', '', $connection_options);
+    parent::__construct('sqlite:'. $connection_options['database'], '', '', array(
+      // Force column names to lower case.
+      PDO::ATTR_CASE => PDO::CASE_LOWER,
+    ));

In #7, hswong3i said "I am not so sure why SQLite looks a bit different from MySQL and PostgreSQL, but I clone the handling for it. Please correct me if that's not work :S". Well, it was right, he wasn't sure.

Dries, could you give us more time to review those patches more carefully from now on?

Dave fix is sane.

That would make sense to do, I'm testing it right now.

In response to #14, it will fail for PostgreSQL. I literally had to code the following to get it to work in the pgsql driver:

    $connection_options += array(
      'transactions' => TRUE,
      // Convert numeric values to strings when fetching.
      PDO::ATTR_STRINGIFY_FETCHES => TRUE,
      // Force column names to lower case.
      PDO::ATTR_CASE => PDO::CASE_LOWER,
    );
    $this->transactionSupport = $connection_options['transactions'];

    $dsn = 'pgsql:host=' . $connection_options['host'] . ' dbname=' . $connection_options['database'];
    if (!empty($connection_options['port'])) {
      $dsn .= ' port=' . $connection_options['port'];
    }

    $options = $connection_options;
    unset($options['transactions']);
    unset($options['host']);
    unset($options['database']);
    unset($options['port']);
    unset($options['username']);
    unset($options['password']);
    unset($options['driver']);

    parent::__construct($dsn, $connection_options['username'], $connection_options['password'], $options);

We should just bring the sqlite driver in line with the mysql/postgresql driver. This also brings the default port for postgresql in line with the mysql driver.

Fair enough.

The confusion comes from the fact that: $connection_options (our Drupal-specific options) != $driver_options (PDO driver options).

Nevermind...#17 will break postgresql. Revised patch that just fixes the sqlite driver and brings it in line with the mysql and pgsql drivers' __construct functions.

Another quick rewrite. Standardizing the drivers __construct function will be a separate issue. This just simply fixes the HEAD sqlite breakage.

Ok, this is the good one, thanks Dave.

How about we focus on the patch in #20 to fix the HEAD broken-ness and create a separate issue for synchronizing the __contstruct functions

Committed to CVS HEAD. Thanks, and sorry if I pulled the trigger on this one too fast.

Ok, let's do this properly, then.

Fixed spelling.

@hswong3i: you made the mess here, let's fix it here.

It is a good clean up, indeed. Committed to CVS HEAD.

This change broke third-party integration for no good reason: #1171866: Enforced fetching of fields/columns in lowercase breaks third-party integration

Forcing lowercase field names completely breaks connections to any other data... can this at least be optional?

Please see the issue referenced by Sun for follow up.