From ff9b00981e6526b75a9884b1d889eb0d3e2d0347 Mon Sep 17 00:00:00 2001 From: sun Date: Fri, 8 Aug 2014 21:53:28 +0200 Subject: [PATCH 1/8] Temporarily use SQLite in Database KTB tests to expose errors. --- .../system/src/Tests/Database/DatabaseTestBase.php | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/core/modules/system/src/Tests/Database/DatabaseTestBase.php b/core/modules/system/src/Tests/Database/DatabaseTestBase.php index da14f5c..e3952e5 100644 --- a/core/modules/system/src/Tests/Database/DatabaseTestBase.php +++ b/core/modules/system/src/Tests/Database/DatabaseTestBase.php @@ -7,6 +7,7 @@ namespace Drupal\system\Tests\Database; +use Drupal\Core\Database\Database; use Drupal\simpletest\DrupalUnitTestBase; /** @@ -19,8 +20,29 @@ public static $modules = array('database_test'); + /** + * @var string + */ + protected $sqliteDb; + function setUp() { parent::setUp(); + + Database::removeConnection('default'); + $this->container->set('database', NULL); + $this->sqliteDb = tempnam(sys_get_temp_dir(), 'test-sqlite'); + Database::addConnectionInfo('default', 'default', array( + 'driver' => 'sqlite', + 'namespace' => 'Drupal\Core\Database\Driver\sqlite', + 'host' => '', + 'database' => $this->sqliteDb, + 'username' => '', + 'password' => '', + 'prefix' => array( + 'default' => '', + ), + )); + $this->installSchema('database_test', array( 'test', 'test_people', @@ -34,6 +56,12 @@ function setUp() { self::addSampleData(); } + protected function tearDown() { + parent::tearDown(); + @unlink($this->sqliteDb); + unset($this->sqliteDb); + } + /** * Sets up tables for NULL handling. */ -- 1.7.11.msysgit.1 From 8fdbc06d9676d2fe97bbb2d7969b560ab88b5a38 Mon Sep 17 00:00:00 2001 From: sun Date: Tue, 5 Aug 2014 17:25:20 +0200 Subject: [PATCH 2/8] Issue #2318191 by sun: Fixed SQLite: StatementPrefetch is dead code due to logic error. --- .../Core/Database/Driver/sqlite/Connection.php | 71 +++++++++++++++++++++- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php index 6cacf4e..15b16a2 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php @@ -68,9 +68,6 @@ class Connection extends DatabaseConnection { public function __construct(\PDO $connection, array $connection_options) { parent::__construct($connection, $connection_options); - // We don't need a specific PDOStatement class here, we simulate it below. - $this->statementClass = NULL; - // This driver defaults to transaction support, except if explicitly passed FALSE. $this->transactionSupport = $this->transactionalDDLSupport = !isset($connection_options['transactions']) || $connection_options['transactions'] !== FALSE; @@ -244,6 +241,74 @@ public static function sqlFunctionRegexp($string, $pattern) { } /** + * {@inheritdoc} + */ + protected function expandArguments(&$query, &$args) { + $modified = parent::expandArguments($query, $args); + + // The PDO SQLite driver always replaces placeholders with strings, which + // breaks numeric expressions (e.g., COUNT(*) >= :count). Replace numeric + // placeholders in the query to work around this bug. + // @see http://bugs.php.net/bug.php?id=45259 + if (empty($args)) { + return $modified; + } + // Check if $args is a simple numeric array. + if (range(0, count($args) - 1) === array_keys($args)) { + // In that case, we have unnamed placeholders. + $count = 0; + $new_args = array(); + foreach ($args as $value) { + if (is_float($value) || is_int($value) || is_numeric($value)) { + if (is_float($value)) { + // Force the conversion to float so as not to loose precision + // in the automatic cast. + $value = sprintf('%F', $value); + } + $query = substr_replace($query, $value, strpos($query, '?'), 1); + } + else { + $placeholder = ':db_statement_placeholder_' . $count++; + $query = substr_replace($query, $placeholder, strpos($query, '?'), 1); + $new_args[$placeholder] = $value; + } + } + $args = $new_args; + $modified = TRUE; + } + // Otherwise this is using named placeholders. + else { + foreach ($args as $placeholder => $value) { + if (is_float($value) || is_int($value) || is_numeric($value)) { + if (is_float($value)) { + // Force the conversion to float so as not to loose precision + // in the automatic cast. + $value = sprintf('%F', $value); + } + + // We will remove this placeholder from the query as PDO throws an + // exception if the number of placeholders in the query and the + // arguments does not match. + unset($args[$placeholder]); + // PDO allows placeholders to not be prefixed by a colon. See + // http://marc.info/?l=php-internals&m=111234321827149&w=2 for + // more. + if ($placeholder[0] != ':') { + $placeholder = ":$placeholder"; + } + // When replacing the placeholders, make sure we search for the + // exact placeholder. For example, if searching for + // ':db_placeholder_1', do not replace ':db_placeholder_11'. + $query = preg_replace('/' . preg_quote($placeholder, '/') . '\b/', $value, $query); + + $modified = TRUE; + } + } + } + return $modified; + } + + /** * SQLite-specific implementation of DatabaseConnection::prepare(). * * We don't use prepared statements at all at this stage. We just create -- 1.7.11.msysgit.1 From 2dca848b6f3adf94f2a573bdb93761ca8f49aeec Mon Sep 17 00:00:00 2001 From: sun Date: Wed, 6 Aug 2014 01:37:10 +0200 Subject: [PATCH 3/8] Issue #2318191 by sun: Fixed SQLite Insert query does not account for INSERT FROM ... SELECT. --- core/lib/Drupal/Core/Database/Driver/sqlite/Insert.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Insert.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Insert.php index 3a0bcaa..8882fcd 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Insert.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Insert.php @@ -22,7 +22,7 @@ public function execute() { if (!$this->preExecute()) { return NULL; } - if (count($this->insertFields)) { + if (count($this->insertFields) || !empty($this->fromQuery)) { return parent::execute(); } else { @@ -35,7 +35,10 @@ public function __toString() { $comments = $this->connection->makeComment($this->comments); // Produce as many generic placeholders as necessary. - $placeholders = array_fill(0, count($this->insertFields), '?'); + $placeholders = array(); + if (!empty($this->insertFields)) { + $placeholders = array_fill(0, count($this->insertFields), '?'); + } // If we're selecting from a SelectQuery, finish building the query and // pass it back, as any remaining options are irrelevant. -- 1.7.11.msysgit.1 From 9a58f5aea748a356eca41cb41822e60fb611cd63 Mon Sep 17 00:00:00 2001 From: sun Date: Wed, 6 Aug 2014 01:35:32 +0200 Subject: [PATCH 4/8] Issue #2318191 by sun: Fixed SQLite: Wrong parameter order in REGEXP user function. --- core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php index 15b16a2..d0e87a3 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php @@ -234,10 +234,16 @@ public static function sqlFunctionRand($seed = NULL) { /** * SQLite compatibility implementation for the REGEXP SQL operator. * - * The REGEXP operator is a special syntax for the regexp() user function. + * The REGEXP operator is natively known, but not implemented by default. + * + * @see http://www.sqlite.org/lang_expr.html#regexp */ - public static function sqlFunctionRegexp($string, $pattern) { - return preg_match('#' . str_replace('#', '\#', $pattern) . '#i', $string); + public static function sqlFunctionRegexp($pattern, $subject) { + // preg_quote() cannot be used here, since $pattern may contain reserved + // regular expression characters already (such as ^, $, etc). Therefore, + // use a rare character as PCRE delimiter. + $pattern = '#' . addcslashes($pattern, '#') . '#i'; + return preg_match($pattern, $subject); } /** -- 1.7.11.msysgit.1 From 1a360e999021532d2fe880a437e20627697ffb96 Mon Sep 17 00:00:00 2001 From: sun Date: Sun, 3 Aug 2014 22:08:36 +0200 Subject: [PATCH 5/8] Issue #2318191 by sun: Fixed SQLite does not interpret '\_' as string literal in LIKE without ESCAPE. --- core/lib/Drupal/Core/Config/DatabaseStorage.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Config/DatabaseStorage.php b/core/lib/Drupal/Core/Config/DatabaseStorage.php index 20e5f1b..57293e5 100644 --- a/core/lib/Drupal/Core/Config/DatabaseStorage.php +++ b/core/lib/Drupal/Core/Config/DatabaseStorage.php @@ -269,7 +269,7 @@ public function decode($raw) { */ public function listAll($prefix = '') { try { - return $this->connection->query('SELECT name FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name LIKE :name', array( + return $this->connection->query('SELECT name FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name LIKE :name ESCAPE \'\\\'', array( ':collection' => $this->collection, ':name' => $this->connection->escapeLike($prefix) . '%', ), $this->options)->fetchCol(); -- 1.7.11.msysgit.1 From 5e08ae1290c906af1ecdcabb95679bfe5a0f6395 Mon Sep 17 00:00:00 2001 From: sun Date: Mon, 4 Aug 2014 21:15:11 +0200 Subject: [PATCH 6/8] Issue #2318191 by sun: Fixed SQLite does not support CONCAT_WS(). --- .../Core/Database/Driver/sqlite/Connection.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php index d0e87a3..f8d1a90 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php @@ -119,6 +119,7 @@ public static function open(array &$connection_options = array()) { $pdo->sqliteCreateFunction('length', 'strlen', 1); $pdo->sqliteCreateFunction('md5', 'md5', 1); $pdo->sqliteCreateFunction('concat', array(__CLASS__, 'sqlFunctionConcat')); + $pdo->sqliteCreateFunction('concat_ws', array(__CLASS__, 'sqlFunctionConcatWs')); $pdo->sqliteCreateFunction('substring', array(__CLASS__, 'sqlFunctionSubstring'), 3); $pdo->sqliteCreateFunction('substring_index', array(__CLASS__, 'sqlFunctionSubstringIndex'), 3); $pdo->sqliteCreateFunction('rand', array(__CLASS__, 'sqlFunctionRand')); @@ -197,6 +198,25 @@ public static function sqlFunctionConcat() { } /** + * SQLite compatibility implementation for the CONCAT_WS() SQL function. + * + * @see http://dev.mysql.com/doc/refman/5.6/en/string-functions.html#function_concat-ws + */ + public static function sqlFunctionConcatWs() { + $args = func_get_args(); + $separator = array_shift($args); + // If the separator is NULL, the result is NULL. + if ($separator === FALSE || is_null($separator)) { + return NULL; + } + // Skip any NULL values after the separator argument. + $args = array_filter($args, function ($value) { + return !is_null($value); + }); + return implode($separator, $args); + } + + /** * SQLite compatibility implementation for the SUBSTRING() SQL function. */ public static function sqlFunctionSubstring($string, $from, $length) { -- 1.7.11.msysgit.1 From fa774627243e451e3a76356c80d4bd46d9f76fab Mon Sep 17 00:00:00 2001 From: sun Date: Tue, 5 Aug 2014 21:23:23 +0200 Subject: [PATCH 7/8] Issue #2318191 by sun: Updated Views Date Handler for SQLite. SQLite does not support advanced date/calendar operations (such as identifying 2000-01-01 as the 52nd week of 1999). --- core/modules/views/src/Plugin/views/query/Sql.php | 15 ++++++++++++-- .../views/src/Tests/Handler/ArgumentDateTest.php | 23 +++++++++++++--------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/core/modules/views/src/Plugin/views/query/Sql.php b/core/modules/views/src/Plugin/views/query/Sql.php index 25b1a23..4296196 100644 --- a/core/modules/views/src/Plugin/views/query/Sql.php +++ b/core/modules/views/src/Plugin/views/query/Sql.php @@ -1641,7 +1641,7 @@ public function getDateField($field) { break; case 'sqlite': if (!empty($offset)) { - $field = "($field + '$offset_seconds')"; + $field = "($field + $offset_seconds)"; } break; } @@ -1755,7 +1755,18 @@ public function getDateFormat($field, $format) { 'A' => '', ); $format = strtr($format, $replace); - return "strftime('$format', $field, 'unixepoch')"; + $expression = "strftime('$format', $field, 'unixepoch')"; + // The expression yields a string, but the comparison value is an + // integer in case the comparison value is a float, integer, or numeric. + // All of the above SQLite format tokens only produce integers. However, + // the given $format may contain 'Y-m-d', which results in a string. + // @see \Drupal\Core\Database\Driver\sqlite\Connection::expandArguments() + // @see http://www.sqlite.org/lang_datefunc.html + // @see http://www.sqlite.org/lang_expr.html#castexpr + if (preg_match('/^(?:%\w)+$/', $format)) { + $expression = "CAST($expression AS NUMERIC)"; + } + return $expression; } } diff --git a/core/modules/views/src/Tests/Handler/ArgumentDateTest.php b/core/modules/views/src/Tests/Handler/ArgumentDateTest.php index 4c66e79..f6ba131 100644 --- a/core/modules/views/src/Tests/Handler/ArgumentDateTest.php +++ b/core/modules/views/src/Tests/Handler/ArgumentDateTest.php @@ -163,15 +163,20 @@ public function testWeekHandler() { ->execute(); $view = Views::getView('test_argument_date'); - $view->setDisplay('embed_3'); - // The first jan 2000 was still in the last week of the previous year. - $this->executeView($view, array(52)); - $expected = array(); - $expected[] = array('id' => 1); - $expected[] = array('id' => 2); - $expected[] = array('id' => 3); - $this->assertIdenticalResultset($view, $expected, $this->columnMap); - $view->destroy(); + + // SQLite does not support advanced date operations (such as interpreting + // 2000-01-01 as the 52nd week of 1999). + if ($this->container->get('database')->driver() !== 'sqlite') { + $view->setDisplay('embed_3'); + // The first jan 2000 was still in the last week of the previous year. + $this->executeView($view, array(52)); + $expected = array(); + $expected[] = array('id' => 1); + $expected[] = array('id' => 2); + $expected[] = array('id' => 3); + $this->assertIdenticalResultset($view, $expected, $this->columnMap); + $view->destroy(); + } $view->setDisplay('embed_3'); $this->executeView($view, array('02')); -- 1.7.11.msysgit.1 From 435742e71772fb817a38d61fc2455b701f67f6f9 Mon Sep 17 00:00:00 2001 From: sun Date: Fri, 8 Aug 2014 23:51:25 +0200 Subject: [PATCH 8/8] [fixup] Use proper Select query builder to be portable. --- core/lib/Drupal/Core/Config/DatabaseStorage.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/lib/Drupal/Core/Config/DatabaseStorage.php b/core/lib/Drupal/Core/Config/DatabaseStorage.php index 57293e5..2c512dc 100644 --- a/core/lib/Drupal/Core/Config/DatabaseStorage.php +++ b/core/lib/Drupal/Core/Config/DatabaseStorage.php @@ -269,10 +269,11 @@ public function decode($raw) { */ public function listAll($prefix = '') { try { - return $this->connection->query('SELECT name FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection AND name LIKE :name ESCAPE \'\\\'', array( - ':collection' => $this->collection, - ':name' => $this->connection->escapeLike($prefix) . '%', - ), $this->options)->fetchCol(); + $select = $this->connection->select($this->table, 'c', $this->options) + ->fields('c', array('name')) + ->condition('collection', $this->collection) + ->condition('name', $this->connection->escapeLike($prefix) . '%', 'LIKE'); + return $select->execute()->fetchCol(); } catch (\Exception $e) { return array(); -- 1.7.11.msysgit.1