From 453985fc2c5556065837aea97ad6b527ca53e881 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Thu, 8 Oct 2020 12:25:20 +1300 Subject: [PATCH 01/16] Use src for code path --- _register_database.php | 10 +++++----- composer.json | 2 +- {code => src}/MSSQLAzureDatabase.php | 0 {code => src}/MSSQLDatabase.php | 0 {code => src}/MSSQLDatabaseConfigurationHelper.php | 0 {code => src}/MSSQLQueryBuilder.php | 0 {code => src}/MSSQLSchemaManager.php | 0 {code => src}/SQLServerConnector.php | 0 {code => src}/SQLServerQuery.php | 0 9 files changed, 6 insertions(+), 6 deletions(-) rename {code => src}/MSSQLAzureDatabase.php (100%) rename {code => src}/MSSQLDatabase.php (100%) rename {code => src}/MSSQLDatabaseConfigurationHelper.php (100%) rename {code => src}/MSSQLQueryBuilder.php (100%) rename {code => src}/MSSQLSchemaManager.php (100%) rename {code => src}/SQLServerConnector.php (100%) rename {code => src}/SQLServerQuery.php (100%) diff --git a/_register_database.php b/_register_database.php index 2a91a1e..3032255 100644 --- a/_register_database.php +++ b/_register_database.php @@ -9,12 +9,12 @@ 'class' => 'MSSQLPDODatabase', 'module' => 'mssql', 'title' => 'SQL Server 2008 (using PDO)', - 'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php', + 'helperPath' => __DIR__.'/src/MSSQLDatabaseConfigurationHelper.php', 'helperClass' => MSSQLDatabaseConfigurationHelper::class, 'supported' => !!MSSQLDatabaseConfigurationHelper::getPDODriver(), 'missingExtensionText' => - 'Either the PDO Extension or - the SQL Server PDO Driver + 'Either the PDO Extension or + the SQL Server PDO Driver are unavailable. Please install or enable these and refresh this page.' )); @@ -24,7 +24,7 @@ 'class' => 'MSSQLDatabase', 'module' => 'mssql', 'title' => 'SQL Server 2008 (using sqlsrv)', - 'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php', + 'helperPath' => __DIR__.'/src/MSSQLDatabaseConfigurationHelper.php', 'helperClass' => MSSQLDatabaseConfigurationHelper::class, 'supported' => function_exists('sqlsrv_connect'), 'missingExtensionText' => @@ -45,7 +45,7 @@ 'class' => 'MSSQLAzureDatabase', 'module' => 'mssql', 'title' => 'MS Azure Database (using sqlsrv)', - 'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php', + 'helperPath' => __DIR__.'/src/MSSQLDatabaseConfigurationHelper.php', 'helperClass' => MSSQLDatabaseConfigurationHelper::class, 'supported' => function_exists('sqlsrv_connect'), 'missingExtensionText' => diff --git a/composer.json b/composer.json index c8e5b13..d1cf319 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,7 @@ }, "autoload": { "psr-4": { - "SilverStripe\\MSSQL\\": "code/" + "SilverStripe\\MSSQL\\": "src/" } }, "prefer-stable": true, diff --git a/code/MSSQLAzureDatabase.php b/src/MSSQLAzureDatabase.php similarity index 100% rename from code/MSSQLAzureDatabase.php rename to src/MSSQLAzureDatabase.php diff --git a/code/MSSQLDatabase.php b/src/MSSQLDatabase.php similarity index 100% rename from code/MSSQLDatabase.php rename to src/MSSQLDatabase.php diff --git a/code/MSSQLDatabaseConfigurationHelper.php b/src/MSSQLDatabaseConfigurationHelper.php similarity index 100% rename from code/MSSQLDatabaseConfigurationHelper.php rename to src/MSSQLDatabaseConfigurationHelper.php diff --git a/code/MSSQLQueryBuilder.php b/src/MSSQLQueryBuilder.php similarity index 100% rename from code/MSSQLQueryBuilder.php rename to src/MSSQLQueryBuilder.php diff --git a/code/MSSQLSchemaManager.php b/src/MSSQLSchemaManager.php similarity index 100% rename from code/MSSQLSchemaManager.php rename to src/MSSQLSchemaManager.php diff --git a/code/SQLServerConnector.php b/src/SQLServerConnector.php similarity index 100% rename from code/SQLServerConnector.php rename to src/SQLServerConnector.php diff --git a/code/SQLServerQuery.php b/src/SQLServerQuery.php similarity index 100% rename from code/SQLServerQuery.php rename to src/SQLServerQuery.php From ecf4308d801165dad441613d31bbf7df515dbc5f Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Thu, 8 Oct 2020 12:26:19 +1300 Subject: [PATCH 02/16] Remove unneeded constructor, connection should happen with connect() --- src/MSSQLAzureDatabase.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/MSSQLAzureDatabase.php b/src/MSSQLAzureDatabase.php index 69df5cf..07a194f 100644 --- a/src/MSSQLAzureDatabase.php +++ b/src/MSSQLAzureDatabase.php @@ -34,11 +34,6 @@ public function fullTextEnabled() return false; } - public function __construct($parameters) - { - $this->connectDatabase($parameters); - } - /** * Connect to a SQL Azure database with the given parameters. * @param array $parameters Connection parameters set by environment From 16b8658a78f0c602d290caab1d2905a60c8cab53 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Thu, 8 Oct 2020 16:52:50 +1300 Subject: [PATCH 03/16] Update docs and lint --- README.md | 18 ++++++++++++++++-- src/MSSQLDatabase.php | 34 ---------------------------------- src/SQLServerConnector.php | 7 +++++++ src/SQLServerQuery.php | 31 ++++++++++++++----------------- 4 files changed, 37 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index b70b9d4..66e0de6 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Allows SilverStripe to use SQL Server databases. * Sean Harvey (Nickname: halkyon) - + * Damian Mooyman (@tractorcow) ## Requirements @@ -39,7 +39,21 @@ These steps will install the latest SilverStripe stable, along with this module * Install SilverStripe: `composer create-project silverstripe/installer /my/website/folder` * Install module: `cd /my/website/folder && composer require silverstripe/mssql ^2` * Open the SilverStripe installer by browsing to install.php, e.g. **http://localhost/silverstripe/install.php** - * Select **SQL Server 2008+** in the database list and enter your SQL Server database details + * Select **SQL Server 2008+** in the database list and enter your SQL Server database details. If you aren't using the + installer then configure the following environment variables: + +``` +SS_DATABASE_CLASS="MSSQLAzureDatabase" # or: `MSSQLDatabase` or `MSSQLPDODatabase` +SS_DATABASE_SERVER="" +SS_DATABASE_NAME="" +SS_DATABASE_USERNAME="" +SS_DATABASE_PASSWORD="" +``` + +Note on OSX / Linux machines you will need to install the ODBC drivers and the PHP extension `sqlsrv` or `pdo_sqlsrv` + +* https://docs.microsoft.com/en-us/sql/connect/odbc/linux-mac/install-microsoft-odbc-driver-sql-server-macos +* https://docs.microsoft.com/en-us/sql/connect/php/installation-tutorial-linux-mac ## Troubleshooting diff --git a/src/MSSQLDatabase.php b/src/MSSQLDatabase.php index 66dcfc1..58fdd30 100644 --- a/src/MSSQLDatabase.php +++ b/src/MSSQLDatabase.php @@ -15,40 +15,6 @@ /** * Microsoft SQL Server 2008+ connector class. - * - *

Connecting using Windows

- * - * If you've got your website running on Windows, it's highly recommended you - * use Microsoft SQL Server Driver for PHP "sqlsrv". - * - * A complete guide to installing a Windows IIS + PHP + SQL Server web stack can be - * found here: http://doc.silverstripe.org/installation-on-windows-server-manual-iis - * - * @see http://sqlsrvphp.codeplex.com/ - * - *

Connecting using Linux or Mac OS X

- * - * The following commands assume you used the default package manager - * to install PHP with the operating system. - * - * Debian, and Ubuntu: - * apt-get install php5-sybase - * - * Fedora, CentOS and RedHat: - * yum install php-mssql - * - * Mac OS X (MacPorts): - * port install php5-mssql - * - * These packages will install the mssql extension for PHP, as well - * as FreeTDS, which will let you connect to SQL Server. - * - * More information available in the SilverStripe developer wiki: - * @see http://doc.silverstripe.org/modules:mssql - * @see http://doc.silverstripe.org/installation-on-windows-server-manual-iis - * - * References: - * @see http://freetds.org */ class MSSQLDatabase extends Database { diff --git a/src/SQLServerConnector.php b/src/SQLServerConnector.php index 7ae76ed..fbd2da8 100644 --- a/src/SQLServerConnector.php +++ b/src/SQLServerConnector.php @@ -3,6 +3,12 @@ namespace SilverStripe\MSSQL; use SilverStripe\ORM\Connect\DBConnector; +use function sqlsrv_connect; +use function sqlsrv_begin_transaction; +use function sqlsrv_rollback; +use function sqlsrv_query; + +use const MSSQL_USE_WINDOWS_AUTHENTICATION; /** * Database connector driver for sqlsrv_ library @@ -64,6 +70,7 @@ public function connect($parameters, $selectDB = false) $this->dbConn = sqlsrv_connect($parameters['server'], $options); if (empty($this->dbConn)) { + var_dump(sqlsrv_errors()); $this->databaseError("Couldn't connect to SQL Server database"); } elseif ($selectDB && !empty($parameters['database'])) { // Check selected database (Azure) diff --git a/src/SQLServerQuery.php b/src/SQLServerQuery.php index d0cf3b5..56f02b5 100644 --- a/src/SQLServerQuery.php +++ b/src/SQLServerQuery.php @@ -4,6 +4,10 @@ use DateTime; use SilverStripe\ORM\Connect\Query; +use function sqlsrv_next_result; +use function sqlsrv_num_rows; +use function sqlsrv_free_stmt; +use const SQLSRV_SCROLL_ABSOLUTE; /** * A result-set from a MSSQL database. @@ -43,23 +47,6 @@ public function __destruct() } } - public function getIterator() - { - if (is_resource($this->handle)) { - while ($data = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC)) { - // special case for sqlsrv - date values are DateTime coming out of the sqlsrv drivers, - // so we convert to the usual Y-m-d H:i:s value! - foreach ($data as $name => $value) { - if ($value instanceof DateTime) { - $data[$name] = $value->format('Y-m-d H:i:s'); - } - } - - yield $data; - } - } - } - public function numRecords() { if (!is_resource($this->handle)) { @@ -73,4 +60,14 @@ public function numRecords() user_error('MSSQLQuery::numRecords() not supported in this version of sqlsrv', E_USER_WARNING); } } + + public function nextRecord() + { + return sqlsrv_next_result($this->handle); + } + + public function seek($row) + { + return sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); + } } From bf6846164768e459bf7e9d33d00535be49b99c83 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 14:29:20 +1300 Subject: [PATCH 04/16] Linting, update documentation --- README.md | 52 +++++------ src/MSSQLSchemaManager.php | 147 ++++++++++++++++--------------- tests/MSSQLDatabaseQueryTest.php | 8 +- 3 files changed, 107 insertions(+), 100 deletions(-) diff --git a/README.md b/README.md index 66e0de6..d260094 100644 --- a/README.md +++ b/README.md @@ -1,22 +1,17 @@ # SQL Server Database Module -Allows SilverStripe to use SQL Server databases. +Allows SilverStripe to use SQL Server databases including SQL databases on Azure. -[![Build status](https://ci.appveyor.com/api/projects/status/hep0l5kbhu64n7l3/branch/master?svg=true)](https://ci.appveyor.com/project/sminnee/silverstripe-mssql-nwvfq/branch/master) - -## Maintainer Contact - - * Sean Harvey (Nickname: halkyon) - - - * Damian Mooyman (@tractorcow) +[![Build status](https://ci.appveyor.com/api/projects/status/hep0l5kbhu64n7l3/branch/master?svg=true)](https://ci.appveyor.com/project/silverstripe/silverstripe-mssql/branch/master) +[![Version](http://img.shields.io/packagist/v/silverstripe/silverstripe-mssql.svg?style=flat-square)](https://packagist.org/packages/silverstripe/silverstripe-mssql) +[![License](http://img.shields.io/packagist/l/silverstripe/silverstripe-mssql.svg?style=flat-square)](LICENSE) ## Requirements * SilverStripe 4+ * SQL Server 2008, 2008 R2, or 2012. -`mssql` PHP api is no longer supported as of 2.0 +`mssql` PHP api is no longer supported as of 2.0. ### *nix @@ -30,17 +25,16 @@ Linux support is only available via the PDO extension. This requires: On windows you can either connect via PDO or `sqlsrv`. Both options require the [SQL Server Driver for PHP](https://msdn.microsoft.com/library/dn865013.aspx?f=255&MSPPError=-2147217396). "sqlsrv" 3.0+ -Note: [SQL Server Express](http://www.microsoft.com/express/Database/) can also be used which is provided free by Microsoft. However, it has limitations such as 10GB maximum database storage. +Note: [SQL Server Express](http://www.microsoft.com/express/Database/) can also be used which is provided free by +Microsoft. However, it has limitations such as 10GB maximum database storage. ## Installation -These steps will install the latest SilverStripe stable, along with this module using [Composer](http://getcomposer.org/): +``` +composer require silverstripe/mssql ^2 +``` - * Install SilverStripe: `composer create-project silverstripe/installer /my/website/folder` - * Install module: `cd /my/website/folder && composer require silverstripe/mssql ^2` - * Open the SilverStripe installer by browsing to install.php, e.g. **http://localhost/silverstripe/install.php** - * Select **SQL Server 2008+** in the database list and enter your SQL Server database details. If you aren't using the - installer then configure the following environment variables: +The following environment variables will need to be set: ``` SS_DATABASE_CLASS="MSSQLAzureDatabase" # or: `MSSQLDatabase` or `MSSQLPDODatabase` @@ -63,12 +57,18 @@ A: Please ensure you have enabled TCP access using **SQL Server Configuration Ma *Q: I just installed SQL Server, but it says that it cannot connect* -A: Sometimes SQL Server will be installed as a non-default instance name, e.g. "SQLExpress" instead of "MSSQLSERVER" (the default.) -If this is the case, you'll need to declare the instance name when setting the server in your PHP database configuration. For example: **(local)\SQLExpress**. The first part before the slash indicates the server host, or IP address. In this case, (local) indicates localhost, which is the same server PHP is running on. The second part is the SQL Server instance name to connect to. +A: Sometimes SQL Server will be installed as a non-default instance name, e.g. "SQLExpress" instead of "MSSQLSERVER" +(the default.) If this is the case, you'll need to declare the instance name when setting the server in your PHP +database configuration. For example: **(local)\SQLExpress**. The first part before the slash indicates the server host, +or IP address. In this case, (local) indicates localhost, which is the same server PHP is running on. The second part +is the SQL Server instance name to connect to. -*Q: I'm getting unicode SQL Server errors connecting to SQL Server database (e.g. Unicode data in a Unicode-only collation or ntext data cannot be sent to clients using DB-Library (such as ISQL) or ODBC version 3.7 or earlier)* +*Q: I'm getting unicode SQL Server errors connecting to SQL Server database (e.g. Unicode data in a Unicode-only +collation or ntext data cannot be sent to clients using DB-Library (such as ISQL) or ODBC version 3.7 or earlier)* -A: If you are using FreeTDS make sure you're using TDS version 8.0 in **freetds.conf**. If on Windows, ensure you use the [SQL Server Driver for PHP](http://www.microsoft.com/downloads/en/details.aspx?displaylang=en&FamilyID=ccdf728b-1ea0-48a8-a84a-5052214caad9) and **NOT** the mssql drivers provided by PHP. +A: If you are using FreeTDS make sure you're using TDS version 8.0 in **freetds.conf**. If on Windows, ensure you use +the [SQL Server Driver for PHP](http://www.microsoft.com/downloads/en/details.aspx?displaylang=en&FamilyID=ccdf728b-1ea0-48a8-a84a-5052214caad9) +and **NOT** the mssql drivers provided by PHP. *Q: Using FreeTDS I can't connect to my SQL Server database. An error in PHP says the server doesn't exist* @@ -83,10 +83,12 @@ Then you can use "myserver" (the bit in square brackets above) as the server nam Note that if you're running Macports, the file is located in **/opt/local/etc/freetds/freetds.conf**. Alternatively, if you don't want to keep adding more entries to the freetds.conf to nominate more SQL Server locations, -you can instead use the full the host/ip and port combination, such as "myserver:1433" (1433 being the default SQL Server port.) -and ensure the "tds version = 8.0" is set globally in the freetds.conf file. +you can instead use the full the host/ip and port combination, such as "myserver:1433" (1433 being the default SQL +Server port.) and ensure the "tds version = 8.0" is set globally in the freetds.conf file. -**Note**: Use *tabs* not spaces when editing freetds.conf, otherwise it will not load the configuration you have specified! +**Note**: Use *tabs* not spaces when editing freetds.conf, otherwise it will not load the configuration you have + specified! -**Note**: Certain distributions of Linux use [SELinux](http://fedoraproject.org/wiki/SELinux) which could block access to your SQL Server database. A rule may need to be added to allow this traffic through. +**Note**: Certain distributions of Linux use [SELinux](http://fedoraproject.org/wiki/SELinux) which could block access +to your SQL Server database. A rule may need to be added to allow this traffic through. diff --git a/src/MSSQLSchemaManager.php b/src/MSSQLSchemaManager.php index f390316..bbc6692 100644 --- a/src/MSSQLSchemaManager.php +++ b/src/MSSQLSchemaManager.php @@ -2,10 +2,11 @@ namespace SilverStripe\MSSQL; +use Exception; use SilverStripe\ORM\Connect\DBSchemaManager; /** - * Represents and handles all schema management for a MS SQL database + * Represents and handles all schema management for a MSSQL database */ class MSSQLSchemaManager extends DBSchemaManager { @@ -15,7 +16,7 @@ class MSSQLSchemaManager extends DBSchemaManager * * @var array */ - protected static $cached_checks = array(); + protected static $cached_checks = []; /** * Builds the internal MS SQL Server index name given the silverstripe table and index name @@ -27,8 +28,6 @@ class MSSQLSchemaManager extends DBSchemaManager */ public function buildMSSQLIndexName($tableName, $indexName, $prefix = 'ix') { - - // Cleanup names of namespaced tables $tableName = str_replace('\\', '_', $tableName); $indexName = str_replace('\\', '_', $indexName); @@ -107,9 +106,9 @@ public function fulltextIndexExists($tableName) } return (bool) $this->preparedQuery(" - SELECT 1 FROM sys.fulltext_indexes i - JOIN sys.objects o ON i.object_id = o.object_id - WHERE o.name = ?", + SELECT 1 FROM sys.fulltext_indexes i + JOIN sys.objects o ON i.object_id = o.object_id + WHERE o.name = ?", array($tableName) )->value(); } @@ -135,27 +134,6 @@ public function getPrimaryKey($tableName) return $indexName; } - /** - * Gets the identity column of a table - * - * @param string $tableName - * @return string|null - */ - public function getIdentityColumn($tableName) - { - return $this->preparedQuery(" - SELECT - TABLE_NAME + '.' + COLUMN_NAME, - TABLE_NAME - FROM - INFORMATION_SCHEMA.COLUMNS - WHERE - TABLE_SCHEMA = ? AND - COLUMNPROPERTY(object_id(TABLE_NAME), COLUMN_NAME, 'IsIdentity') = 1 AND - TABLE_NAME = ? - ", array('dbo', $tableName))->value(); - } - public function createDatabase($name) { $this->query("CREATE DATABASE \"$name\""); @@ -184,6 +162,7 @@ public function databaseList() /** * Create a new table. + * * @param string $tableName The name of the table * @param array $fields A map of field names to field types * @param array $indexes A map of indexes @@ -208,13 +187,17 @@ public function createTable($tableName, $fields = null, $indexes = null, $option $tableName = "#$tableName" . '-' . rand(1000000, 9999999); } - $this->query("CREATE TABLE \"$tableName\" ( - $fieldSchemas - primary key (\"ID\") - );"); + try { + $this->query("CREATE TABLE \"$tableName\" ( + $fieldSchemas + primary key (\"ID\") + );"); + } catch (Exception $e) { + // + } - //we need to generate indexes like this: CREATE INDEX IX_vault_to_export ON vault (to_export); - //This needs to be done AFTER the table creation, so we can set up the fulltext indexes correctly + // we need to generate indexes like this: CREATE INDEX IX_vault_to_export ON vault (to_export); + // This needs to be done AFTER the table creation, so we can set up the fulltext indexes correctly if ($indexes) { foreach ($indexes as $k => $v) { $indexSchemas .= $this->getIndexSqlDefinition($tableName, $k, $v) . "\n"; @@ -289,9 +272,9 @@ public function alterTable($tableName, $newFields = null, $newIndexes = null, $a public function getConstraintName($tableName, $columnName) { return $this->preparedQuery(" - SELECT CONSTRAINT_NAME - FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE - WHERE TABLE_NAME = ? AND COLUMN_NAME = ?", + SELECT CONSTRAINT_NAME + FROM INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE + WHERE TABLE_NAME = ? AND COLUMN_NAME = ?", array($tableName, $columnName) )->value(); } @@ -321,10 +304,10 @@ public function getConstraintCheckClause($tableName, $columnName) // Regenerate cehcks for this table $checks = array(); foreach ($this->preparedQuery(" - SELECT CAST(CHECK_CLAUSE AS TEXT) AS CHECK_CLAUSE, COLUMN_NAME - FROM INFORMATION_SCHEMA.CHECK_CONSTRAINTS AS CC - INNER JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE AS CCU ON CCU.CONSTRAINT_NAME = CC.CONSTRAINT_NAME - WHERE TABLE_NAME = ?", + SELECT CAST(CHECK_CLAUSE AS TEXT) AS CHECK_CLAUSE, COLUMN_NAME + FROM INFORMATION_SCHEMA.CHECK_CONSTRAINTS AS CC + INNER JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE AS CCU ON CCU.CONSTRAINT_NAME = CC.CONSTRAINT_NAME + WHERE TABLE_NAME = ?", array($tableName) ) as $record) { $checks[$record['COLUMN_NAME']] = $record['CHECK_CLAUSE']; @@ -346,13 +329,13 @@ public function getConstraintCheckClause($tableName, $columnName) protected function defaultConstraintName($tableName, $colName) { return $this->preparedQuery(" - SELECT s.name --default name - FROM sys.sysobjects s - join sys.syscolumns c ON s.parent_obj = c.id - WHERE s.xtype = 'd' - and c.cdefault = s.id - and parent_obj = OBJECT_ID(?) - and c.name = ?", + SELECT s.name --default name + FROM sys.sysobjects s + join sys.syscolumns c ON s.parent_obj = c.id + WHERE s.xtype = 'd' + and c.cdefault = s.id + and parent_obj = OBJECT_ID(?) + and c.name = ?", array($tableName, $colName) )->value(); } @@ -414,7 +397,8 @@ protected function alterTableAlterColumn($tableName, $colName, $colSpec) } if (isset($matches['definition'])) { - //We will prevent any changes being made to the ID column. Primary key indexes will have a fit if we do anything here. + // We will prevent any changes being made to the ID column. + // Primary key indexes will have a fit if we do anything here. if ($colName != 'ID') { // SET null / not null @@ -442,6 +426,10 @@ protected function alterTableAlterColumn($tableName, $colName, $colSpec) return implode("\n", $alterQueries); } + /** + * @param string $oldTableName + * @param string $newTableName + */ public function renameTable($oldTableName, $newTableName) { $this->query("EXEC sp_rename \"$oldTableName\", \"$newTableName\""); @@ -459,6 +447,11 @@ public function checkAndRepairTable($tableName) return true; } + /** + * @param string $tableName + * @param string $fieldName + * @param string $fieldSpec + */ public function createField($tableName, $fieldName, $fieldSpec) { $this->query("ALTER TABLE \"$tableName\" ADD \"$fieldName\" $fieldSpec"); @@ -466,6 +459,7 @@ public function createField($tableName, $fieldName, $fieldSpec) /** * Change the database type of the given field. + * * @param string $tableName The name of the tbale the field is in. * @param string $fieldName The name of the field to change. * @param string $fieldSpec The new field specification @@ -475,18 +469,28 @@ public function alterField($tableName, $fieldName, $fieldSpec) $this->query("ALTER TABLE \"$tableName\" CHANGE \"$fieldName\" \"$fieldName\" $fieldSpec"); } + /** + * @param string $tableName + * @param string $oldName + * @param string $newName + * + */ public function renameField($tableName, $oldName, $newName) { $this->query("EXEC sp_rename @objname = '$tableName.$oldName', @newname = '$newName', @objtype = 'COLUMN'"); } + /** + * @param string $table + * + * @return array + */ public function fieldList($table) { - //This gets us more information than we need, but I've included it all for the moment.... $fieldRecords = $this->preparedQuery("SELECT ordinal_position, column_name, data_type, column_default, - is_nullable, character_maximum_length, numeric_precision, numeric_scale, collation_name - FROM information_schema.columns WHERE table_name = ? - ORDER BY ordinal_position;", + is_nullable, character_maximum_length, numeric_precision, numeric_scale, collation_name + FROM information_schema.columns WHERE table_name = ? + ORDER BY ordinal_position;", array($table) ); @@ -602,13 +606,12 @@ public function createIndex($tableName, $indexName, $indexSpec) */ protected function getIndexSqlDefinition($tableName, $indexName, $indexSpec) { + if (is_array($indexSpec['columns'])) { + $indexSpec['columns'] = $this->implodeColumnList($indexSpec['columns']); + } - // Determine index name $index = $this->buildMSSQLIndexName($tableName, $indexName); - // Consolidate/Cleanup spec into array format - $indexSpec = $this->parseIndexSpec($indexName, $indexSpec); - $drop = "IF EXISTS (SELECT name FROM sys.indexes WHERE name = '$index' AND object_id = object_id(SCHEMA_NAME() + '.$tableName')) DROP INDEX $index ON \"$tableName\";"; // create a type-specific index @@ -621,16 +624,16 @@ protected function getIndexSqlDefinition($tableName, $indexName, $indexSpec) $primary_key = $this->getPrimaryKey($tableName); if ($primary_key) { - return "$drop CREATE FULLTEXT INDEX ON \"$tableName\" ({$indexSpec['value']})" + return "$drop CREATE FULLTEXT INDEX ON \"$tableName\" ({$indexSpec['columns']})" . "KEY INDEX $primary_key WITH CHANGE_TRACKING AUTO;"; } } if ($indexSpec['type'] == 'unique') { - return "$drop CREATE UNIQUE INDEX $index ON \"$tableName\" ({$indexSpec['value']});"; + return "$drop CREATE UNIQUE INDEX $index ON \"$tableName\" ({$indexSpec['columns']});"; } - return "$drop CREATE INDEX $index ON \"$tableName\" ({$indexSpec['value']});"; + return "$drop CREATE INDEX $index ON \"$tableName\" ({$indexSpec['columns']});"; } public function alterIndex($tableName, $indexName, $indexSpec) @@ -640,13 +643,14 @@ public function alterIndex($tableName, $indexName, $indexSpec) /** * Return the list of indexes in a table. + * * @param string $table The table name. * @return array */ public function indexList($table) { $indexes = $this->query("EXEC sp_helpindex '$table';"); - $indexList = array(); + $indexList = []; // Enumerate all basic indexes foreach ($indexes as $index) { @@ -662,11 +666,11 @@ public function indexList($table) // Extract columns $columns = $this->quoteColumnSpecString($index['index_keys']); - $indexList[$indexName] = $this->parseIndexSpec($indexName, array( + $indexList[$indexName] = [ 'name' => $indexName, - 'value' => $columns, + 'columns' => $columns, 'type' => $indexType - )); + ]; } // Now we need to check to see if we have any fulltext indexes attached to this table: @@ -674,7 +678,7 @@ public function indexList($table) $result = $this->query('EXEC sp_help_fulltext_columns;'); // Extract columns from this fulltext definition - $columns = array(); + $columns = []; foreach ($result as $row) { if ($row['TABLE_NAME'] == $table) { $columns[] = $row['FULLTEXT_COLUMN_NAME']; @@ -682,11 +686,11 @@ public function indexList($table) } if (!empty($columns)) { - $indexList['SearchFields'] = $this->parseIndexSpec('SearchFields', array( + $indexList['SearchFields'] = [ 'name' => 'SearchFields', - 'value' => $this->implodeColumnList($columns), + 'columns' => $this->implodeColumnList($columns), 'type' => 'fulltext' - )); + ]; } } @@ -703,9 +707,9 @@ public function indexList($table) public function indexNames($tableName) { return $this->preparedQuery(' - SELECT ind.name FROM sys.indexes ind - INNER JOIN sys.tables t ON ind.object_id = t.object_id - WHERE is_primary_key = 0 AND t.name = ? AND ind.name IS NOT NULL', + SELECT ind.name FROM sys.indexes ind + INNER JOIN sys.tables t ON ind.object_id = t.object_id + WHERE is_primary_key = 0 AND t.name = ? AND ind.name IS NOT NULL', array($tableName) )->column(); } @@ -759,6 +763,7 @@ public function decimal($values) } $defaultValue = '0'; + if (isset($values['default']) && is_numeric($values['default'])) { $defaultValue = $values['default']; } diff --git a/tests/MSSQLDatabaseQueryTest.php b/tests/MSSQLDatabaseQueryTest.php index 9d8207b..5980efe 100644 --- a/tests/MSSQLDatabaseQueryTest.php +++ b/tests/MSSQLDatabaseQueryTest.php @@ -9,9 +9,9 @@ class MSSQLDatabaseQueryTest extends SapphireTest public static $fixture_file = 'MSSQLDatabaseQueryTest.yml'; - protected $extraDataObjects = array( + protected $extraDataObjects = [ 'MSSQLDatabaseQueryTestDataObject' - ); + ]; public function testDateValueFormatting() { @@ -28,8 +28,8 @@ public function testDatetimeValueFormatting() class MSSQLDatabaseQueryTestDataObject extends DataObject implements TestOnly { - private static $db = array( + private static $db = [ 'TestDate' => 'Date', 'TestDatetime' => 'Datetime' - ); + ]; } From 2da5a55888d6d6042ab36fae8ecaf257fd309463 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 15:39:23 +1300 Subject: [PATCH 05/16] Lint clean --- src/MSSQLAzureDatabase.php | 8 +++++--- src/MSSQLDatabase.php | 1 + src/MSSQLDatabaseConfigurationHelper.php | 7 ++++++- src/SQLServerConnector.php | 11 +++++++---- src/SQLServerQuery.php | 24 ++++++++++++++++++------ 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/MSSQLAzureDatabase.php b/src/MSSQLAzureDatabase.php index 07a194f..ed97ae2 100644 --- a/src/MSSQLAzureDatabase.php +++ b/src/MSSQLAzureDatabase.php @@ -9,6 +9,7 @@ * Some important things about SQL Azure: * * Selecting a database is not supported. + * * In order to change the database currently in use, you need to connect to * the database using the "Database" parameter with sqlsrv_connect() * @@ -16,8 +17,6 @@ * have two query results open at once. * * Fulltext indexes are not supported. - * - * @author Sean Harvey */ class MSSQLAzureDatabase extends MSSQLDatabase { @@ -27,7 +26,7 @@ class MSSQLAzureDatabase extends MSSQLDatabase * * @var array */ - protected $parameters = array(); + protected $parameters = []; public function fullTextEnabled() { @@ -90,6 +89,7 @@ protected function connectDatabase($database) public function selectDatabase($name, $create = false, $errorLevel = E_USER_ERROR) { $this->fullTextEnabled = null; + if (!$this->schemaManager->databaseExists($name)) { // Check DB creation permisson if (!$create) { @@ -102,7 +102,9 @@ public function selectDatabase($name, $create = false, $errorLevel = E_USER_ERRO } $this->schemaManager->createDatabase($name); } + $this->connectDatabase($name); + return true; } } diff --git a/src/MSSQLDatabase.php b/src/MSSQLDatabase.php index 58fdd30..e914ecb 100644 --- a/src/MSSQLDatabase.php +++ b/src/MSSQLDatabase.php @@ -504,6 +504,7 @@ protected function inspectQuery($sql) { // Any DDL discards transactions. $isDDL = $this->getConnector()->isQueryDDL($sql); + if ($isDDL) { $this->resetTransactionNesting(); } diff --git a/src/MSSQLDatabaseConfigurationHelper.php b/src/MSSQLDatabaseConfigurationHelper.php index 1bdc8eb..cb19203 100644 --- a/src/MSSQLDatabaseConfigurationHelper.php +++ b/src/MSSQLDatabaseConfigurationHelper.php @@ -96,11 +96,13 @@ public static function getPDODriver() { if (!class_exists('PDO')) { return null; } + foreach(PDO::getAvailableDrivers() as $driver) { - if(in_array($driver, array('sqlsrv', 'dblib'))) { + if (in_array($driver, array('sqlsrv', 'dblib'))) { return $driver; } } + return null; } @@ -122,6 +124,7 @@ protected function quote($conn, $value) } else { user_error('Invalid database connection', E_USER_ERROR); } + return null; } @@ -150,6 +153,7 @@ protected function query($conn, $sql) } } } + return $items; } @@ -256,6 +260,7 @@ public function requireDatabaseAlterPermissions($databaseConfig) // Make sure to select the current database when checking permission against this database $this->query($conn, "USE \"{$databaseConfig['database']}\""); } + $permissions = $this->query($conn, "select COUNT(*) from sys.fn_my_permissions(NULL,'DATABASE') WHERE permission_name like 'create table';"); $success = $permissions[0] > 0; } diff --git a/src/SQLServerConnector.php b/src/SQLServerConnector.php index fbd2da8..ee15bf8 100644 --- a/src/SQLServerConnector.php +++ b/src/SQLServerConnector.php @@ -70,7 +70,6 @@ public function connect($parameters, $selectDB = false) $this->dbConn = sqlsrv_connect($parameters['server'], $options); if (empty($this->dbConn)) { - var_dump(sqlsrv_errors()); $this->databaseError("Couldn't connect to SQL Server database"); } elseif ($selectDB && !empty($parameters['database'])) { // Check selected database (Azure) @@ -84,6 +83,7 @@ public function connect($parameters, $selectDB = false) public function transactionStart() { $result = sqlsrv_begin_transaction($this->dbConn); + if (!$result) { $this->databaseError("Couldn't start the transaction."); } @@ -95,6 +95,7 @@ public function transactionStart() public function transactionEnd() { $result = sqlsrv_commit($this->dbConn); + if (!$result) { $this->databaseError("Couldn't commit the transaction."); } @@ -122,11 +123,13 @@ public function getLastError() { $errorMessages = array(); $errors = sqlsrv_errors(); + if ($errors) { foreach ($errors as $info) { $errorMessages[] = implode(', ', array($info['SQLSTATE'], $info['code'], $info['message'])); } } + return implode('; ', $errorMessages); } @@ -142,6 +145,7 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) // Run query $parsedParameters = $this->parameterValues($parameters); + if (empty($parsedParameters)) { $handle = sqlsrv_query($this->dbConn, $sql); } else { @@ -154,14 +158,14 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) return null; } - // Report result $this->lastAffectedRows = sqlsrv_rows_affected($handle); + return new SQLServerQuery($this, $handle); } public function query($sql, $errorLevel = E_USER_ERROR) { - return $this->preparedQuery($sql, array(), $errorLevel); + return $this->preparedQuery($sql, [], $errorLevel); } public function selectDatabase($name) @@ -180,7 +184,6 @@ public function __destruct() public function getVersion() { - // @todo - use sqlsrv_server_info? return trim($this->query("SELECT CONVERT(char(15), SERVERPROPERTY('ProductVersion'))")->value()); } diff --git a/src/SQLServerQuery.php b/src/SQLServerQuery.php index 56f02b5..55c4fcf 100644 --- a/src/SQLServerQuery.php +++ b/src/SQLServerQuery.php @@ -2,9 +2,9 @@ namespace SilverStripe\MSSQL; -use DateTime; +use SilverStripe\ORM\Connect\DatabaseException; use SilverStripe\ORM\Connect\Query; -use function sqlsrv_next_result; +use function sqlsrv_fetch_array; use function sqlsrv_num_rows; use function sqlsrv_free_stmt; use const SQLSRV_SCROLL_ABSOLUTE; @@ -20,14 +20,14 @@ class SQLServerQuery extends Query * * @var SQLServerConnector */ - private $connector; + protected $connector; /** * The internal MSSQL handle that points to the result set. * * @var resource */ - private $handle; + protected $handle; /** * Hook the result-set given into a Query class, suitable for use by sapphire. @@ -63,11 +63,23 @@ public function numRecords() public function nextRecord() { - return sqlsrv_next_result($this->handle); + if (is_resource($this->handle)) { + $row = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC); + + return $row; + } else { + return false; + } } public function seek($row) { - return sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); + if (is_object($this->handle)) { + sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); + $result = $this->nextRecord(); + sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); + return $result; + } + return null; } } From a7f30bf807012cbf3dcfcd182eaf64acbeb95ba5 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 15:40:35 +1300 Subject: [PATCH 06/16] FIX hasTable returning no result for a valid table. --- src/MSSQLSchemaManager.php | 62 +++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/src/MSSQLSchemaManager.php b/src/MSSQLSchemaManager.php index bbc6692..bb5eab2 100644 --- a/src/MSSQLSchemaManager.php +++ b/src/MSSQLSchemaManager.php @@ -147,6 +147,7 @@ public function dropDatabase($name) public function databaseExists($name) { $databases = $this->databaseList(); + foreach ($databases as $dbname) { if ($dbname == $name) { return true; @@ -183,7 +184,6 @@ public function createTable($tableName, $fields = null, $indexes = null, $option // Temporary tables start with "#" in MSSQL-land if (!empty($options['temporary'])) { - // Randomize the temp table name to avoid conflicts in the tempdb table which derived databases share $tableName = "#$tableName" . '-' . rand(1000000, 9999999); } @@ -193,7 +193,7 @@ public function createTable($tableName, $fields = null, $indexes = null, $option primary key (\"ID\") );"); } catch (Exception $e) { - // + $this->alterationMessage('Could not CREATE TABLE '. $tableName .': '. $e->getMessage(), 'error'); } // we need to generate indexes like this: CREATE INDEX IX_vault_to_export ON vault (to_export); @@ -491,20 +491,21 @@ public function fieldList($table) is_nullable, character_maximum_length, numeric_precision, numeric_scale, collation_name FROM information_schema.columns WHERE table_name = ? ORDER BY ordinal_position;", - array($table) + [$table] ); // Cache the records from the query - otherwise a lack of multiple active result sets // will cause subsequent queries to fail in this method - $fields = array(); - $output = array(); + $fields = []; + $output = []; + foreach ($fieldRecords as $record) { - $fields[] = $record; + if ($record) { + $fields[] = $record; + } } foreach ($fields as $field) { - // Update the data_type field to be a complete column definition string for use by - // SS_Database::requireField() switch ($field['data_type']) { case 'int': case 'bigint': @@ -514,16 +515,17 @@ public function fieldList($table) if ($field['data_type'] != 'bigint' && $field['data_type'] != 'int' && $sizeSuffix = $field['numeric_precision']) { $field['data_type'] .= "($sizeSuffix)"; } - if ($field['is_nullable'] == 'YES') { $field['data_type'] .= ' null'; } else { $field['data_type'] .= ' not null'; } + if ($field['column_default']) { - $default=substr($field['column_default'], 2, -2); + $default = substr($field['column_default'], 2, -2); $field['data_type'] .= " default $default"; } + break; case 'decimal': @@ -537,33 +539,40 @@ public function fieldList($table) } else { $field['data_type'] .= ' not null'; } + if ($field['column_default']) { - $default=substr($field['column_default'], 2, -2); + $default = substr($field['column_default'], 2, -2); $field['data_type'] .= " default $default"; } + break; case 'nvarchar': case 'varchar': //Check to see if there's a constraint attached to this column: $clause = $this->getConstraintCheckClause($table, $field['column_name']); + if ($clause) { $constraints = $this->enumValuesFromCheckClause($clause); - $default=substr($field['column_default'], 2, -2); + $default = substr($field['column_default'], 2, -2); + $field['data_type'] = $this->enum(array( 'default' => $default, 'name' => $field['column_name'], 'enums' => $constraints, 'table' => $table )); + break; } default: $sizeSuffix = $field['character_maximum_length']; + if ($sizeSuffix == '-1') { $sizeSuffix = 'max'; } + if ($sizeSuffix) { $field['data_type'] .= "($sizeSuffix)"; } @@ -573,11 +582,14 @@ public function fieldList($table) } else { $field['data_type'] .= ' not null'; } - if ($field['column_default']) { - $default=substr($field['column_default'], 2, -2); + + if (isset($field['column_default']) && $field['column_default']) { + $default = substr($field['column_default'], 2, -2); + $field['data_type'] .= " default '$default'"; } } + $output[$field['column_name']] = $field; } @@ -664,11 +676,9 @@ public function indexList($table) $baseIndexName = $this->buildMSSQLIndexName($table, ''); $indexName = substr($index['index_name'], strlen($baseIndexName)); - // Extract columns - $columns = $this->quoteColumnSpecString($index['index_keys']); $indexList[$indexName] = [ 'name' => $indexName, - 'columns' => $columns, + 'columns' => $this->explodeColumnString($index['index_keys']), 'type' => $indexType ]; } @@ -688,7 +698,7 @@ public function indexList($table) if (!empty($columns)) { $indexList['SearchFields'] = [ 'name' => 'SearchFields', - 'columns' => $this->implodeColumnList($columns), + 'columns' => $columns, 'type' => 'fulltext' ]; } @@ -914,10 +924,10 @@ public function IdColumn($asDbValue = false, $hasAutoIncPK = true) public function hasTable($tableName) { - return (bool)$this->preparedQuery( - "SELECT table_name FROM information_schema.tables WHERE table_name = ?", - array($tableName) - )->value(); + $sqlTable = str_replace('_', '\\_', $this->database->quoteString($tableName)); + $has = $this->query("SELECT COUNT(*) FROM information_schema.tables WHERE table_name = $sqlTable")->value(); + + return ($has && $has > 0); } /** @@ -930,7 +940,7 @@ public function hasTable($tableName) */ public function enumValuesForField($tableName, $fieldName) { - $classes = array(); + $classes = []; // Get the enum of all page types from the SiteTree table $clause = $this->getConstraintCheckClause($tableName, $fieldName); @@ -942,11 +952,6 @@ public function enumValuesForField($tableName, $fieldName) } /** - * This is a lookup table for data types. - * - * For instance, MSSQL uses 'BIGINT', while MySQL uses 'UNSIGNED' - * and PostgreSQL uses 'INT'. - * * @param string $type * @return string */ @@ -955,6 +960,7 @@ public function dbDataType($type) $values = array( 'unsigned integer'=>'BIGINT' ); + if (isset($values[$type])) { return $values[$type]; } else { From fba320f98ffab7e506b08f8b150ae542c7701f76 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 15:53:16 +1300 Subject: [PATCH 07/16] FIX Index name generation --- src/MSSQLDatabase.php | 1 + src/MSSQLSchemaManager.php | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/MSSQLDatabase.php b/src/MSSQLDatabase.php index e914ecb..e2e3ba9 100644 --- a/src/MSSQLDatabase.php +++ b/src/MSSQLDatabase.php @@ -451,6 +451,7 @@ public function transactionRollback($savepoint = false) if (!$this->transactionNesting) { return false; } + --$this->transactionNesting; if ($this->transactionNesting > 0) { $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting); diff --git a/src/MSSQLSchemaManager.php b/src/MSSQLSchemaManager.php index bb5eab2..74d5bb6 100644 --- a/src/MSSQLSchemaManager.php +++ b/src/MSSQLSchemaManager.php @@ -241,11 +241,13 @@ public function alterTable($tableName, $newFields = null, $newIndexes = null, $a $alterList[] = $this->alterTableAlterColumn($tableName, $k, $v); } } + if ($alteredIndexes) { foreach ($alteredIndexes as $k => $v) { $alterList[] = $this->getIndexSqlDefinition($tableName, $k, $v); } } + if ($newIndexes) { foreach ($newIndexes as $k => $v) { $alterList[] = $this->getIndexSqlDefinition($tableName, $k, $v); @@ -968,8 +970,13 @@ public function dbDataType($type) } } - protected function indexKey($table, $index, $spec) + /** + * @param string $tableName + * @param string $indexName + * @param mixed $spec + */ + protected function indexKey($tableName, $indexName, $spec) { - return $index; + return $this->buildMSSQLIndexName($tableName, $indexName); } } From 9f9da77908b85638076df87583ac3a44ffeb79c6 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 19:09:46 +1300 Subject: [PATCH 08/16] FIX indexSpec always reporting a change --- src/MSSQLDatabaseConfigurationHelper.php | 2 ++ src/MSSQLSchemaManager.php | 31 +++++++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/MSSQLDatabaseConfigurationHelper.php b/src/MSSQLDatabaseConfigurationHelper.php index cb19203..e3ac96c 100644 --- a/src/MSSQLDatabaseConfigurationHelper.php +++ b/src/MSSQLDatabaseConfigurationHelper.php @@ -47,7 +47,9 @@ protected function createConnection($databaseConfig, &$error) $parameters['database'] = $databaseConfig['database']; $parameters['multipleactiveresultsets'] = 0; } + $conn = @sqlsrv_connect($databaseConfig['server'], $parameters); + if ($conn) { return $conn; } diff --git a/src/MSSQLSchemaManager.php b/src/MSSQLSchemaManager.php index 74d5bb6..79e8cc1 100644 --- a/src/MSSQLSchemaManager.php +++ b/src/MSSQLSchemaManager.php @@ -609,6 +609,27 @@ public function createIndex($tableName, $indexName, $indexSpec) $this->query($this->getIndexSqlDefinition($tableName, $indexName, $indexSpec)); } + + /** + * This takes the index spec which has been provided by a class (ie static $indexes = blah blah) + * and turns it into a proper string. + * Some indexes may be arrays, such as fulltext and unique indexes, and this allows database-specific + * arrays to be created. See {@link requireTable()} for details on the index format. + * + * @see http://dev.mysql.com/doc/refman/5.0/en/create-index.html + * @see parseIndexSpec() for approximate inverse + * + * @param string|array $indexSpec + * @return string + */ + protected function convertIndexSpec($indexSpec) + { + $indexSpec['type'] = trim($indexSpec['type']); + $spec = parent::convertIndexSpec($indexSpec); + + return $spec; + } + /** * Return SQL for dropping and recreating an index * @@ -675,10 +696,10 @@ public function indexList($table) } // Extract name from index - $baseIndexName = $this->buildMSSQLIndexName($table, ''); + $baseIndexName = $index['index_name']; $indexName = substr($index['index_name'], strlen($baseIndexName)); - $indexList[$indexName] = [ + $indexList[$baseIndexName] = [ 'name' => $indexName, 'columns' => $this->explodeColumnString($index['index_keys']), 'type' => $indexType @@ -722,16 +743,18 @@ public function indexNames($tableName) SELECT ind.name FROM sys.indexes ind INNER JOIN sys.tables t ON ind.object_id = t.object_id WHERE is_primary_key = 0 AND t.name = ? AND ind.name IS NOT NULL', - array($tableName) + [$tableName] )->column(); } public function tableList() { - $tables = array(); + $tables = []; + foreach ($this->query("EXEC sp_tables @table_owner = 'dbo';") as $record) { $tables[strtolower($record['TABLE_NAME'])] = $record['TABLE_NAME']; } + return $tables; } From 6f4ef94829f39910571425385b52075e550d8849 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 19:10:40 +1300 Subject: [PATCH 09/16] FIX Handle IDENTITY_INSERT errors automagically --- composer.json | 66 ++++++++++++++++++++------------------ src/SQLServerConnector.php | 15 +++++++++ 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/composer.json b/composer.json index d1cf319..c04f7dc 100644 --- a/composer.json +++ b/composer.json @@ -1,35 +1,39 @@ { - "name": "silverstripe/mssql", - "description": "Adds MSSQL support to SilverStripe", - "type": "silverstripe-vendormodule", - "keywords": ["silverstripe", "mssql", "database"], - "authors": [ - { - "name": "Sam Minnee", - "email": "sam@silverstripe.com" - }, - { - "name": "Sean Harvey", - "email": "sean@silverstripe.com" - } - ], - "require": { - "silverstripe/framework": "^4" - }, - "suggest": { + "name": "silverstripe/mssql", + "description": "Adds MSSQL support to SilverStripe", + "type": "silverstripe-vendormodule", + "keywords": ["silverstripe", "mssql", "database"], + "authors": [ + { + "name": "Sam Minnee", + "email": "sam@silverstripe.com" + }, + { + "name": "Sean Harvey", + "email": "sean@silverstripe.com" + } + ], + "require": { + "silverstripe/framework": "^4" + }, + "suggest": { "ext-sqlsrv": "Required to support MSSQLDatabase as the server type", "ext-pdo_sqlsrv": "Required to support MSSQLPDODatabase as the server type" - }, - "extra": { - "branch-alias": { - "dev-master": "3.x-dev" - } - }, - "autoload": { - "psr-4": { - "SilverStripe\\MSSQL\\": "src/" - } - }, - "prefer-stable": true, - "minimum-stability": "dev" + }, + "scripts": { + "lint": "phpcs src/ tests/php/", + "lint-clean": "phpcbf src/ tests/php/" + }, + "extra": { + "branch-alias": { + "dev-master": "3.x-dev" + } + }, + "autoload": { + "psr-4": { + "SilverStripe\\MSSQL\\": "src/" + } + }, + "prefer-stable": true, + "minimum-stability": "dev" } diff --git a/src/SQLServerConnector.php b/src/SQLServerConnector.php index ee15bf8..9913903 100644 --- a/src/SQLServerConnector.php +++ b/src/SQLServerConnector.php @@ -154,6 +154,21 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) // Check for error if (!$handle) { + $error = $this->getLastError(); + + if (preg_match("/Cannot insert explicit value for identity column in table '(.*)'/", $error, $matches)) { + sqlsrv_query($this->dbConn, "SET IDENTITY_INSERT \"$matches[1]\" ON"); + $result = $this->preparedQuery($sql, $parameters, $errorLevel); + + if ($result) { + sqlsrv_query($this->dbConn, "SET IDENTITY_INSERT \"$matches[1]\" OFF"); + + return $result; + } else { + return null; + } + } + $this->databaseError($this->getLastError(), $errorLevel, $sql, $parsedParameters); return null; } From 08ed7b506f6307b68718a3cf3fbe4bdb893b6009 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Mon, 12 Oct 2020 21:44:13 +1300 Subject: [PATCH 10/16] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d260094..aceb51c 100644 --- a/README.md +++ b/README.md @@ -3,8 +3,8 @@ Allows SilverStripe to use SQL Server databases including SQL databases on Azure. [![Build status](https://ci.appveyor.com/api/projects/status/hep0l5kbhu64n7l3/branch/master?svg=true)](https://ci.appveyor.com/project/silverstripe/silverstripe-mssql/branch/master) -[![Version](http://img.shields.io/packagist/v/silverstripe/silverstripe-mssql.svg?style=flat-square)](https://packagist.org/packages/silverstripe/silverstripe-mssql) -[![License](http://img.shields.io/packagist/l/silverstripe/silverstripe-mssql.svg?style=flat-square)](LICENSE) +[![Version](http://img.shields.io/packagist/v/silverstripe/mssql.svg?style=flat-square)](https://packagist.org/packages/silverstripe/mssql) +[![License](http://img.shields.io/packagist/l/silverstripe/mssql.svg?style=flat-square)](LICENSE) ## Requirements From 1c35624dd3994ccaa989a1a6b3b4dbb71ad28990 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 12:24:46 +1300 Subject: [PATCH 11/16] Add license key --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index c04f7dc..5f782c6 100644 --- a/composer.json +++ b/composer.json @@ -2,6 +2,7 @@ "name": "silverstripe/mssql", "description": "Adds MSSQL support to SilverStripe", "type": "silverstripe-vendormodule", + "license": "BSD-3-Clause", "keywords": ["silverstripe", "mssql", "database"], "authors": [ { From d188b7e784a5912e719ff7fb66e875edc70a7b87 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 14:42:09 +1300 Subject: [PATCH 12/16] FIX seek() not returning a valid record --- src/SQLServerQuery.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SQLServerQuery.php b/src/SQLServerQuery.php index 55c4fcf..0b4bb57 100644 --- a/src/SQLServerQuery.php +++ b/src/SQLServerQuery.php @@ -74,12 +74,12 @@ public function nextRecord() public function seek($row) { - if (is_object($this->handle)) { - sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); - $result = $this->nextRecord(); - sqlsrv_fetch($this->handle, SQLSRV_SCROLL_ABSOLUTE, $row); + if (is_resource($this->handle)) { + $result = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC, SQLSRV_SCROLL_ABSOLUTE, $row); + return $result; } + return null; } } From 732553c57c1be94e87d29fa3f996d1f287e6ca96 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 15:30:50 +1300 Subject: [PATCH 13/16] FIX Correctly return false on no results rather than empty array --- src/SQLServerConnector.php | 7 ++++++- src/SQLServerQuery.php | 18 +++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/SQLServerConnector.php b/src/SQLServerConnector.php index 9913903..f6aa4bf 100644 --- a/src/SQLServerConnector.php +++ b/src/SQLServerConnector.php @@ -144,7 +144,11 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) $this->lastAffectedRows = 0; // Run query - $parsedParameters = $this->parameterValues($parameters); + if ($parameters) { + $parsedParameters = $this->parameterValues($parameters); + } else { + $parsedParameters = []; + } if (empty($parsedParameters)) { $handle = sqlsrv_query($this->dbConn, $sql); @@ -170,6 +174,7 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) } $this->databaseError($this->getLastError(), $errorLevel, $sql, $parsedParameters); + return null; } diff --git a/src/SQLServerQuery.php b/src/SQLServerQuery.php index 0b4bb57..8f4b69d 100644 --- a/src/SQLServerQuery.php +++ b/src/SQLServerQuery.php @@ -50,7 +50,7 @@ public function __destruct() public function numRecords() { if (!is_resource($this->handle)) { - return false; + return null; } // WARNING: This will only work if the cursor type is scrollable! @@ -64,12 +64,14 @@ public function numRecords() public function nextRecord() { if (is_resource($this->handle)) { - $row = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC); + $result = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC); - return $row; - } else { - return false; + if ($result && !empty($result)) { + return $result; + } } + + return false; } public function seek($row) @@ -77,9 +79,11 @@ public function seek($row) if (is_resource($this->handle)) { $result = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC, SQLSRV_SCROLL_ABSOLUTE, $row); - return $result; + if ($result && !empty($result)) { + return $result; + } } - return null; + return false; } } From cc80227614d28fbdd9fad512714f9cde7c377ff8 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 15:55:54 +1300 Subject: [PATCH 14/16] FIX MultipleActiveResultSets is required --- src/MSSQLAzureDatabase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MSSQLAzureDatabase.php b/src/MSSQLAzureDatabase.php index ed97ae2..f9611a4 100644 --- a/src/MSSQLAzureDatabase.php +++ b/src/MSSQLAzureDatabase.php @@ -57,7 +57,7 @@ protected function connectDatabase($database) { $parameters = $this->parameters; $parameters['database'] = $database; - $parameters['multipleactiveresultsets'] = 0; + $parameters['multipleactiveresultsets'] = 1; // Ensure that driver is available (required by PDO) if (empty($parameters['driver'])) { From 0cc9a140cd25e9cf48d5e7ec451a696872872923 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 16:08:27 +1300 Subject: [PATCH 15/16] FIX Silverstripe expects dates as strings --- src/MSSQLAzureDatabase.php | 1 + src/MSSQLDatabaseConfigurationHelper.php | 3 ++- src/SQLServerConnector.php | 15 +++++++++------ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/MSSQLAzureDatabase.php b/src/MSSQLAzureDatabase.php index f9611a4..043b6d4 100644 --- a/src/MSSQLAzureDatabase.php +++ b/src/MSSQLAzureDatabase.php @@ -57,6 +57,7 @@ protected function connectDatabase($database) { $parameters = $this->parameters; $parameters['database'] = $database; + $parameters['returndatesasstrings'] = 1; $parameters['multipleactiveresultsets'] = 1; // Ensure that driver is available (required by PDO) diff --git a/src/MSSQLDatabaseConfigurationHelper.php b/src/MSSQLDatabaseConfigurationHelper.php index e3ac96c..e71da73 100644 --- a/src/MSSQLDatabaseConfigurationHelper.php +++ b/src/MSSQLDatabaseConfigurationHelper.php @@ -45,7 +45,8 @@ protected function createConnection($databaseConfig, &$error) // Azure has additional parameter requirements if ($this->isAzure($databaseConfig)) { $parameters['database'] = $databaseConfig['database']; - $parameters['multipleactiveresultsets'] = 0; + $parameters['multipleactiveresultsets'] = 1; + $parameters['returndatesasstrings'] = 1; } $conn = @sqlsrv_connect($databaseConfig['server'], $parameters); diff --git a/src/SQLServerConnector.php b/src/SQLServerConnector.php index f6aa4bf..91c11b5 100644 --- a/src/SQLServerConnector.php +++ b/src/SQLServerConnector.php @@ -47,13 +47,16 @@ public function connect($parameters, $selectDB = false) } $charset = isset($parameters['charset']) ? $parameters : 'UTF-8'; - $multiResultSets = isset($parameters['multipleactiveresultsets']) - ? $parameters['multipleactiveresultsets'] - : true; - $options = array( + + $options = [ 'CharacterSet' => $charset, - 'MultipleActiveResultSets' => $multiResultSets - ); + 'ReturnDatesAsStrings' => isset($parameters['returndatesasstrings']) + ? $parameters['returndatesasstrings'] + : true, + 'MultipleActiveResultSets' => isset($parameters['multipleactiveresultsets']) + ? $parameters['multipleactiveresultsets'] + : true + ]; if (!(defined('MSSQL_USE_WINDOWS_AUTHENTICATION') && MSSQL_USE_WINDOWS_AUTHENTICATION == true) && empty($parameters['windowsauthentication']) From 367fc172e87e209175d83911a4230a7d0654b666 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 13 Oct 2020 16:52:56 +1300 Subject: [PATCH 16/16] FIX ORDER BY cannot be part of a subselect --- src/MSSQLQueryBuilder.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/MSSQLQueryBuilder.php b/src/MSSQLQueryBuilder.php index 8a251d8..052ce9d 100644 --- a/src/MSSQLQueryBuilder.php +++ b/src/MSSQLQueryBuilder.php @@ -5,6 +5,8 @@ use InvalidArgumentException; use SilverStripe\ORM\Queries\SQLSelect; use SilverStripe\ORM\Connect\DBQueryBuilder; +use SilverStripe\ORM\Queries\SQLConditionalExpression; +use SilverStripe\ORM\Queries\SQLUpdate; /** * Builds a SQL query string from a SQLExpression object @@ -69,6 +71,7 @@ protected function buildSelectQuery(SQLSelect $query, array &$parameters) $parameters[] = $offset + 1; $parameters[] = $offset + $limit; } + return "SELECT * FROM ($sql) AS Numbered WHERE $limitCondition ORDER BY Number"; } @@ -88,6 +91,21 @@ public function buildOrderByFragment(SQLSelect $query, array &$parameters) return ''; } + public function buildWhereFragment(SQLConditionalExpression $query, array &$parameters) + { + // Get parameterised elements + $where = parent::buildWhereFragment($query, $parameters); + + if (!$where) { + return ''; + } + + // remove explict order in where + $where = preg_replace("/ORDER BY ([^\)])+ [ASC|DESC]+/", '', $where); + + return $where; + } + /** * Extracts the limit and offset from the limit clause * @@ -99,6 +117,7 @@ protected function parseLimit(SQLSelect $query) { $limit = ''; $offset = '0'; + if (is_array($query->getLimit())) { $limitArr = $query->getLimit(); if (isset($limitArr['limit'])) { @@ -119,6 +138,7 @@ protected function parseLimit(SQLSelect $query) $limit = $bits[0]; } } + return array($limit, $offset); } }