From 75f4a35f712144aaa23d796fbf2de7d3614bd486 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Fri, 28 Jun 2019 14:41:59 +1200 Subject: [PATCH 1/6] =?UTF-8?q?FIX:=20Don=E2=80=99t=20drop=20first=20row?= =?UTF-8?q?=20on=20repeated=20iteration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/silverstripe/silverstripe-framework/issues/9097 Related: https://github.com/silverstripe/silverstripe-framework/issues/9098 Co-authored-by: Guy Marriott --- code/PostgreSQLQuery.php | 51 ++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/code/PostgreSQLQuery.php b/code/PostgreSQLQuery.php index 0b96c61..f010a11 100644 --- a/code/PostgreSQLQuery.php +++ b/code/PostgreSQLQuery.php @@ -58,8 +58,10 @@ class PostgreSQLQuery extends Query public function seek($row) { - pg_result_seek($this->handle, $row); - return $this->nextRecord(); + // Specifying the zero-th record here will reset the pointer + $result = pg_fetch_array($this->handle, $row, PGSQL_NUM); + + return $this->parseResult($result); } public function numRecords() @@ -73,26 +75,35 @@ class PostgreSQLQuery extends Query // Correct non-string types if ($row) { - $record = []; - - foreach ($row as $i => $v) { - $k = $this->columnNames[$i]; - $record[$k] = $v; - $type = pg_field_type($this->handle, $i); - if (isset(self::$typeMapping[$type])) { - if ($type === 'bool' && $record[$k] === 't') { - $record[$k] = 1; - - // Note that boolean 'f' will be converted to 0 by this - } else { - settype($record[$k], self::$typeMapping[$type]); - } - } - } - - return $record; + return $this->parseResult($row); } return false; } + + /** + * @param array $row + * @return array + */ + protected function parseResult(array $row) + { + $record = []; + + foreach ($row as $i => $v) { + $k = $this->columnNames[$i]; + $record[$k] = $v; + $type = pg_field_type($this->handle, $i); + if (isset(self::$typeMapping[$type])) { + if ($type === 'bool' && $record[$k] === 't') { + $record[$k] = 1; + + // Note that boolean 'f' will be converted to 0 by this + } else { + settype($record[$k], self::$typeMapping[$type]); + } + } + } + + return $record; + } } From 66376db094434403b340fa21545b98bf97a40a83 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 3 Sep 2019 15:22:14 +1200 Subject: [PATCH 2/6] DOCS Known issue about collations Moving from https://docs.silverstripe.org/en/4/getting_started/server_requirements/ since it's too much noise there. Also removing the maintainer contact, that's an outdated concept. --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 82b290d..5000b06 100644 --- a/README.md +++ b/README.md @@ -3,16 +3,11 @@ [![Build Status](https://travis-ci.org/silverstripe/silverstripe-postgresql.png?branch=master)](https://travis-ci.org/silverstripe/silverstripe-postgresql) [![SilverStripe supported module](https://img.shields.io/badge/silverstripe-supported-0071C4.svg)](https://www.silverstripe.org/software/addons/silverstripe-commercially-supported-module-list/) -## Maintainer Contact - - * Sam Minnee (Nickname: sminnee) - ## Requirements * SilverStripe 4.0 * PostgreSQL 8.3.x or greater must be installed * PostgreSQL <8.3.0 may work if T-Search is manually installed -* Known to work on OS X Leopard, Windows Server 2008 R2 and Linux ## Installation @@ -30,6 +25,12 @@ See docs/en for more information about configuring the module. All column and table names must be double-quoted. PostgreSQL automatically lower-cases columns, and your queries will fail if you don't. +Collations have known issues when installed on Alpine, MacOS X and BSD derivatives +(see [PostgreSQL FAQ](https://wiki.postgresql.org/wiki/FAQ#Why_do_my_strings_sort_incorrectly.3F)). +We do not support such installations, although they still may work correctly for you. +As a workaround for PostgreSQL 10+ you could manually switch to ICU collations (e.g. und-x-icu). +There are no known workarounds for PostgreSQL <10. + Ts_vector columns are not automatically detected by the built-in search filters. That means if you're doing a search through the CMS on a ModelAdmin object, it will use LIKE queries which are very slow. If you're writing your From bf4fb87a01cd376a99e9e22c2680fde1d49eaead Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 3 Sep 2019 15:41:23 +1200 Subject: [PATCH 3/6] API Increased support to 9.2 Anything older than 9.3 is unsupported by Postgres: https://www.postgresql.org/support/versioning/. I don't think we should claim support in our module for unsupported versions, particularly if we don't have CI on them. Since we're *only* running CI on 9.2 at the moment, that's the safe claim for our module support, even though it's already unsupported by Postgres. See https://docs.travis-ci.com/user/database-setup/#using-a-different-postgresql-version I'll raise a separate ticket about testing and supporting newer versions, it's out of scope for this PR. --- README.md | 33 +++++++++++---- docs/en/README.md | 101 +--------------------------------------------- 2 files changed, 26 insertions(+), 108 deletions(-) diff --git a/README.md b/README.md index 82b290d..a49c7ec 100644 --- a/README.md +++ b/README.md @@ -10,20 +10,37 @@ ## Requirements * SilverStripe 4.0 -* PostgreSQL 8.3.x or greater must be installed -* PostgreSQL <8.3.0 may work if T-Search is manually installed -* Known to work on OS X Leopard, Windows Server 2008 R2 and Linux +* PostgreSQL >=9.2 +* Note: PostgreSQL 10 has not been tested ## Installation - 1. Install via composer `composer require silverstripe/postgresql` or extract the contents - so they reside as a `postgresql` directory inside your SilverStripe project code - 2. Open the installer by browsing to install.php, e.g. http://localhost/silverstripe/install.php - 3. Select PostgreSQL in the database list and enter your database details +``` +composer require silverstripe/postgresql +``` + +## Configuration + +### Environment file + +Add the following settings to your `.env` file: + +``` +SS_DATABASE_CLASS=PostgreSQLDatabase +SS_DATABASE_USERNAME= +SS_DATABASE_PASSWORD= +``` + +See [environment variables](https://docs.silverstripe.org/en/4/getting_started/environment_management) for more details. Note that a database will automatically be created via `dev/build`. + +### Through the installer + +Open the installer by browsing to install.php, e.g. http://localhost/install.php +Select PostgreSQL in the database list and enter your database details ## Usage Overview -See docs/en for more information about configuring the module. +See [docs/en](docs/en/README.md) for more information about configuring the module. ## Known issues diff --git a/docs/en/README.md b/docs/en/README.md index 1276517..6202473 100644 --- a/docs/en/README.md +++ b/docs/en/README.md @@ -1,86 +1,5 @@ # PostgreSQL Database Module -SilverStripe now has tentative support for PostgreSQL ('Postgres'). - -## Requirements - -SilverStripe 2.4.0 or greater. (PostgreSQL support is NOT available -in 2.3.). - -SilverStripe supports Postgres versions 8.3.x, 8.4.x and onwards. -Postgres 8.3.0 launched in February 2008, so SilverStripe has a fairly -modern but not bleeding edge Postgres version requirement. - -Support for 8.2.x is theoretically possible if you're willing to manually -install T-search. 8.2.x has not been tested either, so there may be other -compatibility issues. The EnterpriseDB versions of Postgres also work, if -you'd prefer a tuned version. - -## Installation - -You have three options to install PostgreSQL support with SilverStripe. - -### Option 1 - Installer - -The first option is to use the installer. However, this is currently only -supported since SilverStripe 2.4.0 beta2 (or using the daily builds). - -1. Set up SilverStripe somewhere where you can start the installer - you -should only see one database “MySQL” to install with. -2. Download a copy of the “postgresql” module from here: -http://silverstripe.org/postgresql-module -3. Extract the archive you downloaded. Rename the directory from -“postgresql-trunk-rxxxx” to “postgresql” and copy it into the SilverStripe -directory you just set up -4. Open the installer once again, and a new option “PostgreSQL” should appear. -You can now proceed through the installation without having to change any code. - -### Option 2 - Manual - -The second option is to setup PostgreSQL support manually. This can be achieved -by following these instructions: - -1. Set up a fresh working copy of SilverStripe -2. Download a copy of the “postgresql” module from here: http://silverstripe.org/postgresql-module -3. Extract the archive you downloaded. Rename the directory from -“postgresql-trunk-rxxxx” to “postgresql” and copy it into the SilverStripe -directory you just set up. -4. Open up your mysite/_config.php file and add (or update) the $databaseConfig -array like so: - -> $databaseConfig = array( -> 'type' => 'PostgreSQLDatabase', -> 'server' => '[server address e.g. localhost]', -> 'username' => 'postgres', -> 'password' => 'mypassword', -> 'database' => 'SS_mysite' -> ); - -Finally, visit dev/build so that SilverStripe can build the database schema and -default records. - -### Option 3 - Environment file - -Finally, the third option is to change your environment to point to -PostgreSQLDatabase as a database class. Do this if you're currently using an - _ss_environment.php file. - -1. Download a copy of the “postgresql” module from here: http://silverstripe.org/postgresql-module -2. Extract the archive you downloaded. Rename the directory from -postgresql-trunk-rxxxx” to “postgresql” and copy it into your SS directory -3. Add the following to your existing _ss_environment.php file: - -> define('SS_DATABASE_CLASS', 'PostgreSQLDatabase'); - -Last steps: - -1. Ensure your SS_DATABASE_USERNAME and SS_DATABASE_PASSWORD defines in -_ss_environment.php are correct to the PostgreSQL server. -2. Ensure that your mysite/_config.php file has a database name defined, such -as “SS_mysite”. -3. Visit dev/build so that SilverStripe can build the database schema and -default records - ## Features Here is a quick list of what's different in the Postgres module (a full @@ -300,22 +219,4 @@ Otherwise this extension will try to connect to "postgres" Database to check DB connection, no matter what you entered in the "Database Name" field during installation. -Make sure you have set the "search_path" correct for your database user. - -## Known Issues - -All column and table names must be double-quoted. PostgreSQL automatically -lower-cases columns, and your queries will fail if you don't. - -Ts_vector columns are not automatically detected by the built-in search filters. -That means if you're doing a search through the CMS on a ModelAdmin object, it -will use LIKE queries which are very slow. - -If you're writing your own front-end search system, you can specify the columns -to use for search purposes, and you get the full benefits of T-Search. - -If you are using unsupported modules, there may be instances of MySQL-specific -SQL queries which will need to be made database-agnostic where possible. - - - +Make sure you have set the "search_path" correct for your database user. \ No newline at end of file From 753d73e1feb5d508bbe34b86e6cf809c43b70335 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Thu, 26 Sep 2019 10:25:41 +1200 Subject: [PATCH 4/6] BUG Fix issues preventing a site from being migrated from SS3 to SS4 (#104) * BUG Enum value change wasn't being detected by alterTableAlterColumn because backslashes were not accounting * BUG Update renameTable to also rename constraints * BUG Add unit tests to cover requireTable and renameTable * Fix liniting errors * MINOR Update build to use xenial and add extra PHP version --- .travis.yml | 12 ++- code/PostgreSQLSchemaManager.php | 45 ++++++++- tests/PostgreSQLSchemaManagerTest.php | 138 ++++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 5 deletions(-) create mode 100644 tests/PostgreSQLSchemaManagerTest.php diff --git a/.travis.yml b/.travis.yml index fb44e1a..a89dda4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,10 @@ language: php -dist: trusty +dist: xenial + +services: + - postgresql + - cache: directories: @@ -19,6 +23,12 @@ matrix: - php: 7.1 env: - PHPUNIT_TEST=framework + - php: 7.2 + env: + - PHPUNIT_TEST=framework + - php: 7.3 + env: + - PHPUNIT_TEST=framework - php: 7.1 env: - PHPUNIT_TEST=postgresql diff --git a/code/PostgreSQLSchemaManager.php b/code/PostgreSQLSchemaManager.php index 37f3563..9c5b678 100644 --- a/code/PostgreSQLSchemaManager.php +++ b/code/PostgreSQLSchemaManager.php @@ -494,9 +494,15 @@ class PostgreSQLSchemaManager extends DBSchemaManager // First, we split the column specifications into parts // TODO: this returns an empty array for the following string: int(11) not null auto_increment // on second thoughts, why is an auto_increment field being passed through? - - $pattern = '/^([\w(\,)]+)\s?((?:not\s)?null)?\s?(default\s[\w\.\']+)?\s?(check\s[\w()\'",\s]+)?$/i'; + $pattern = '/^([\w(\,)]+)\s?((?:not\s)?null)?\s?(default\s[\w\.\'\\\\]+)?\s?(check\s[\w()\'",\s\\\\]+)?$/i'; preg_match($pattern, $colSpec, $matches); + // example value this regex is expected to parse: + // varchar(255) not null default 'SS\Test\Player' check ("ClassName" in ('SS\Test\Player', 'Player', null)) + // split into: + // * varchar(255) + // * not null + // * default 'SS\Test\Player' + // * check ("ClassName" in ('SS\Test\Player', 'Player', null)) if (sizeof($matches) == 0) { return ''; @@ -537,8 +543,8 @@ class PostgreSQLSchemaManager extends DBSchemaManager if ($this->hasTable("{$tableName}_Live")) { $updateConstraint .= "UPDATE \"{$tableName}_Live\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);"; } - if ($this->hasTable("{$tableName}_versions")) { - $updateConstraint .= "UPDATE \"{$tableName}_versions\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);"; + if ($this->hasTable("{$tableName}_Versions")) { + $updateConstraint .= "UPDATE \"{$tableName}_Versions\" SET \"$colName\"='$default' WHERE \"$colName\" NOT IN ($constraint_values);"; } $this->query($updateConstraint); @@ -560,8 +566,17 @@ class PostgreSQLSchemaManager extends DBSchemaManager public function renameTable($oldTableName, $newTableName) { + $constraints = $this->getConstraintForTable($oldTableName); $this->query("ALTER TABLE \"$oldTableName\" RENAME TO \"$newTableName\""); + + if ($constraints) { + foreach ($constraints as $old) { + $new = preg_replace('/^' . $oldTableName . '/', $newTableName, $old); + $this->query("ALTER TABLE \"$newTableName\" RENAME CONSTRAINT \"$old\" TO \"$new\";"); + } + } unset(self::$cached_fieldlists[$oldTableName]); + unset(self::$cached_constraints[$oldTableName]); } public function checkAndRepairTable($tableName) @@ -961,6 +976,28 @@ class PostgreSQLSchemaManager extends DBSchemaManager return self::$cached_constraints[$constraint]; } + /** + * Retrieve a list of constraints for the provided table name. + * @param string $tableName + * @return array + */ + private function getConstraintForTable($tableName) + { + // Note the PostgreSQL `like` operator is case sensitive + $constraints = $this->preparedQuery( + " + SELECT conname + FROM pg_catalog.pg_constraint r + INNER JOIN pg_catalog.pg_namespace n + ON r.connamespace = n.oid + WHERE r.contype = 'c' AND conname like ? AND n.nspname = ? + ORDER BY 1;", + array($tableName . '_%', $this->database->currentSchema()) + )->column('conname'); + + return $constraints; + } + /** * A function to return the field names and datatypes for the particular table * diff --git a/tests/PostgreSQLSchemaManagerTest.php b/tests/PostgreSQLSchemaManagerTest.php new file mode 100644 index 0000000..1ad1fd2 --- /dev/null +++ b/tests/PostgreSQLSchemaManagerTest.php @@ -0,0 +1,138 @@ +quiet(); + + $this->createSS3Table(); + + try { + DB::query('INSERT INTO "ClassNamesUpgrade" ("ClassName") VALUES (\'App\MySite\FooBar\')'); + $this->assertFalse(true, 'SS3 Constaint should have blocked the previous insert.'); + } catch (DatabaseException $ex) { + } + + $dbSchema->schemaUpdate(function () use ($dbSchema) { + $dbSchema->requireTable( + 'ClassNamesUpgrade', + [ + 'ID' => 'PrimaryKey', + 'ClassName' => 'Enum(array("App\\\\MySite\\\\FooBar"))', + ] + ); + }); + + DB::query('INSERT INTO "ClassNamesUpgrade" ("ClassName") VALUES (\'App\MySite\FooBar\')'); + $count = DB::query('SELECT count(*) FROM "ClassNamesUpgrade" WHERE "ClassName" = \'App\MySite\FooBar\'') + ->value(); + + $this->assertEquals(1, $count); + } finally { + DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade"'); + DB::query('DROP SEQUENCE IF EXISTS "ClassNamesUpgrade_ID_seq"'); + } + } + + private function createSS3Table() + { + DB::query(<<quiet(); + + $this->createSS3VersionedTable(); + + $this->assertConstraintCount(1, 'ClassNamesUpgrade_versioned_ClassName_check'); + + $dbSchema->schemaUpdate(function () use ($dbSchema) { + $dbSchema->renameTable( + 'ClassNamesUpgrade_versioned', + 'ClassNamesUpgrade_Versioned' + ); + }); + + $this->assertTableCount(0, 'ClassNamesUpgrade_versioned'); + $this->assertTableCount(1, 'ClassNamesUpgrade_Versioned'); + $this->assertConstraintCount(0, 'ClassNamesUpgrade_versioned_ClassName_check'); + $this->assertConstraintCount(1, 'ClassNamesUpgrade_Versioned_ClassName_check'); + } finally { + DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade_Versioned"'); + DB::query('DROP TABLE IF EXISTS "ClassNamesUpgrade_versioned"'); + DB::query('DROP SEQUENCE IF EXISTS "ClassNamesUpgrade_versioned_ID_seq"'); + } + } + + private function assertConstraintCount($expected, $constraintName) + { + $count = DB::prepared_query( + 'SELECT count(*) FROM pg_catalog.pg_constraint WHERE conname like ?', + [$constraintName] + )->value(); + + $this->assertEquals($expected, $count); + } + + private function assertTableCount($expected, $tableName) + { + $count = DB::prepared_query( + 'SELECT count(*) FROM pg_catalog.pg_tables WHERE "tablename" like ?', + [$tableName] + )->value(); + + $this->assertEquals($expected, $count); + } + + private function createSS3VersionedTable() + { + DB::query(<< Date: Wed, 2 Oct 2019 14:45:33 +1300 Subject: [PATCH 5/6] Remove branch alias for 2.3.0 release --- composer.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 4167b2f..a2cd095 100644 --- a/composer.json +++ b/composer.json @@ -21,11 +21,7 @@ "require-dev": { "phpunit/phpunit": "^5.7" }, - "extra": { - "branch-alias": { - "2.x-dev": "2.3.x-dev" - } - }, + "extra": { }, "autoload": { "psr-4": { "SilverStripe\\PostgreSQL\\": "code/", From 3e38f845e3bea931b50b7c898445d8a6f57cccdb Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Wed, 2 Oct 2019 14:47:29 +1300 Subject: [PATCH 6/6] Bump branch alias to 2.4. --- composer.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index a2cd095..8660c7b 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,11 @@ "require-dev": { "phpunit/phpunit": "^5.7" }, - "extra": { }, + "extra": { + "branch-alias": { + "2.x-dev": "2.4.x-dev" + } + }, "autoload": { "psr-4": { "SilverStripe\\PostgreSQL\\": "code/",