From 2d331942f553a5ed67a22e4db8e3c85d4be62e8d Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 10:10:07 +0530 Subject: [PATCH 1/6] Reduce duplication Signed-off-by: Madhura Jayaratne --- libraries/DBQbe.class.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index ae67968d2b..2724b8f757 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1642,18 +1642,14 @@ class PMA_DbQbe ) { $finalized[$masterTable] = $clause; $added = true; - - // We are done if all tables are in $finalized - if (count($finalized) == count($allTables)) { - break 3; - } - } else if (! isset($finalized[$foreignTable]) + } elseif (! isset($finalized[$foreignTable]) && isset($finalized[$masterTable]) && in_array($foreignTable, $allTables) ) { $finalized[$foreignTable] = $clause; $added = true; - + } + if ($added) { // We are done if all tables are in $finalized if (count($finalized) == count($allTables)) { break 3; From 7a4efae40502fcf953085292e47df43a40c337aa Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 10:32:15 +0530 Subject: [PATCH 2/6] Better variable names Signed-off-by: Madhura Jayaratne --- libraries/DBQbe.class.php | 60 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index 2724b8f757..14cddd2757 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1304,23 +1304,23 @@ class PMA_DbQbe /** * Provides UNIQUE columns and INDEX columns present in criteria tables * - * @param array $all_tables Tables involved in the search - * @param array $all_columns Columns involved in the search + * @param array $search_tables Tables involved in the search + * @param array $search_columns Columns involved in the search * @param array $where_clause_columns Columns having criteria where clause * * @return array having UNIQUE and INDEX columns */ - private function _getIndexes($all_tables, $all_columns, + private function _getIndexes($search_tables, $search_columns, $where_clause_columns ) { $unique_columns = array(); $index_columns = array(); - foreach ($all_tables as $table) { + foreach ($search_tables as $table) { $indexes = $GLOBALS['dbi']->getTableIndexes($this->_db, $table); foreach ($indexes as $index) { $column = $table . '.' . $index['Column_name']; - if (isset($all_columns[$column])) { + if (isset($search_columns[$column])) { if ($index['Non_unique'] == 0) { if (isset($where_clause_columns[$column])) { $unique_columns[$column] = 'Y'; @@ -1347,27 +1347,27 @@ class PMA_DbQbe /** * Provides UNIQUE columns and INDEX columns present in criteria tables * - * @param array $all_tables Tables involved in the search - * @param array $all_columns Columns involved in the search + * @param array $search_tables Tables involved in the search + * @param array $search_columns Columns involved in the search * @param array $where_clause_columns Columns having criteria where clause * * @return array having UNIQUE and INDEX columns */ - private function _getLeftJoinColumnCandidates($all_tables, $all_columns, + private function _getLeftJoinColumnCandidates($search_tables, $search_columns, $where_clause_columns ) { $GLOBALS['dbi']->selectDb($this->_db); // Get unique columns and index columns $indexes = $this->_getIndexes( - $all_tables, $all_columns, $where_clause_columns + $search_tables, $search_columns, $where_clause_columns ); $unique_columns = $indexes['unique']; $index_columns = $indexes['index']; list($candidate_columns, $needsort) = $this->_getLeftJoinColumnCandidatesBest( - $all_tables, $where_clause_columns, $unique_columns, $index_columns + $search_tables, $where_clause_columns, $unique_columns, $index_columns ); // If we came up with $unique_columns (very good) or $index_columns (still @@ -1403,14 +1403,14 @@ class PMA_DbQbe /** * Provides the main table to form the LEFT JOIN clause * - * @param array $all_tables Tables involved in the search - * @param array $all_columns Columns involved in the search + * @param array $search_tables Tables involved in the search + * @param array $search_columns Columns involved in the search * @param array $where_clause_columns Columns having criteria where clause * @param array $where_clause_tables Tables having criteria where clause * * @return string table name */ - private function _getMasterTable($all_tables, $all_columns, + private function _getMasterTable($search_tables, $search_columns, $where_clause_columns, $where_clause_tables ) { if (count($where_clause_tables) == 1) { @@ -1425,7 +1425,7 @@ class PMA_DbQbe // because he is using one of his databases as pmadb, // the last db selected is not always the one where we need to work) $candidate_columns = $this->_getLeftJoinColumnCandidates( - $all_tables, $all_columns, $where_clause_columns + $search_tables, $search_columns, $where_clause_columns ); // Generally, we need to display all the rows of foreign (referenced) @@ -1537,27 +1537,27 @@ class PMA_DbQbe $from_clause = ''; if (isset($_POST['criteriaColumn']) && count($_POST['criteriaColumn']) > 0) { // Initialize some variables - $all_tables = $all_columns = array(); + $search_tables = $search_columns = array(); // We only start this if we have fields, otherwise it would be dumb foreach ($_POST['criteriaColumn'] as $value) { $parts = explode('.', $value); if (! empty($parts[0]) && ! empty($parts[1])) { $table = str_replace('`', '', $parts[0]); - $all_tables[$table] = $table; - $all_columns[] = $table . '.' . str_replace('`', '', $parts[1]); + $search_tables[$table] = $table; + $search_columns[] = $table . '.' . str_replace('`', '', $parts[1]); } } // end while // Create LEFT JOINS out of Relations - $from_clause = $this->_getJoinForFromClause($all_tables, $all_columns); + $from_clause = $this->_getJoinForFromClause($search_tables, $search_columns); // In case relations are not defined, just generate the FROM clause // from the list of tables, however we don't generate any JOIN if (empty($from_clause)) { // Create cartesian product $from_clause = implode( - ", ", array_map('PMA_Util::backquote', $all_tables) + ", ", array_map('PMA_Util::backquote', $search_tables) ); } } // end count($_POST['criteriaColumn']) > 0 @@ -1568,19 +1568,19 @@ class PMA_DbQbe /** * Formulates the WHERE clause by JOINing tables * - * @param array $allTables Tables involved in the search - * @param array $allColumns Columns involved in the search + * @param array $searchTables Tables involved in the search + * @param array $searchColumns Columns involved in the search * * @return string table name */ - private function _getJoinForFromClause($allTables, $allColumns) + private function _getJoinForFromClause($searchTables, $searchColumns) { // $relations[master_table][foreign_table] => clause $relations = array(); // Fill $relations with inter table relationship data - foreach ($allTables as $oneTable) { + foreach ($searchTables as $oneTable) { $relations[$oneTable] = array(); $foreigners = PMA_getForeigners($GLOBALS['db'], $oneTable); @@ -1621,7 +1621,7 @@ class PMA_DbQbe // Get master table $master = $this->_getMasterTable( - $allTables, $allColumns, + $searchTables, $searchColumns, $whereClauseColumns, $whereClauseTables ); @@ -1644,14 +1644,14 @@ class PMA_DbQbe $added = true; } elseif (! isset($finalized[$foreignTable]) && isset($finalized[$masterTable]) - && in_array($foreignTable, $allTables) + && in_array($foreignTable, $searchTables) ) { $finalized[$foreignTable] = $clause; $added = true; } if ($added) { // We are done if all tables are in $finalized - if (count($finalized) == count($allTables)) { + if (count($finalized) == count($searchTables)) { break 3; } } @@ -1665,7 +1665,7 @@ class PMA_DbQbe // Tables that can not be combined with the table cluster // that includes master table - $unfinalized = array_diff($allTables, array_keys($finalized)); + $unfinalized = array_diff($searchTables, array_keys($finalized)); // Add these tables as cartesian product before joined tables $join = implode(', ', array_map('PMA_Util::backquote', $unfinalized)); @@ -1855,7 +1855,7 @@ class PMA_DbQbe /** * Get best * - * @param array $all_tables All tables + * @param array $search_tables Tables involved in the search * @param array $where_clause_columns Columns with where clause * @param array $unique_columns Unique columns * @param array $index_columns Indexed columns @@ -1863,7 +1863,7 @@ class PMA_DbQbe * @return array */ private function _getLeftJoinColumnCandidatesBest( - $all_tables, $where_clause_columns, $unique_columns, $index_columns + $search_tables, $where_clause_columns, $unique_columns, $index_columns ) { // now we want to find the best. if (isset($unique_columns) && count($unique_columns) > 0) { @@ -1879,7 +1879,7 @@ class PMA_DbQbe $needsort = 0; return array($candidate_columns, $needsort); } else { - $candidate_columns = $all_tables; + $candidate_columns = $search_tables; $needsort = 0; return array($candidate_columns, $needsort); } From fae84aca9a7f964f74b3480d8a8ded2a3321ed07 Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 10:45:19 +0530 Subject: [PATCH 3/6] Refactor: move to a new method Signed-off-by: Madhura Jayaratne --- libraries/DBQbe.class.php | 72 ++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index 14cddd2757..2b72d1b46f 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1581,37 +1581,7 @@ class PMA_DbQbe // Fill $relations with inter table relationship data foreach ($searchTables as $oneTable) { - $relations[$oneTable] = array(); - - $foreigners = PMA_getForeigners($GLOBALS['db'], $oneTable); - foreach ($foreigners as $field => $foreigner) { - // Foreign keys data - if ($field == 'foreign_keys_data') { - foreach ($foreigner as $oneKey) { - $clauses = array(); - // There may be multiple column relations - foreach ($oneKey['index_list'] as $index => $oneField) { - $clauses[] = PMA_Util::backquote($oneTable) . "." - . PMA_Util::backquote($oneField) . " = " - . PMA_Util::backquote( - $oneKey['ref_table_name'] - ) . "." - . PMA_Util::backquote( - $oneKey['ref_index_list'][$index] - ); - } - // Combine multiple column relations with AND - $relations[$oneTable][$oneKey['ref_table_name']] - = implode(" AND ", $clauses); - } - } else { // Internal relations - $relations[$oneTable][$foreigner['foreign_table']] - = PMA_Util::backquote($oneTable) . "." - . PMA_Util::backquote($field) . " = " - . PMA_Util::backquote($foreigner['foreign_table']) . "." - . PMA_Util::backquote($foreigner['foreign_field']); - } - } + $this->_loadRelationsForTable($relations, $oneTable); } // Get tables and columns with valid where clauses @@ -1688,6 +1658,46 @@ class PMA_DbQbe return $join; } + /** + * Loads relations for a given table into the $relations array + * + * @param array $relations array of relations + * @param string $oneTable the table + * + * @return void + */ + private function _loadRelationsForTable(&$relations, $oneTable) + { + $relations[$oneTable] = array(); + + $foreigners = PMA_getForeigners($GLOBALS['db'], $oneTable); + foreach ($foreigners as $field => $foreigner) { + // Foreign keys data + if ($field == 'foreign_keys_data') { + foreach ($foreigner as $oneKey) { + $clauses = array(); + // There may be multiple column relations + foreach ($oneKey['index_list'] as $index => $oneField) { + $clauses[] + = PMA_Util::backquote($oneTable) . "." + . PMA_Util::backquote($oneField) . " = " + . PMA_Util::backquote($oneKey['ref_table_name']) . "." + . PMA_Util::backquote($oneKey['ref_index_list'][$index]); + } + // Combine multiple column relations with AND + $relations[$oneTable][$oneKey['ref_table_name']] + = implode(" AND ", $clauses); + } + } else { // Internal relations + $relations[$oneTable][$foreigner['foreign_table']] + = PMA_Util::backquote($oneTable) . "." + . PMA_Util::backquote($field) . " = " + . PMA_Util::backquote($foreigner['foreign_table']) . "." + . PMA_Util::backquote($foreigner['foreign_field']); + } + } + } + /** * Provides the generated SQL query * From f965893c7b9c97d376122c700b003e1e89fa0216 Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 10:53:39 +0530 Subject: [PATCH 4/6] Refactor: Move table JOINing logic to a separate method Signed-off-by: Madhura Jayaratne --- libraries/DBQbe.class.php | 74 +++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index 2b72d1b46f..43a6808347 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1602,36 +1602,8 @@ class PMA_DbQbe // Add master tables $finalized[$master] = ''; } - - while (true) { - $added = false; - foreach ($relations as $masterTable => $foreignData) { - foreach ($foreignData as $foreignTable => $clause) { - if (! isset($finalized[$masterTable]) - && isset($finalized[$foreignTable]) - ) { - $finalized[$masterTable] = $clause; - $added = true; - } elseif (! isset($finalized[$foreignTable]) - && isset($finalized[$masterTable]) - && in_array($foreignTable, $searchTables) - ) { - $finalized[$foreignTable] = $clause; - $added = true; - } - if ($added) { - // We are done if all tables are in $finalized - if (count($finalized) == count($searchTables)) { - break 3; - } - } - } - } - // If no new tables were added during this iteration, break; - if (! $added) { - break; - } - } + // Fill the $finalized array with JOIN clauses for each table + $this->_fillJoinClauses($finalized, $relations); // Tables that can not be combined with the table cluster // that includes master table @@ -1698,6 +1670,48 @@ class PMA_DbQbe } } + /** + * Fills the $finalized arrays with JOIN clauses for each of the tables + * + * @param array $finalized JOIN clauses for each table + * @param array $relations Relations among tables + * @param array $searchTables Tables involved in the search + * + * @return void + */ + private function _fillJoinClauses(&$finalized, $relations) + { + while (true) { + $added = false; + foreach ($relations as $masterTable => $foreignData) { + foreach ($foreignData as $foreignTable => $clause) { + if (! isset($finalized[$masterTable]) + && isset($finalized[$foreignTable]) + ) { + $finalized[$masterTable] = $clause; + $added = true; + } elseif (! isset($finalized[$foreignTable]) + && isset($finalized[$masterTable]) + && in_array($foreignTable, $searchTables) + ) { + $finalized[$foreignTable] = $clause; + $added = true; + } + if ($added) { + // We are done if all tables are in $finalized + if (count($finalized) == count($searchTables)) { + return; + } + } + } + } + // If no new tables were added during this iteration, break; + if (! $added) { + return; + } + } + } + /** * Provides the generated SQL query * From 714ab04d5135136e32fc08c19e191eadeeb6e343 Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 11:47:22 +0530 Subject: [PATCH 5/6] bug #4795 Query builder: missing joint for the intermediary table Signed-off-by: Madhura Jayaratne --- ChangeLog | 1 + libraries/DBQbe.class.php | 67 +++++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index d38c43db3c..db042d2ec5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -58,6 +58,7 @@ phpMyAdmin - ChangeLog + rfe #1594 Use plain English descriptors instead of script names for icon link destinations + rfe #1541 Disable foreign key checks for some operations - bug #4957 "With selected" links doesn't work in table browse +- bug #4795 Query builder: missing joint for the intermediary table 4.4.10.0 (not yet released) - bug #4950 Issues in database selection for replication diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index 43a6808347..480d166872 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1575,7 +1575,6 @@ class PMA_DbQbe */ private function _getJoinForFromClause($searchTables, $searchColumns) { - // $relations[master_table][foreign_table] => clause $relations = array(); @@ -1603,13 +1602,68 @@ class PMA_DbQbe $finalized[$master] = ''; } // Fill the $finalized array with JOIN clauses for each table - $this->_fillJoinClauses($finalized, $relations); + $this->_fillJoinClauses($finalized, $relations, $searchTables); + + // JOIN clause + $join = ''; // Tables that can not be combined with the table cluster // that includes master table $unfinalized = array_diff($searchTables, array_keys($finalized)); - // Add these tables as cartesian product before joined tables - $join = implode(', ', array_map('PMA_Util::backquote', $unfinalized)); + if (count($unfinalized) > 0) { + + // We need to look for intermediary tables to JOIN unfinalized tables + // Heuristic to chose intermediary tables is to look for tables + // having relationships with unfinalized tables + foreach ($unfinalized as $oneTable) { + + $references = PMA_getChildReferences($this->_db, $oneTable); + foreach ($references as $column => $columnReferences) { + foreach ($columnReferences as $reference) { + + // Only from this schema + if ($reference['table_schema'] == $this->_db) { + $table = $reference['table_name']; + + $this->_loadRelationsForTable($relations, $table); + + // Make copies + $tempFinalized = $finalized; + $tempSearchTables = $searchTables; + $tempSearchTables[] = $table; + + $this->_fillJoinClauses( + $tempFinalized, $relations, $tempSearchTables + ); + + $tempUnfinalized = array_diff( + $tempSearchTables, array_keys($tempFinalized) + ); + + // Take greedy approach, if the unfinalized count + // drops we keep the new tables + if (count($tempUnfinalized) < count($unfinalized)) { + $finalized = $tempFinalized; + $searchTables = $tempSearchTables; + } + + if (count($tempUnfinalized) == 0) { + break 3; + } + } + } + } + } + + $unfinalized = array_diff($searchTables, array_keys($finalized)); + // If there are still unfinalized tables + if (count($unfinalized) > 0) { + // Add these tables as cartesian product before joined tables + $join .= implode( + ', ', array_map('PMA_Util::backquote', $unfinalized) + ); + } + } $first = true; // Add joined tables @@ -1679,11 +1733,12 @@ class PMA_DbQbe * * @return void */ - private function _fillJoinClauses(&$finalized, $relations) + private function _fillJoinClauses(&$finalized, $relations, $searchTables) { while (true) { $added = false; - foreach ($relations as $masterTable => $foreignData) { + foreach ($searchTables as $masterTable) { + $foreignData = $relations[$masterTable]; foreach ($foreignData as $foreignTable => $clause) { if (! isset($finalized[$masterTable]) && isset($finalized[$foreignTable]) From 45cb8e8e7cf8f56d41a3cc75be5ad7444feb943e Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Wed, 17 Jun 2015 11:56:04 +0530 Subject: [PATCH 6/6] Improve comments Signed-off-by: Madhura Jayaratne --- libraries/DBQbe.class.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libraries/DBQbe.class.php b/libraries/DBQbe.class.php index 480d166872..47fc8c79a7 100644 --- a/libraries/DBQbe.class.php +++ b/libraries/DBQbe.class.php @@ -1608,7 +1608,7 @@ class PMA_DbQbe $join = ''; // Tables that can not be combined with the table cluster - // that includes master table + // which includes master table $unfinalized = array_diff($searchTables, array_keys($finalized)); if (count($unfinalized) > 0) { @@ -1632,6 +1632,7 @@ class PMA_DbQbe $tempSearchTables = $searchTables; $tempSearchTables[] = $table; + // Try joining with the added table $this->_fillJoinClauses( $tempFinalized, $relations, $tempSearchTables ); @@ -1639,14 +1640,15 @@ class PMA_DbQbe $tempUnfinalized = array_diff( $tempSearchTables, array_keys($tempFinalized) ); - - // Take greedy approach, if the unfinalized count - // drops we keep the new tables + // Take greedy approach. + // If the unfinalized count drops we keep the new table + // and switch temporary varibles with the original ones if (count($tempUnfinalized) < count($unfinalized)) { $finalized = $tempFinalized; $searchTables = $tempSearchTables; } + // We are done if no unfinalized tables anymore if (count($tempUnfinalized) == 0) { break 3; }