Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize OracleSchemaManager::listTables #2766

Closed
wants to merge 54 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
95e0a7b
Optimize Oracle SchemaManager #2676
Jul 3, 2017
dec643f
Adding a functional test for Oracle to create schema on a db with lar…
Jul 4, 2017
bf1a170
Fixes for listTables() and adjusted test.
Jul 4, 2017
2ffcec0
Fixes for OraclePlatform::getListTablesColumnsSQL() and ::getListTabl…
Jul 4, 2017
1448b5d
Temporarily removed changes, extended test method to include various …
Jul 5, 2017
ae73a14
Fixing syntax for foreign keys, adding initial checks for schema corr…
Jul 5, 2017
cf51fa3
Adding asserts to check columns, primary keys, indexes and foreign ke…
Jul 5, 2017
2133c82
getColumns and getIndexes return arrays of objects, not names - fixed.
Jul 5, 2017
e03bc4e
Various fixes to test method to align with current state result.
Jul 5, 2017
d0a9cca
Fixed a typo in a helper method name.
Jul 5, 2017
8a789f6
Fixing order of array keys in the test.
Jul 5, 2017
d1bc080
Allow getListTableColumnsSQL to return SQL to fetch all database tabl…
Jul 5, 2017
40e3ef7
Fixes for hardcoded SQL string tests
Jul 5, 2017
9c86b88
Implementation of OracleSchemaManager::listTables override.
Jul 5, 2017
8f2762f
Allow getListTableColumnsSQL to return SQL to fetch all database tabl…
Jul 6, 2017
7947849
Implementation of OracleSchemaManager::listTables override.
Jul 6, 2017
09bdf2d
Use LEFT JOINS instead of subselects in getListTableIndexesSQL
Jul 6, 2017
d4f28bc
Changed order of the tables and joins in the FROM clause of getListTa…
Jul 6, 2017
c6df19c
More changes in getListTableIndexesSQL
Jul 6, 2017
872ca93
Using ALL_* views instead of USER_* views to fetch indexes
Jul 6, 2017
73b5383
Connection object is not available in AbstractPlatform
Jul 6, 2017
844badd
Forgot removing call to listTableIndexes.
Jul 6, 2017
e2b7711
Use LEFT JOINS instead of subselects in getListTableColumnsSQL
Jul 6, 2017
4db79e8
Adjusted OraclePlatformTest::getReturnsGetListTableColumnsSQL for the…
Jul 6, 2017
ed83bec
Implementation of OracleSchemaManager::listTables override.
Jul 7, 2017
7bd9ca6
Stretch test to creating 2150 tables and expecting processing in 3 se…
Jul 7, 2017
8c505e3
Reduced number of test tables to allow test to run without errors on …
Jul 7, 2017
e205f62
Reverted timing check for createSchema to 15 sec.
Jul 7, 2017
e29e546
Implement requested changes.
Jul 10, 2017
d8329c7
Fixed a Scrutinizer warning about the variable $quotedTableIdentifier…
Jul 10, 2017
1090c87
Fix for Scrutinizer still complaining about $quotedTableIdentifier
Jul 10, 2017
8492bf0
Added tests for OracleSchemaManager::getListTableIndexesSQL, ::getLis…
Jul 12, 2017
816d973
Use nullable syntax in OraclePlatform::_getPortableTableDefinition
Jul 18, 2017
e06174d
Revert some of the changes according to latest comments, and introduc…
Sep 6, 2017
554fbd7
Requested changes on SQL syntax and pre-grouping of records by table …
Sep 7, 2017
fab02a7
Adding debugging code
Sep 7, 2017
0bc13ec
More debugging
Sep 7, 2017
fba72c9
Fix for quoted table names
Sep 7, 2017
b7c9c79
Debugging for test on number of SQL queries executed by createSchema
Sep 7, 2017
26d3d02
Adding test for number of queries expected to be run by createSchema
Sep 7, 2017
a4dba40
Simplify OracleSchemaManager::createSchema
Sep 8, 2017
78c8517
Simplified tests and removed some out-of-scope changes.
Sep 8, 2017
760ffb3
Fixed typos.
Sep 8, 2017
a0ef3d8
Unify OraclePlatform SQL for columns/indexes/foreign keys, keeping BC…
Oct 31, 2017
9b94db2
Fixing a missing parameter in method calls
Oct 31, 2017
5308389
Requested changes
Dec 15, 2017
6027ade
Do not linefeed before <<<SQL
Dec 15, 2017
1183a5e
Fixed PHP 7.1 syntax
Dec 15, 2017
fcbf41d
Requested changes.
mondrake Dec 16, 2017
514ab47
Merge branch 'master' into opt_oracle_schema_manager
mondrake Jun 20, 2019
b780a16
Merge branch 'opt_oracle_schema_manager' of https://github.com/mondra…
mondrake Jun 20, 2019
98597c0
PHPCS fixes.
mondrake Jun 20, 2019
706c4c3
PHPCS fixes.
mondrake Jun 20, 2019
b25bfc6
Align name of test protected variables to master.
mondrake Jun 20, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 139 additions & 89 deletions lib/Doctrine/DBAL/Platforms/OraclePlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,22 @@ public function getClobTypeDeclarationSQL(array $field)
return 'CLOB';
}

/**
* Returns the database condition for querying the schema.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
*/
private function getDatabaseCondition(?string $database) : string
{
if ($database !== null && $database !== '/') {
$databaseIdentifier = $this->normalizeIdentifier($database);

return $this->quoteStringLiteral($databaseIdentifier->getName());
}

return "(SELECT SYS_CONTEXT('userenv', 'current_schema') FROM DUAL)";
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -404,43 +420,57 @@ protected function _getCreateTableSQL($table, array $columns, array $options = [
return $sql;
}

/**
* Returns the SQL for a list of indexes in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
* @param string $table The table. If left NULL, the SQL will return all the indexes in the database.
*/
private function getListIndexesSQL(?string $database, ?string $table) : string
{
$databaseCondition = $this->getDatabaseCondition($database);
$tableCondition = '';

if ($table !== null) {
$tableIdentifier = $this->normalizeIdentifier($table);
$quotedTableIdentifier = $this->quoteStringLiteral($tableIdentifier->getName());
$tableCondition = ' AND ind_col.table_name = ' . $quotedTableIdentifier;
}

return <<<SQL
morozov marked this conversation as resolved.
Show resolved Hide resolved
SELECT ind_col.table_name as table_name,
ind_col.index_name AS name,
ind.index_type AS type,
decode(ind.uniqueness, 'NONUNIQUE', 0, 'UNIQUE', 1) AS is_unique,
ind_col.column_name AS column_name,
ind_col.column_position AS column_pos,
con.constraint_type AS is_primary
FROM all_ind_columns ind_col
LEFT JOIN all_indexes ind ON ind.owner = ind_col.index_owner AND ind.index_name = ind_col.index_name
LEFT JOIN all_constraints con ON con.owner = ind_col.index_owner AND con.index_name = ind_col.index_name
WHERE ind_col.index_owner = $databaseCondition$tableCondition
ORDER BY ind_col.table_name, ind_col.index_name, ind_col.column_position
SQL;
}

/**
* {@inheritDoc}
*
* @link http://ezcomponents.org/docs/api/trunk/DatabaseSchema/ezcDbSchemaOracleReader.html
*/
public function getListTableIndexesSQL($table, $currentDatabase = null)
{
$table = $this->normalizeIdentifier($table);
$table = $this->quoteStringLiteral($table->getName());
return $this->getListIndexesSQL($currentDatabase, $table);
}

return "SELECT uind_col.index_name AS name,
(
SELECT uind.index_type
FROM user_indexes uind
WHERE uind.index_name = uind_col.index_name
) AS type,
decode(
(
SELECT uind.uniqueness
FROM user_indexes uind
WHERE uind.index_name = uind_col.index_name
),
'NONUNIQUE',
0,
'UNIQUE',
1
) AS is_unique,
uind_col.column_name AS column_name,
uind_col.column_position AS column_pos,
(
SELECT ucon.constraint_type
FROM user_constraints ucon
WHERE ucon.index_name = uind_col.index_name
) AS is_primary
FROM user_ind_columns uind_col
WHERE uind_col.table_name = " . $table . '
ORDER BY uind_col.column_position ASC';
/**
* Returns the SQL for a list of all indexes in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
*/
public function getListAllIndexesSQL(?string $database = null) : string
{
return $this->getListIndexesSQL($database, null);
}

/**
Expand Down Expand Up @@ -598,36 +628,55 @@ private function getAutoincrementIdentifierName(Identifier $table)
: $identifierName;
}

/**
* Returns the SQL for a list of foreign keys in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
* @param string $table The table. If left NULL, the SQL will return all the indexes in the database.
*/
private function getListForeignKeysSQL(?string $database, ?string $table) : string
{
$databaseCondition = $this->getDatabaseCondition($database);
$tableCondition = '';

if ($table !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add newlines around if to improve readability

$tableIdentifier = $this->normalizeIdentifier($table);
$quotedTableIdentifier = $this->quoteStringLiteral($tableIdentifier->getName());
$tableCondition = ' AND cols.table_name = ' . $quotedTableIdentifier;
}

return <<<SQL
SELECT cols.table_name,
alc.constraint_name,
alc.DELETE_RULE,
cols.column_name "local_column",
cols.position,
r_cols.table_name "references_table",
r_cols.column_name "foreign_column"
FROM all_cons_columns cols
LEFT JOIN all_constraints alc ON alc.owner = cols.owner AND alc.constraint_name = cols.constraint_name
LEFT JOIN all_cons_columns r_cols ON r_cols.owner = alc.r_owner AND r_cols.constraint_name = alc.r_constraint_name AND r_cols.position = cols.position
WHERE cols.owner = $databaseCondition AND alc.constraint_type = 'R'$tableCondition
ORDER BY cols.table_name, cols.constraint_name, cols.position
SQL;
}

/**
* {@inheritDoc}
*/
public function getListTableForeignKeysSQL($table)
{
$table = $this->normalizeIdentifier($table);
$table = $this->quoteStringLiteral($table->getName());
return $this->getListForeignKeysSQL(null, $table);
}

return "SELECT alc.constraint_name,
alc.DELETE_RULE,
cols.column_name \"local_column\",
cols.position,
(
SELECT r_cols.table_name
FROM user_cons_columns r_cols
WHERE alc.r_constraint_name = r_cols.constraint_name
AND r_cols.position = cols.position
) AS \"references_table\",
(
SELECT r_cols.column_name
FROM user_cons_columns r_cols
WHERE alc.r_constraint_name = r_cols.constraint_name
AND r_cols.position = cols.position
) AS \"foreign_column\"
FROM user_cons_columns cols
JOIN user_constraints alc
ON alc.constraint_name = cols.constraint_name
AND alc.constraint_type = 'R'
AND alc.table_name = " . $table . '
ORDER BY cols.constraint_name ASC, cols.position ASC';
/**
* Returns the SQL for a list of all foreign keys in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
*/
public function getListAllForeignKeysSQL(?string $database = null) : string
{
return $this->getListForeignKeysSQL($database, null);
}

/**
Expand All @@ -642,47 +691,48 @@ public function getListTableConstraintsSQL($table)
}

/**
* {@inheritDoc}
* Returns the SQL for a list of columns in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
* @param string $table The table. If left NULL, the SQL will return all the indexes in the database.
*/
public function getListTableColumnsSQL($table, $database = null)
private function getListColumnsSQL(?string $database, ?string $table) : string
{
$table = $this->normalizeIdentifier($table);
$table = $this->quoteStringLiteral($table->getName());

$tabColumnsTableName = 'user_tab_columns';
$colCommentsTableName = 'user_col_comments';
$tabColumnsOwnerCondition = '';
$colCommentsOwnerCondition = '';
$databaseCondition = $this->getDatabaseCondition($database);
$tableCondition = '';

if ($database !== null && $database !== '/') {
$database = $this->normalizeIdentifier($database);
$database = $this->quoteStringLiteral($database->getName());
$tabColumnsTableName = 'all_tab_columns';
$colCommentsTableName = 'all_col_comments';
$tabColumnsOwnerCondition = ' AND c.owner = ' . $database;
$colCommentsOwnerCondition = ' AND d.OWNER = c.OWNER';
if ($table !== null) {
$tableIdentifier = $this->normalizeIdentifier($table);
$quotedTableIdentifier = $this->quoteStringLiteral($tableIdentifier->getName());
$tableCondition = ' AND c.table_name = ' . $quotedTableIdentifier;
}

return sprintf(
<<<'SQL'
SELECT c.*,
(
SELECT d.comments
FROM %s d
WHERE d.TABLE_NAME = c.TABLE_NAME%s
AND d.COLUMN_NAME = c.COLUMN_NAME
) AS comments
FROM %s c
WHERE c.table_name = %s%s
ORDER BY c.column_id
SQL
,
$colCommentsTableName,
$colCommentsOwnerCondition,
$tabColumnsTableName,
$table,
$tabColumnsOwnerCondition
);
return <<<SQL
SELECT c.*,
d.comments AS comments
FROM all_tab_columns c
LEFT JOIN all_col_comments d ON d.OWNER = c.OWNER AND d.TABLE_NAME = c.TABLE_NAME AND d.COLUMN_NAME = c.COLUMN_NAME
WHERE c.owner = $databaseCondition$tableCondition
ORDER BY c.table_name, c.column_id
SQL;
}

/**
* {@inheritDoc}
*/
public function getListTableColumnsSQL($table, $database = null)
{
return $this->getListColumnsSQL($database, $table);
}

/**
* Returns the SQL for a list of all columns in the database.
*
* @param string $database The database schema. If NULL or '/', the currently active schema will be queried.
*/
public function getListAllColumnsSQL(?string $database = null) : string
{
return $this->getListColumnsSQL($database, null);
}

/**
Expand Down
67 changes: 67 additions & 0 deletions lib/Doctrine/DBAL/Schema/OracleSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,73 @@
*/
class OracleSchemaManager extends AbstractSchemaManager
{
/**
* Holds instance of the database platform used for this schema manager.
*
* @var OraclePlatform
*/
protected $_platform;

/**
* {@inheritdoc}
*/
public function listTables()
{
$currentDatabase = $this->_conn->getDatabase();

$tableNames = $this->listTableNames();

// Get all column definitions in one database call.
$columnsByTable = $this->getAssetRecordsByTable($this->_platform->getListAllColumnsSQL($currentDatabase));

// Get all foreign keys definitions in one database call.
$foreignKeysByTable = $this->getAssetRecordsByTable($this->_platform->getListAllForeignKeysSQL($currentDatabase));

// Get all indexes definitions in one database call.
$indexesByTable = $this->getAssetRecordsByTable($this->_platform->getListAllIndexesSQL($currentDatabase));

$tables = [];
foreach ($tableNames as $quotedTableName) {
$tableName = trim($quotedTableName, $this->_platform->getIdentifierQuoteCharacter());

$columns = $this->_getPortableTableColumnList($quotedTableName, '', $columnsByTable[$tableName]);

$foreignKeys = [];
if (isset($foreignKeysByTable[$tableName])) {
$foreignKeys = $this->_getPortableTableForeignKeysList($foreignKeysByTable[$tableName]);
}

$indexes = [];
if (isset($indexesByTable[$tableName])) {
$indexes = $this->_getPortableTableIndexesList($indexesByTable[$tableName], $quotedTableName);
}

$tables[] = new Table($quotedTableName, $columns, $indexes, $foreignKeys);
}

return $tables;
}

/**
* Helper method to group a set of asset records by the table name.
*
* @param string $sql An SQL statement to be executed, that contains a
* TABLE_NAME field for grouping.
*
* @return mixed[] An associative array with key being the table name, and
* value a simple array of records associated with the table.
*/
private function getAssetRecordsByTable(string $sql) : array
{
$input = $this->_conn->fetchAll($sql);
$output = [];
foreach ($input as $record) {
$output[$record['TABLE_NAME']][] = $record;
}

return $output;
}

/**
* {@inheritdoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,29 @@ public function testCreateAndListSequences() : void
{
self::markTestSkipped("Skipped for uppercase letters are contained in sequences' names. Fix the schema manager in 3.0.");
}

/**
* @group DBAL-2766
*/
public function testCreateSchemaNumberOfQueriesInvariable() : void
{
// Introspect the db schema.
$preCount = $this->sqlLoggerStack->currentQuery;
$schema = $this->schemaManager->createSchema();
$firstQueryCount = $this->sqlLoggerStack->currentQuery - $preCount;

// Create a couple of additional tables.
$this->connection->executeUpdate("CREATE TABLE tbl_test_2766_0 (x_id VARCHAR2(255) DEFAULT 'x' NOT NULL, x_data CLOB DEFAULT NULL NULL, x_number NUMBER(10) DEFAULT 0 NOT NULL, PRIMARY KEY(x_id))");
$this->connection->executeUpdate("CREATE TABLE tbl_test_2766_1 (x_id VARCHAR2(255) DEFAULT 'x' NOT NULL, x_data CLOB DEFAULT NULL NULL, x_number NUMBER(10) DEFAULT 0 NOT NULL, x_parent_id VARCHAR2(255) DEFAULT 'x' NOT NULL, CONSTRAINT tbl_test_2766_fk_1 FOREIGN KEY (x_parent_id) REFERENCES tbl_test_2766_0(x_id), PRIMARY KEY(x_id))");
$this->connection->executeUpdate('CREATE UNIQUE INDEX tbl_test_2766_uix_1 ON tbl_test_2766_1 (x_number)');

// Introspect the db schema again.
$preCount = $this->sqlLoggerStack->currentQuery;
$schema = $this->schemaManager->createSchema();
$secondQueryCount = $this->sqlLoggerStack->currentQuery - $preCount;

// The number of queries needed to execute createSchema should be the
// same regardless of additional tables added.
self::assertEquals($firstQueryCount, $secondQueryCount);
}
}
Loading