From e4151572062be8894b69e77bdc9b2fb5a157cc25 Mon Sep 17 00:00:00 2001 From: Mira Paulik Date: Tue, 26 Jan 2016 09:39:34 +0100 Subject: [PATCH] SqlsrvDriver::applyLimit(): fixed limit and offset behaviour for odbc 11+ SqlsrvReflector: changed constrains metadata loading from INFORMATION_SCHEMA to sys schema to get complete list of all constraints, not PK only --- src/Dibi/Drivers/SqlsrvDriver.php | 10 +++--- src/Dibi/Drivers/SqlsrvReflector.php | 14 ++++---- tests/dibi/Connection.fetch.phpt | 4 +-- tests/dibi/Fluent.fetch.limit.mssql.phpt | 20 +++++------ tests/dibi/Fluent.fetch.phpt | 2 +- tests/dibi/Fluent.select.phpt | 1 + tests/dibi/Result.meta.phpt | 16 ++++----- tests/dibi/Sqlsrv.limits.phpt | 43 ++++++++++++++++-------- tests/dibi/Translator.phpt | 9 ++++- tests/dibi/data/sqlsrv.sql | 1 + tests/dibi/meta.phpt | 2 +- 11 files changed, 74 insertions(+), 48 deletions(-) diff --git a/src/Dibi/Drivers/SqlsrvDriver.php b/src/Dibi/Drivers/SqlsrvDriver.php index c3202b4f..26bd2296 100644 --- a/src/Dibi/Drivers/SqlsrvDriver.php +++ b/src/Dibi/Drivers/SqlsrvDriver.php @@ -311,13 +311,15 @@ class SqlsrvDriver implements Dibi\Driver, Dibi\ResultDriver throw new Dibi\NotSupportedException('Offset is not supported by this database.'); } elseif ($limit !== NULL) { - $sql = 'SELECT TOP ' . (int) $limit . ' * FROM (' . $sql . ') t'; + $sql = sprintf('SELECT TOP (%d) * FROM (%s) t', $limit, $sql); } - } elseif ($limit !== NULL || $offset) { + } elseif ($limit !== NULL) { // requires ORDER BY, see https://technet.microsoft.com/en-us/library/gg699618(v=sql.110).aspx - $sql .= ' OFFSET ' . (int) $offset . ' ROWS ' - . 'FETCH NEXT ' . (int) $limit . ' ROWS ONLY'; + $sql = sprintf('%s OFFSET %d ROWS FETCH NEXT %d ROWS ONLY', rtrim($sql), $offset, $limit); + } elseif ($offset) { + // requires ORDER BY, see https://technet.microsoft.com/en-us/library/gg699618(v=sql.110).aspx + $sql = sprintf('%s OFFSET %d ROWS', rtrim($sql), $offset); } } diff --git a/src/Dibi/Drivers/SqlsrvReflector.php b/src/Dibi/Drivers/SqlsrvReflector.php index 1daf73e2..d78f4ab3 100644 --- a/src/Dibi/Drivers/SqlsrvReflector.php +++ b/src/Dibi/Drivers/SqlsrvReflector.php @@ -105,19 +105,19 @@ class SqlsrvReflector implements Dibi\Reflector */ public function getIndexes($table) { - $keyUsagesRes = $this->driver->query("SELECT * FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE TABLE_NAME = {$this->driver->escapeText($table)}"); + $keyUsagesRes = $this->driver->query(sprintf("EXEC [sys].[sp_helpindex] @objname = N%s", $this->driver->escapeText($table))); $keyUsages = []; while ($row = $keyUsagesRes->fetch(TRUE)) { - $keyUsages[$row['CONSTRAINT_NAME']][(int) $row['ORDINAL_POSITION'] - 1] = $row['COLUMN_NAME']; + $keyUsages[$row['index_name']] = explode(',', $row['index_keys']); } - $res = $this->driver->query("SELECT * FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE TABLE_NAME = {$this->driver->escapeText($table)}"); + $res = $this->driver->query("SELECT [i].* FROM [sys].[indexes] [i] INNER JOIN [sys].[tables] [t] ON [i].[object_id] = [t].[object_id] WHERE [t].[name] = {$this->driver->escapeText($table)}"); $indexes = []; while ($row = $res->fetch(TRUE)) { - $indexes[$row['CONSTRAINT_NAME']]['name'] = $row['CONSTRAINT_NAME']; - $indexes[$row['CONSTRAINT_NAME']]['unique'] = $row['CONSTRAINT_TYPE'] === 'UNIQUE'; - $indexes[$row['CONSTRAINT_NAME']]['primary'] = $row['CONSTRAINT_TYPE'] === 'PRIMARY KEY'; - $indexes[$row['CONSTRAINT_NAME']]['columns'] = isset($keyUsages[$row['CONSTRAINT_NAME']]) ? $keyUsages[$row['CONSTRAINT_NAME']] : []; + $indexes[$row['name']]['name'] = $row['name']; + $indexes[$row['name']]['unique'] = $row['is_unique'] === 1; + $indexes[$row['name']]['primary'] = $row['is_primary_key'] === 1; + $indexes[$row['name']]['columns'] = isset($keyUsages[$row['name']]) ? $keyUsages[$row['name']] : []; } return array_values($indexes); } diff --git a/tests/dibi/Connection.fetch.phpt b/tests/dibi/Connection.fetch.phpt index ca149278..072d6b7e 100644 --- a/tests/dibi/Connection.fetch.phpt +++ b/tests/dibi/Connection.fetch.phpt @@ -14,7 +14,7 @@ $conn->loadFile(__DIR__ . "/data/$config[system].sql"); // fetch a single value -$res = $conn->query('SELECT [title] FROM [products]'); +$res = $conn->query('SELECT [title] FROM [products] ORDER BY [product_id]'); Assert::same('Chair', $res->fetchSingle()); @@ -63,7 +63,7 @@ Assert::equal([ // more complex association array function query($conn) { - return $conn->query($conn->getConfig('system') === 'odbc' ? ' + return $conn->query(in_array($conn->getConfig('system'), ['odbc', 'sqlsrv']) ? ' SELECT products.title, customers.name, orders.amount FROM ([products] INNER JOIN [orders] ON [products.product_id] = [orders.product_id]) diff --git a/tests/dibi/Fluent.fetch.limit.mssql.phpt b/tests/dibi/Fluent.fetch.limit.mssql.phpt index c0735b66..20314ee7 100644 --- a/tests/dibi/Fluent.fetch.limit.mssql.phpt +++ b/tests/dibi/Fluent.fetch.limit.mssql.phpt @@ -40,28 +40,28 @@ $fluent = $conn->select('*') ->orderBy('customer_id'); Assert::same( - reformat('SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), (string) $fluent ); $fluent->fetch(); Assert::same( - 'SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t', + 'SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t', dibi::$sql ); $fluent->fetchSingle(); Assert::same( - reformat('SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); $fluent->fetchAll(0, 3); Assert::same( - reformat('SELECT TOP 3 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (3) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); Assert::same( - reformat('SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), (string) $fluent ); @@ -69,16 +69,16 @@ Assert::same( $fluent->limit(0); $fluent->fetch(); Assert::same( - reformat('SELECT TOP 0 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (0) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); $fluent->fetchSingle(); Assert::same( - reformat('SELECT TOP 0 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (0) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); Assert::same( - reformat('SELECT TOP 0 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (0) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), (string) $fluent ); @@ -87,12 +87,12 @@ $fluent->removeClause('limit'); $fluent->removeClause('offset'); $fluent->fetch(); Assert::same( - reformat('SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); $fluent->fetchSingle(); Assert::same( - reformat('SELECT TOP 1 * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), + reformat('SELECT TOP (1) * FROM ( SELECT * FROM [customers] ORDER BY [customer_id]) t'), dibi::$sql ); Assert::same( diff --git a/tests/dibi/Fluent.fetch.phpt b/tests/dibi/Fluent.fetch.phpt index 2d93a4af..9b03575e 100644 --- a/tests/dibi/Fluent.fetch.phpt +++ b/tests/dibi/Fluent.fetch.phpt @@ -28,7 +28,7 @@ Assert::equal([ // more complex association array -if ($config['system'] !== 'odbc') { +if (!in_array($config['system'], ['odbc', 'sqlsrv'])) { $res = $conn->select(['products.title' => 'title', 'customers.name' => 'name'])->select('orders.amount')->as('amount') ->from('products') ->innerJoin('orders')->using('(product_id)') diff --git a/tests/dibi/Fluent.select.phpt b/tests/dibi/Fluent.select.phpt index 3e0c7c13..0d957962 100644 --- a/tests/dibi/Fluent.select.phpt +++ b/tests/dibi/Fluent.select.phpt @@ -94,6 +94,7 @@ $fluent = $conn->select('*') Assert::same( reformat([ 'odbc' => 'SELECT TOP 1 * FROM ( SELECT * , (SELECT count(*) FROM [precteni] AS [P] WHERE P.id_clanku = C.id_clanku) FROM [clanky] AS [C] WHERE id_clanku=123) t', + 'sqlsrv' => ' SELECT * , (SELECT count(*) FROM [precteni] AS [P] WHERE P.id_clanku = C.id_clanku) FROM [clanky] AS [C] WHERE id_clanku=123 OFFSET 0 ROWS FETCH NEXT 1 ROWS ONLY', ' SELECT * , (SELECT count(*) FROM [precteni] AS [P] WHERE P.id_clanku = C.id_clanku) FROM [clanky] AS [C] WHERE id_clanku=123 LIMIT 1', ]), (string) $fluent diff --git a/tests/dibi/Result.meta.phpt b/tests/dibi/Result.meta.phpt index 0357384f..96b863c9 100644 --- a/tests/dibi/Result.meta.phpt +++ b/tests/dibi/Result.meta.phpt @@ -1,7 +1,7 @@ loadFile(__DIR__ . "/data/$config[system].sql"); $info = $conn->query(' SELECT products.product_id, orders.order_id, customers.name, products.product_id + 1 AS [xXx] - FROM products - INNER JOIN orders USING (product_id) - INNER JOIN customers USING (customer_id) + FROM ([products] + INNER JOIN [orders] ON [products.product_id] = [orders.product_id]) + INNER JOIN [customers] ON [orders.customer_id] = [customers.customer_id] ')->getInfo(); @@ -25,7 +25,7 @@ Assert::same( ); -if ($config['driver'] !== 'sqlite3' && $config['driver'] !== 'pdo') { +if (!in_array($config['driver'], ['sqlite3', 'pdo', 'sqlsrv'])) { Assert::same( ['products.product_id', 'orders.order_id', 'customers.name', 'xXx'], $info->getColumnNames(TRUE) @@ -36,18 +36,18 @@ if ($config['driver'] !== 'sqlite3' && $config['driver'] !== 'pdo') { $columns = $info->getColumns(); Assert::same('product_id', $columns[0]->getName()); -if ($config['driver'] !== 'sqlite3' && $config['driver'] !== 'pdo') { +if (!in_array($config['driver'], ['sqlite3', 'pdo', 'sqlsrv'])) { Assert::same('products', $columns[0]->getTableName()); } Assert::null($columns[0]->getVendorInfo('xxx')); -if ($config['system'] !== 'sqlite') { +if (!in_array($config['system'], ['sqlite', 'sqlsrv'])) { Assert::same('i', $columns[0]->getType()); } Assert::null($columns[0]->isNullable()); Assert::same('xXx', $columns[3]->getName()); Assert::null($columns[3]->getTableName()); -if ($config['system'] !== 'sqlite') { +if (!in_array($config['system'], ['sqlite', 'sqlsrv'])) { Assert::same('i', $columns[0]->getType()); } Assert::null($columns[3]->isNullable()); diff --git a/tests/dibi/Sqlsrv.limits.phpt b/tests/dibi/Sqlsrv.limits.phpt index d4bd25eb..cda91dd2 100644 --- a/tests/dibi/Sqlsrv.limits.phpt +++ b/tests/dibi/Sqlsrv.limits.phpt @@ -9,7 +9,10 @@ use Tester\Assert; require __DIR__ . '/bootstrap.php'; $tests = function ($conn) { - $version = $conn->getDriver()->getResource()->getAttribute(PDO::ATTR_SERVER_VERSION); + $resource = $conn->getDriver()->getResource(); + $version = is_resource($resource) + ? sqlsrv_server_info($resource)['SQLServerVersion'] + : $resource->getAttribute(PDO::ATTR_SERVER_VERSION); // MsSQL2012+ if (version_compare($version, '11.0') >= 0) { @@ -32,31 +35,43 @@ $tests = function ($conn) { ); // Offset invalid - Assert::same( - 'SELECT 1', - $conn->translate('SELECT 1 %ofs', -10) + Assert::error( + function () use ($conn) { + $conn->translate('SELECT 1 %ofs', -10); + }, + 'Dibi\NotSupportedException', + 'Negative offset or limit.' ); // Limit invalid - Assert::same( - 'SELECT 1', - $conn->translate('SELECT 1 %lmt', -10) + Assert::error( + function () use ($conn) { + $conn->translate('SELECT 1 %lmt', -10); + }, + 'Dibi\NotSupportedException', + 'Negative offset or limit.' ); // Limit invalid, offset valid - Assert::same( - 'SELECT 1', - $conn->translate('SELECT 1 %ofs %lmt', 10, -10) + Assert::error( + function () use ($conn) { + $conn->translate('SELECT 1 %ofs %lmt', 10, -10); + }, + 'Dibi\NotSupportedException', + 'Negative offset or limit.' ); // Limit valid, offset invalid - Assert::same( - 'SELECT 1', - $conn->translate('SELECT 1 %ofs %lmt', -10, 10) + Assert::error( + function () use ($conn) { + $conn->translate('SELECT 1 %ofs %lmt', -10, 10); + }, + 'Dibi\NotSupportedException', + 'Negative offset or limit.' ); } else { Assert::same( - 'SELECT TOP 1 * FROM (SELECT 1) t', + 'SELECT TOP (1) * FROM (SELECT 1) t', $conn->translate('SELECT 1 %lmt', 1) ); diff --git a/tests/dibi/Translator.phpt b/tests/dibi/Translator.phpt index 503cfa06..6b072434 100644 --- a/tests/dibi/Translator.phpt +++ b/tests/dibi/Translator.phpt @@ -141,6 +141,7 @@ Assert::same( Assert::same( reformat([ 'odbc' => 'SELECT TOP 2 * FROM (SELECT * FROM [products] ) t', + 'sqlsrv' => 'SELECT * FROM [products] OFFSET 0 ROWS FETCH NEXT 2 ROWS ONLY', 'SELECT * FROM [products] LIMIT 2', ]), $conn->translate('SELECT * FROM [products] %lmt', 2) @@ -153,7 +154,10 @@ if ($config['system'] === 'odbc') { } else { // with limit = 2, offset = 1 Assert::same( - reformat('SELECT * FROM [products] LIMIT 2 OFFSET 1'), + reformat([ + 'sqlsrv' => 'SELECT * FROM [products] OFFSET 1 ROWS FETCH NEXT 2 ROWS ONLY', + 'SELECT * FROM [products] LIMIT 2 OFFSET 1' + ]), $conn->translate('SELECT * FROM [products] %lmt %ofs', 2, 1) ); @@ -162,6 +166,7 @@ if ($config['system'] === 'odbc') { reformat([ 'mysql' => 'SELECT * FROM `products` LIMIT 18446744073709551615 OFFSET 50', 'postgre' => 'SELECT * FROM "products" OFFSET 50', + 'sqlsrv' => 'SELECT * FROM [products] OFFSET 50 ROWS', 'SELECT * FROM [products] LIMIT -1 OFFSET 50', ]), $conn->translate('SELECT * FROM [products] %ofs', 50) @@ -224,6 +229,7 @@ if ($config['system'] === 'postgre') { reformat([ 'sqlite' => "SELECT * FROM products WHERE (title LIKE 'C%' ESCAPE '\\' AND title LIKE '%r' ESCAPE '\\') OR title LIKE '%a\n\\%\\_\\\\''\"%' ESCAPE '\\'", 'odbc' => "SELECT * FROM products WHERE (title LIKE 'C%' AND title LIKE '%r') OR title LIKE '%a\n[%][_]\\''\"%'", + 'sqlsrv' => "SELECT * FROM products WHERE (title LIKE 'C%' AND title LIKE '%r') OR title LIKE '%a\n[%][_]\\''\"%'", "SELECT * FROM products WHERE (title LIKE 'C%' AND title LIKE '%r') OR title LIKE '%a\\n\\%\\_\\\\\\\\\'\"%'", ]), $conn->translate($args[0], $args[1], $args[2], $args[3]) @@ -425,6 +431,7 @@ Assert::same( Assert::same( reformat([ 'odbc' => 'SELECT TOP 10 * FROM (SELECT * FROM [test] WHERE [id] LIKE \'%d%t\' ) t', + 'sqlsrv' => 'SELECT * FROM [test] WHERE [id] LIKE \'%d%t\' OFFSET 0 ROWS FETCH NEXT 10 ROWS ONLY', 'SELECT * FROM [test] WHERE [id] LIKE \'%d%t\' LIMIT 10', ]), $conn->translate("SELECT * FROM [test] WHERE %n LIKE '%d%t' %lmt", 'id', 10) diff --git a/tests/dibi/data/sqlsrv.sql b/tests/dibi/data/sqlsrv.sql index 983ca4bf..9be70d6e 100644 --- a/tests/dibi/data/sqlsrv.sql +++ b/tests/dibi/data/sqlsrv.sql @@ -8,6 +8,7 @@ CREATE TABLE products ( title varchar(50) NOT NULL, PRIMARY KEY(product_id) ); +CREATE INDEX [title] ON [dbo].[products] ([title]); SET IDENTITY_INSERT products ON; INSERT INTO products (product_id, title) VALUES (1, 'Chair'); diff --git a/tests/dibi/meta.phpt b/tests/dibi/meta.phpt index a435943e..cb4158d9 100644 --- a/tests/dibi/meta.phpt +++ b/tests/dibi/meta.phpt @@ -48,7 +48,7 @@ Assert::same('title', $column->getName()); Assert::same('products', $column->getTable()->getName()); Assert::same('s', $column->getType()); Assert::type('string', $column->getNativeType()); -Assert::same(100, $column->getSize()); +Assert::same($config['system'] === 'sqlsrv' ? 50 : 100, $column->getSize()); Assert::false($column->isNullable()); Assert::false($column->isAutoIncrement()); //Assert::null($column->getDefault());