From 5c0deabe5f7638c87abf02dcea3f06949da5ac5b Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sat, 14 Oct 2023 16:00:03 +0300 Subject: [PATCH 1/8] indexing virtual fields with dbal --- app/config/schema/schema.json | 3 ++- app/src/Lib/Util/SchemaManager.php | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index bae9ccfdc..b1f53456d 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -442,7 +442,8 @@ "identifiers_i1": { "columns": [ "identifier", "type_id", "person_id" ] }, "identifiers_i2": { "columns": [ "identifier", "type_id", "external_identity_id" ] }, "identifiers_i3": { "columns": [ "type_id" ] }, - "identifiers_i4": { "needed": false, "columns": [ "provisioning_target_id" ] } + "identifiers_i4": { "needed": false, "columns": [ "provisioning_target_id" ] }, + "identifiers_i5": { "columns": [ "lower(identifier)", "type_id" ] } }, "mvea": [ "person", "external_identity", "group" ], "sourced": true diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index 1819ecf86..ef3686850 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -261,12 +261,31 @@ protected function processSchema( // $flags and $options as passed to Index(), but otherwise undocumented $flags = []; $options = []; + + // XXX DBAL does not support expression indexes. We will trick DBAL by + // temporary adding the virtual column in the table. This will make + // no difference to the table end result since the actual query list + // will be executed later. What we actually do is treat the expression + // as a temporary virtual column + // https://www.postgresql.org/docs/14/indexes-expressional.html + $tempColumns = []; + foreach ($iCfg->columns as $clm_name) { + if (! $table->hasColumn($clm_name)) { + $table->addColumn($clm_name, "string"); + $tempColumns[] = $clm_name; + } + } if(isset($iCfg->unique) && $iCfg->unique) { $table->addUniqueConstraint($iCfg->columns, $iName, $flags, $options); } else { $table->addIndex($iCfg->columns, $iName, $flags, $options); } + + foreach ($tempColumns as $clm_name) { + $table->dropColumn($clm_name); + } + $tempColumns = []; } } From c2b637c43cb70463476f5335ef8576d0045f2c6d Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 22 Oct 2023 11:42:25 +0300 Subject: [PATCH 2/8] override index recovery for PostgreSQL databases --- app/config/schema/schema.json | 2 +- .../Lib/Util/CmgPostgreSQLSchemaManager.php | 107 ++++++++++++++++++ app/src/Lib/Util/CmgSchemaManagerFactory.php | 34 ++++++ app/src/Lib/Util/SchemaManager.php | 9 +- 4 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 app/src/Lib/Util/CmgPostgreSQLSchemaManager.php create mode 100644 app/src/Lib/Util/CmgSchemaManagerFactory.php diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index b1f53456d..9ea2d7a21 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -443,7 +443,7 @@ "identifiers_i2": { "columns": [ "identifier", "type_id", "external_identity_id" ] }, "identifiers_i3": { "columns": [ "type_id" ] }, "identifiers_i4": { "needed": false, "columns": [ "provisioning_target_id" ] }, - "identifiers_i5": { "columns": [ "lower(identifier)", "type_id" ] } + "identifiers_i5": { "columns": [ "lower(identifier::text)", "type_id" ] } }, "mvea": [ "person", "external_identity", "group" ], "sourced": true diff --git a/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php new file mode 100644 index 000000000..d557def5a --- /dev/null +++ b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php @@ -0,0 +1,107 @@ + $colName) { + $buffer[] = [ + 'key_name' => $row['relname'], + 'column_name' => trim($colName), + 'non_unique' => ! $row['indisunique'], + 'primary' => $row['indisprimary'], + 'where' => $row['where'], + ]; + } + } + + return AbstractSchemaManager::_getPortableTableIndexesList($buffer, $tableName); + } + + + protected function selectIndexColumns(string $databaseName, ?string $tableName = null): Result + { + $sql = 'SELECT'; + + if ($tableName === null) { + $sql .= ' tc.relname AS table_name, tn.nspname AS schema_name,'; + } + + $sql .= <<<'SQL' + quote_ident(ic.relname) AS relname, + i.indisunique, + i.indisprimary, + i.indkey, + i.indexprs, + i.indrelid, + ARRAY( + SELECT pg_get_indexdef(i.indexrelid, k + 1, true) + FROM generate_subscripts(i.indkey, 1) AS k + ORDER BY k + ) AS indkey_names, + pg_get_expr(indpred, indrelid) AS "where" + FROM pg_index i + JOIN pg_class AS tc ON tc.oid = i.indrelid + JOIN pg_namespace tn ON tn.oid = tc.relnamespace + JOIN pg_class AS ic ON ic.oid = i.indexrelid + WHERE ic.oid IN ( + SELECT indexrelid + FROM pg_index i, pg_class c, pg_namespace n +SQL; + + $conditions = array_merge([ + 'c.oid = i.indrelid', + 'c.relnamespace = n.oid', + ], $this->buildQueryConditions($tableName)); + + $sql .= ' WHERE ' . implode(' AND ', $conditions) . ')'; + + return $this->_conn->executeQuery($sql); + } + + /** + * @param string|null $tableName + * + * @return list + */ + private function buildQueryConditions($tableName): array + { + $conditions = []; + + if ($tableName !== null) { + if (strpos($tableName, '.') !== false) { + [$schemaName, $tableName] = explode('.', $tableName); + $conditions[] = 'n.nspname = ' . $this->_platform->quoteStringLiteral($schemaName); + } else { + $conditions[] = 'n.nspname = ANY(current_schemas(false))'; + } + + $identifier = new Identifier($tableName); + $conditions[] = 'c.relname = ' . $this->_platform->quoteStringLiteral($identifier->getName()); + } + + $conditions[] = "n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')"; + + return $conditions; + } +} + diff --git a/app/src/Lib/Util/CmgSchemaManagerFactory.php b/app/src/Lib/Util/CmgSchemaManagerFactory.php new file mode 100644 index 000000000..4d54e7fa3 --- /dev/null +++ b/app/src/Lib/Util/CmgSchemaManagerFactory.php @@ -0,0 +1,34 @@ +defaultFactory = new DefaultSchemaManagerFactory(); + } + + public function createSchemaManager(Connection $connection): AbstractSchemaManager + { + $platform = $connection->getDatabasePlatform(); + if ($platform instanceof PostgreSQL1000Platform + || $platform instanceof PostgreSQLPlatform) { + return new CmgPostgreSQLSchemaManager($connection, $platform); + } + + return $this->defaultFactory->createSchemaManager($connection); + } +} diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index ef3686850..ee53ab284 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -32,6 +32,7 @@ use Cake\Console\ConsoleIo; use Cake\Datasource\ConnectionInterface; use Cake\Datasource\ConnectionManager; +use App\Lib\Util\CmgSchemaManagerFactory; use Doctrine\DBAL\DriverManager; use Doctrine\DBAL\Schema\Comparator; @@ -79,7 +80,9 @@ public function __construct(?ConsoleIo $io=null, string $connection='default') { $cfg = $db->config(); $config = new \Doctrine\DBAL\Configuration(); - + // Load the COmanage custom CmgSchemaManagerFactory + $config->setSchemaManagerFactory(new CmgSchemaManagerFactory()); + $cfargs = [ 'dbname' => $cfg['database'], 'user' => $cfg['username'], @@ -325,12 +328,12 @@ protected function processSchema( // This is the SQL that represents the desired state of the database $toSql = $schema->toSql($this->conn->getDatabasePlatform()); - + // SchemaManager provides info about the database $sm = $this->conn->createSchemaManager(); // The is the current database representation - $curSchema = $sm->createSchema(); + $curSchema = $sm->introspectSchema(); $fromSql = $curSchema->toSql($this->conn->getDatabasePlatform()); From 3492096e7b377c8601791a3cb823d03ec52b859d Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 22 Oct 2023 21:09:53 +0300 Subject: [PATCH 3/8] Add MySQL support for expression indexes --- app/src/Lib/Util/CmgMySQLSchemaManager.php | 76 +++++++++++++++++++ .../Lib/Util/CmgPostgreSQLSchemaManager.php | 2 +- app/src/Lib/Util/CmgSchemaManagerFactory.php | 7 ++ app/src/Lib/Util/SchemaManager.php | 20 ++++- 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 app/src/Lib/Util/CmgMySQLSchemaManager.php diff --git a/app/src/Lib/Util/CmgMySQLSchemaManager.php b/app/src/Lib/Util/CmgMySQLSchemaManager.php new file mode 100644 index 000000000..384a31a48 --- /dev/null +++ b/app/src/Lib/Util/CmgMySQLSchemaManager.php @@ -0,0 +1,76 @@ + $v) { + $v = array_change_key_case($v, CASE_LOWER); + if ($v['key_name'] === 'PRIMARY') { + $v['primary'] = true; + } else { + $v['primary'] = false; + } + + if (strpos($v['index_type'], 'FULLTEXT') !== false) { + $v['flags'] = ['FULLTEXT']; + } elseif (strpos($v['index_type'], 'SPATIAL') !== false) { + $v['flags'] = ['SPATIAL']; + } + + // Ignore prohibited prefix `length` for spatial index + if (strpos($v['index_type'], 'SPATIAL') === false) { + $v['length'] = isset($v['sub_part']) ? (int) $v['sub_part'] : null; + } + + $tableIndexes[$k] = $v; + } + + return AbstractSchemaManager::_getPortableTableIndexesList($tableIndexes, $tableName); + } + + protected function selectIndexColumns(string $databaseName, ?string $tableName = null): Result + { + $sql = 'SELECT'; + + if ($tableName === null) { + $sql .= ' TABLE_NAME,'; + } + + $sql .= <<<'SQL' + NON_UNIQUE AS Non_Unique, + INDEX_NAME AS Key_name, + CONCAT(COALESCE(COLUMN_NAME, ''), COALESCE(EXPRESSION, '')) AS Column_Name, + SUB_PART AS Sub_Part, + INDEX_TYPE AS Index_Type, + EXPRESSION AS Expression +FROM information_schema.STATISTICS +SQL; + + $conditions = ['TABLE_SCHEMA = ?']; + $params = [$databaseName]; + + if ($tableName !== null) { + $conditions[] = 'TABLE_NAME = ?'; + $params[] = $tableName; + } + + $sql .= ' WHERE ' . implode(' AND ', $conditions) . ' ORDER BY SEQ_IN_INDEX'; + + return $this->_conn->executeQuery($sql, $params); + } + +} \ No newline at end of file diff --git a/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php index d557def5a..ec3e1c20e 100644 --- a/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php +++ b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php @@ -20,7 +20,7 @@ protected function _getPortableTableIndexesList($tableIndexes, $tableName = null foreach ($tableIndexes as $row) { $colNumbers = array_map('intval', explode(' ', $row['indkey'])); $colNames = explode("," , trim($row["indkey_names"], "{,}")); - $colNumName = array_combine($colNumbers, $colNames); + $colNumName = array_combine($colNumbers, $colNames); // required for getting the order of the columns right. foreach ($colNumName as $colNum => $colName) { diff --git a/app/src/Lib/Util/CmgSchemaManagerFactory.php b/app/src/Lib/Util/CmgSchemaManagerFactory.php index 4d54e7fa3..e373951e6 100644 --- a/app/src/Lib/Util/CmgSchemaManagerFactory.php +++ b/app/src/Lib/Util/CmgSchemaManagerFactory.php @@ -3,8 +3,11 @@ namespace App\Lib\Util; use App\Lib\Util\CmgPostgreSQLSchemaManager; +use App\Lib\Util\CmgMySQLSchemaManager; use Doctrine\DBAL\Driver\AbstractPostgreSQLDriver; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\Platforms\MySQL80Platform; +use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory; use Doctrine\DBAL\Schema\SchemaManagerFactory; @@ -29,6 +32,10 @@ public function createSchemaManager(Connection $connection): AbstractSchemaManag return new CmgPostgreSQLSchemaManager($connection, $platform); } + if ($platform instanceof MySQLPlatform) { + return new CmgMySQLSchemaManager($connection, $platform); + } + return $this->defaultFactory->createSchemaManager($connection); } } diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index ee53ab284..888b560f1 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -271,9 +271,23 @@ protected function processSchema( // will be executed later. What we actually do is treat the expression // as a temporary virtual column // https://www.postgresql.org/docs/14/indexes-expressional.html + // https://dev.mysql.com/doc/refman/8.0/en/create-index.html#create-index-functional-key-parts + // https://www.sqlite.org/expridx.html + // XXX MariadBD is not supported $tempColumns = []; - foreach ($iCfg->columns as $clm_name) { + foreach ($iCfg->columns as $idx => $clm_name) { if (! $table->hasColumn($clm_name)) { + if(in_array($this->driver, array( + "Cake\Database\Driver\Mysql", + "Cake\Database\Driver\Sqlite", + ))) { + // MySQL requires the column name to be wrapped with parenthesis + // Instead of indexing a simple column, you can create the index on the result + // of any function applied to a column or multiple columns. + // Be aware of the double parentheses. The syntax is correct since the expression + // must be enclosed within parentheses to distinguish it from columns or column prefixes. + $iCfg->columns[$idx] = $clm_name = "({$clm_name})"; + } $table->addColumn($clm_name, "string"); $tempColumns[] = $clm_name; } @@ -294,7 +308,7 @@ protected function processSchema( // (For Registry) If an attribute is "sourced" it is a CO Person attribute // that is copied via a Pipeline from an External Identity that was created from - // an External Identity Source, so we need a foreign key into ourself. + // an External Identity Source, so we need a foreign key into ourselves. if(isset($tCfg->sourced) && $tCfg->sourced) { $sColumn = "source_" . $tablePrefix.\Cake\Utility\Inflector::singularize($tName) . "_id"; @@ -332,7 +346,7 @@ protected function processSchema( // SchemaManager provides info about the database $sm = $this->conn->createSchemaManager(); - // The is the current database representation + // This is the current database representation $curSchema = $sm->introspectSchema(); $fromSql = $curSchema->toSql($this->conn->getDatabasePlatform()); From 4a393af17aa6eb16d2f0d6b32d031d6d1ba5af28 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 22 Oct 2023 21:18:20 +0300 Subject: [PATCH 4/8] fix dbal deprecation --- app/src/Lib/Util/SchemaManager.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index 888b560f1..eb333a47b 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -62,6 +62,9 @@ class SchemaManager { // The column library from the main config protected $columnLibrary = null; + // The current platform + protected $platform = null; + /** * Construct a new SchemaManager. * @@ -98,6 +101,7 @@ public function __construct(?ConsoleIo $io=null, string $connection='default') { $this->conn = DriverManager::getConnection($cfargs, $config); $this->driver = $cfg['driver']; + $this->platform = $this->conn->getDatabasePlatform(); } /** @@ -358,8 +362,7 @@ protected function processSchema( // schema file). $comparator = new Comparator(); $schemaDiff = $comparator->compareSchemas($curSchema, $schema); - - $diffSql = $schemaDiff->toSaveSql($this->conn->getDatabasePlatform()); + $diffSql = $this->platform->getAlterSchemaSQL($schemaDiff); // We don't start a transaction since in general we always want to move to // the desired state, and if we fail in flight it's probably a bug that From da3278a626178b60a76ff3267dc195b5336a5fa0 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Sun, 22 Oct 2023 21:21:08 +0300 Subject: [PATCH 5/8] use protected property instead of calling again --- app/src/Lib/Util/SchemaManager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index eb333a47b..3f64b2c58 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -345,7 +345,7 @@ protected function processSchema( } // This is the SQL that represents the desired state of the database - $toSql = $schema->toSql($this->conn->getDatabasePlatform()); + $toSql = $schema->toSql($this->platform); // SchemaManager provides info about the database $sm = $this->conn->createSchemaManager(); @@ -353,7 +353,7 @@ protected function processSchema( // This is the current database representation $curSchema = $sm->introspectSchema(); - $fromSql = $curSchema->toSql($this->conn->getDatabasePlatform()); + $fromSql = $curSchema->toSql($this->platform); try { // We manually call compare so we can get the SchemaDiff object. We need From 0c46e61ba144dbbcbbfd644665ff75c34337fe3c Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Mon, 23 Oct 2023 17:11:56 +0300 Subject: [PATCH 6/8] improve index list query creators --- app/src/Lib/Util/CmgMySQLSchemaManager.php | 2 +- app/src/Lib/Util/CmgPostgreSQLSchemaManager.php | 9 ++++++--- app/src/Lib/Util/SchemaManager.php | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/src/Lib/Util/CmgMySQLSchemaManager.php b/app/src/Lib/Util/CmgMySQLSchemaManager.php index 384a31a48..e8169a56e 100644 --- a/app/src/Lib/Util/CmgMySQLSchemaManager.php +++ b/app/src/Lib/Util/CmgMySQLSchemaManager.php @@ -53,7 +53,7 @@ protected function selectIndexColumns(string $databaseName, ?string $tableName = $sql .= <<<'SQL' NON_UNIQUE AS Non_Unique, INDEX_NAME AS Key_name, - CONCAT(COALESCE(COLUMN_NAME, ''), COALESCE(EXPRESSION, '')) AS Column_Name, + CONCAT(COALESCE(COLUMN_NAME, ''), COALESCE(CONCAT('(', EXPRESSION, ')' ), '')) AS Column_Name, SUB_PART AS Sub_Part, INDEX_TYPE AS Index_Type, EXPRESSION AS Expression diff --git a/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php index ec3e1c20e..a1c946264 100644 --- a/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php +++ b/app/src/Lib/Util/CmgPostgreSQLSchemaManager.php @@ -54,9 +54,12 @@ protected function selectIndexColumns(string $databaseName, ?string $tableName = i.indexprs, i.indrelid, ARRAY( - SELECT pg_get_indexdef(i.indexrelid, k + 1, true) - FROM generate_subscripts(i.indkey, 1) AS k - ORDER BY k + SELECT regexp_replace(pg_get_indexdef(i.indexrelid, k + 1, true), + '::.*[^()]', -- regular expression tha matches the type of the column + '', -- replace with empty space + 'g') + FROM generate_subscripts(i.indkey, 1) as k + ORDER BY k ) AS indkey_names, pg_get_expr(indpred, indrelid) AS "where" FROM pg_index i diff --git a/app/src/Lib/Util/SchemaManager.php b/app/src/Lib/Util/SchemaManager.php index 3f64b2c58..416f4e95a 100644 --- a/app/src/Lib/Util/SchemaManager.php +++ b/app/src/Lib/Util/SchemaManager.php @@ -349,7 +349,7 @@ protected function processSchema( // SchemaManager provides info about the database $sm = $this->conn->createSchemaManager(); - + // This is the current database representation $curSchema = $sm->introspectSchema(); From 224c5df488183834771bb3b2957d14a0e0fe995a Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 24 Oct 2023 21:20:22 +0300 Subject: [PATCH 7/8] update schema.json --- app/config/schema/schema.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index 9ea2d7a21..76c8f8f0c 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -443,7 +443,7 @@ "identifiers_i2": { "columns": [ "identifier", "type_id", "external_identity_id" ] }, "identifiers_i3": { "columns": [ "type_id" ] }, "identifiers_i4": { "needed": false, "columns": [ "provisioning_target_id" ] }, - "identifiers_i5": { "columns": [ "lower(identifier::text)", "type_id" ] } + "identifiers_i5": { "columns": [ "lower(identifier)", "type_id" ] } }, "mvea": [ "person", "external_identity", "group" ], "sourced": true @@ -657,9 +657,9 @@ "reference_identifier": {} }, "indexes": { - "ext_identity_sources_records_i1": { "columns": [ "external_identity_source_id" ] }, - "ext_identity_sources_records_i2": { "columns": [ "external_identity_id" ] }, - "ext_identity_sources_records_i3": { "columns": [ "external_identity_source_id", "source_key" ] } + "ext_identity_source_records_i1": { "columns": [ "external_identity_source_id" ] }, + "ext_identity_source_records_i2": { "columns": [ "external_identity_id" ] }, + "ext_identity_source_records_i3": { "columns": [ "external_identity_source_id", "source_key" ] } } } }, From c1f09dd7b9a9b10ae98fc4d0c16c1c804d57d37e Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 24 Oct 2023 22:23:39 +0300 Subject: [PATCH 8/8] source key size must not exceed 1022 --- app/config/schema/schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config/schema/schema.json b/app/config/schema/schema.json index 76c8f8f0c..ba50b9212 100644 --- a/app/config/schema/schema.json +++ b/app/config/schema/schema.json @@ -650,7 +650,7 @@ "columns": { "id": {}, "external_identity_source_id": { "type": "integer", "foreignkey": { "table": "external_identity_sources", "column": "id" }, "notnull": true }, - "source_key": { "type": "string", "size": 1024 }, + "source_key": { "type": "string", "size": 1022 }, "source_record": { "type": "text" }, "last_update": { "type": "datetime" }, "external_identity_id": {},