From 2e101cc7336f2a448dd1682a49f8b343386f8c07 Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Tue, 27 Sep 2011 21:06:21 +0530 Subject: [PATCH 1/3] bug #3412862 [relation] Relations/constraints are dropped/created on every change --- ChangeLog | 1 + tbl_relation.php | 29 +++++++++++------------------ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index cc46481b18..a45a6cad17 100644 --- a/ChangeLog +++ b/ChangeLog @@ -14,6 +14,7 @@ phpMyAdmin - ChangeLog - bug #3411633 [core] Call to undefined function PMA_isSuperuser() - bug #3413743 [interface] Display options link missing after search - bug #3324161 [core] CSP policy causing designer JS buttons to fail +- bug #3412862 [relation] Relations/constraints are dropped/created on every change 3.4.5.0 (2011-09-14) - bug #3375325 [interface] Page list in navigation frame looks odd diff --git a/tbl_relation.php b/tbl_relation.php index 400fc6319e..e4dcc16df5 100644 --- a/tbl_relation.php +++ b/tbl_relation.php @@ -72,9 +72,9 @@ function PMA_generate_dropdown($dropdown_question, $select_name, $choices, $sele } /** - * Split a string on backquote pairs + * Split a string on backquote pairs * - * @param string original string + * @param string original string * @return array containing the elements (and their surrounding backquotes) * * @access public @@ -97,7 +97,7 @@ function PMA_backquote_split($text) } $elements[] = substr($text, $first_backquote, $second_backquote - $first_backquote + 1); $pos = $second_backquote + 1; - } + } return($elements); } @@ -216,25 +216,18 @@ if (isset($_REQUEST['destination_foreign'])) { } elseif (PMA_backquote($existrel_foreign[$master_field]['foreign_db']) != $foreign_db || PMA_backquote($existrel_foreign[$master_field]['foreign_table']) != $foreign_table || PMA_backquote($existrel_foreign[$master_field]['foreign_field']) != $foreign_field - || ($_REQUEST['on_delete'][$master_field_md5] != (!empty($existrel_foreign[$master_field]['on_delete']) ? $existrel_foreign[$master_field]['on_delete'] : '')) - || ($_REQUEST['on_update'][$master_field_md5] != (!empty($existrel_foreign[$master_field]['on_update']) ? $existrel_foreign[$master_field]['on_update'] : '')) + || ($_REQUEST['on_delete'][$master_field_md5] != (!empty($existrel_foreign[$master_field]['on_delete']) ? $existrel_foreign[$master_field]['on_delete'] : 'RESTRICT')) + || ($_REQUEST['on_update'][$master_field_md5] != (!empty($existrel_foreign[$master_field]['on_update']) ? $existrel_foreign[$master_field]['on_update'] : 'RESTRICT')) ) { // another foreign key is already defined for this field // or // an option has been changed for ON DELETE or ON UPDATE - // remove existing key + // remove existing key and add the new one $sql_query = 'ALTER TABLE ' . PMA_backquote($table) . ' DROP FOREIGN KEY ' - . PMA_backquote($existrel_foreign[$master_field]['constraint']) . ';'; - - // I tried to send both in one query but it failed - PMA_DBI_query($sql_query); - $display_query .= $sql_query . "\n"; - - // add another - $sql_query = 'ALTER TABLE ' . PMA_backquote($table) - . ' ADD FOREIGN KEY (' + . PMA_backquote($existrel_foreign[$master_field]['constraint']) . ', ' + . 'ADD FOREIGN KEY (' . PMA_backquote($master_field) . ')' . ' REFERENCES ' . $foreign_db . '.' @@ -376,7 +369,7 @@ if ($cfgRelation['relwork'] || PMA_foreignkey_supported($tbl_type)) { && isset($curr_table[1]) && strtoupper($curr_table[1]) == $tbl_type) { // explicitely ask for non-quoted list of indexed columns - // need to obtain backquoted values to support dots inside values + // need to obtain backquoted values to support dots inside values $selectboxall_foreign = array_merge($selectboxall_foreign, $current_table->getIndexedColumns($backquoted = true)); } } // end while over tables @@ -506,9 +499,9 @@ if ($col_rs && PMA_DBI_num_rows($col_rs) > 0) { Date: Tue, 27 Sep 2011 21:19:57 +0530 Subject: [PATCH 2/3] coding style improvements and fixing some doc blocks --- tbl_relation.php | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tbl_relation.php b/tbl_relation.php index 7e4975871b..fc3d868aed 100644 --- a/tbl_relation.php +++ b/tbl_relation.php @@ -48,9 +48,11 @@ $options_array = array( /** * Generate dropdown choices * - * @param string Message to display - * @param string Name of the field + * @param array $choices Choices for dropdown + * @param string $selected_value Selected value + * * @return string The existing value (for selected) * * @access public @@ -74,7 +76,8 @@ function PMA_generate_dropdown($dropdown_question, $select_name, $choices, $sele /** * Split a string on backquote pairs * - * @param string original string + * @param string $text original string + * * @return array containing the elements (and their surrounding backquotes) * * @access public @@ -121,8 +124,7 @@ if ($cfgRelation['displaywork']) { } // will be used in the logic for internal relations and foreign keys: -$me_fields_name = - isset($_REQUEST['fields_name']) +$me_fields_name = isset($_REQUEST['fields_name']) ? $_REQUEST['fields_name'] : null; @@ -137,8 +139,7 @@ if (isset($destination) && $cfgRelation['relwork']) { if (! empty($foreign_string)) { $foreign_string = trim($foreign_string, '`'); - list($foreign_db, $foreign_table, $foreign_field) = - explode('.', $foreign_string); + list($foreign_db, $foreign_table, $foreign_field) = explode('.', $foreign_string); if (! isset($existrel[$master_field])) { $upd_query = 'INSERT INTO ' . PMA_backquote($GLOBALS['cfgRelation']['db']) . '.' . PMA_backquote($cfgRelation['relation']) . '(master_db, master_table, master_field, foreign_db, foreign_table, foreign_field)' @@ -261,12 +262,13 @@ if (isset($_REQUEST['destination_foreign'])) { $seen_error = true; } if (substr($tmp_error, 1, 4) == '1216' - || substr($tmp_error, 1, 4) == '1452') { + || substr($tmp_error, 1, 4) == '1452' + ) { PMA_mysqlDie($tmp_error, $sql_query, false, '', false); echo PMA_showMySQLDocu('manual_Table_types', 'InnoDB_foreign_key_constraints') . "\n"; } if (substr($tmp_error, 1, 4) == '1005') { - $message = PMA_Message::error( __('Error creating foreign key on %1$s (check data types)')); + $message = PMA_Message::error(__('Error creating foreign key on %1$s (check data types)')); $message->addParam($master_field); $message->display(); echo PMA_showMySQLDocu('manual_Table_types', 'InnoDB_foreign_key_constraints') . "\n"; @@ -366,8 +368,9 @@ if ($cfgRelation['relwork'] || PMA_foreignkey_supported($tbl_type)) { // if foreign keys are supported, collect all keys from other // tables of the same engine if (PMA_foreignkey_supported($tbl_type) - && isset($curr_table[1]) - && strtoupper($curr_table[1]) == $tbl_type) { + && isset($curr_table[1]) + && strtoupper($curr_table[1]) == $tbl_type + ) { // explicitely ask for non-quoted list of indexed columns // need to obtain backquoted values to support dots inside values $selectboxall_foreign = array_merge($selectboxall_foreign, $current_table->getIndexedColumns($backquoted = true)); From 0ad1a0f831c8fb9c8f68dd8be29d845ee54b7d65 Mon Sep 17 00:00:00 2001 From: Madhura Jayaratne Date: Tue, 27 Sep 2011 21:49:41 +0530 Subject: [PATCH 3/3] Fix doc blocks and multiline if statements --- libraries/relation.lib.php | 206 +++++++++++++++++++++++-------------- 1 file changed, 128 insertions(+), 78 deletions(-) diff --git a/libraries/relation.lib.php b/libraries/relation.lib.php index b7ea862e88..5e3d7ca083 100644 --- a/libraries/relation.lib.php +++ b/libraries/relation.lib.php @@ -12,8 +12,9 @@ if (! defined('PHPMYADMIN')) { /** * Executes a query as controluser if possible, otherwise as normal user * - * @param string the query to execute - * @param boolean whether to display SQL error messages or not + * @param string $sql the query to execute + * @param boolean $show_error whether to display SQL error messages or not + * @param int $options query options * * @return integer the result set, or false if no result set * @@ -42,7 +43,8 @@ function PMA_query_as_controluser($sql, $show_error = true, $options = 0) } // end of the "PMA_query_as_controluser()" function /** - * @param bool $verbose whether to print diagnostic info + * @param bool $verbose whether to print diagnostic info + * * @return array $cfgRelation */ function PMA_getRelationsParam($verbose = false) @@ -65,7 +67,9 @@ function PMA_getRelationsParam($verbose = false) /** * prints out diagnostic info for pma relation feature * - * @param array $cfgRelation + * @param array $cfgRelation + * + * @return nothing */ function PMA_printRelationsParamDiagnostic($cfgRelation) { @@ -157,10 +161,12 @@ function PMA_printRelationsParamDiagnostic($cfgRelation) /** * prints out one diagnostic message for a feature * - * @param string feature name in a message string - * @param string the $GLOBALS['cfgRelation'] parameter to check - * @param array utility messages - * @param boolean whether to skip a line after the message + * @param string $feature_name feature name in a message string + * @param string $relation_parameter the $GLOBALS['cfgRelation'] parameter to check + * @param array $messages utility messages + * @param boolean $skip_line whether to skip a line after the message + * + * @return nothing */ function PMA_printDiagMessageForFeature($feature_name, $relation_parameter, $messages, $skip_line=true) { @@ -175,10 +181,12 @@ function PMA_printDiagMessageForFeature($feature_name, $relation_parameter, $mes /** * prints out one diagnostic message for a configuration parameter * - * @param string config parameter name to display - * @param boolean whether this parameter is set - * @param array utility messages - * @param string anchor in Documentation.html + * @param string $parameter config parameter name to display + * @param boolean $relation_parameter_set whether this parameter is set + * @param array $messages utility messages + * @param string $doc_anchor anchor in Documentation.html + * + * @return nothing */ function PMA_printDiagMessageForParameter($parameter, $relation_parameter_set, $messages, $doc_anchor) { @@ -216,7 +224,8 @@ function PMA__getRelationsParam() $cfgRelation['db'] = null; if ($GLOBALS['server'] == 0 || empty($GLOBALS['cfg']['Server']['pmadb']) - || ! PMA_DBI_select_db($GLOBALS['cfg']['Server']['pmadb'], $GLOBALS['controllink'])) { + || ! PMA_DBI_select_db($GLOBALS['cfg']['Server']['pmadb'], $GLOBALS['controllink']) + ) { // No server selected -> no bookmark table // we return the array with the falses in it, // to avoid some 'Unitialized string offset' errors later @@ -304,8 +313,9 @@ function PMA__getRelationsParam() PMA_DBI_free_result($mime_rs); if ($mime_field_mimetype - && $mime_field_transformation - && $mime_field_transformation_options) { + && $mime_field_transformation + && $mime_field_transformation_options + ) { $cfgRelation['mimework'] = true; } } else { @@ -344,11 +354,12 @@ function PMA__getRelationsParam() } if ($cfgRelation['relwork'] && $cfgRelation['displaywork'] - && $cfgRelation['pdfwork'] && $cfgRelation['commwork'] - && $cfgRelation['mimework'] && $cfgRelation['historywork'] - && $cfgRelation['recentwork'] && $cfgRelation['uiprefswork'] - && $cfgRelation['trackingwork'] && $cfgRelation['userconfigwork'] - && $cfgRelation['bookmarkwork'] && $cfgRelation['designerwork']) { + && $cfgRelation['pdfwork'] && $cfgRelation['commwork'] + && $cfgRelation['mimework'] && $cfgRelation['historywork'] + && $cfgRelation['recentwork'] && $cfgRelation['uiprefswork'] + && $cfgRelation['trackingwork'] && $cfgRelation['userconfigwork'] + && $cfgRelation['bookmarkwork'] && $cfgRelation['designerwork'] + ) { $cfgRelation['allworks'] = true; } @@ -359,12 +370,14 @@ function PMA__getRelationsParam() * Gets all Relations to foreign tables for a given table or * optionally a given column in a table * - * @access public - * @param string $db the name of the db to check for - * @param string $table the name of the table to check for - * @param string $column the name of the column to check for - * @param string $source the source for foreign key information + * @param string $db the name of the db to check for + * @param string $table the name of the table to check for + * @param string $column the name of the column to check for + * @param string $source the source for foreign key information + * * @return array db,table,column + * + * @access public */ function PMA_getForeigners($db, $table, $column = '', $source = 'both') { @@ -448,7 +461,8 @@ function PMA_getForeigners($db, $table, $column = '', $source = 'both') if (isset($GLOBALS[$relations_key][$table])) { foreach ($GLOBALS[$relations_key][$table] as $field => $relations) { if ((! strlen($column) || $column == $field) - && (! isset($foreign[$field]) || ! strlen($foreign[$field]))) { + && (! isset($foreign[$field]) || ! strlen($foreign[$field])) + ) { $foreign[$field] = $relations; } } @@ -461,10 +475,12 @@ function PMA_getForeigners($db, $table, $column = '', $source = 'both') /** * Gets the display field of a table * - * @access public - * @param string $db the name of the db to check for - * @param string $table the name of the table to check for + * @param string $db the name of the db to check for + * @param string $table the name of the table to check for + * * @return string field name + * + * @access public */ function PMA_getDisplayField($db, $table) { @@ -491,8 +507,10 @@ function PMA_getDisplayField($db, $table) */ if ($db == 'information_schema') { switch ($table) { - case 'CHARACTER_SETS': return 'DESCRIPTION'; - case 'TABLES': return 'TABLE_COMMENT'; + case 'CHARACTER_SETS': + return 'DESCRIPTION'; + case 'TABLES': + return 'TABLE_COMMENT'; } } @@ -506,10 +524,12 @@ function PMA_getDisplayField($db, $table) /** * Gets the comments for all rows of a table or the db itself * - * @access public - * @param string the name of the db to check for - * @param string the name of the table to check for + * @param string $db the name of the db to check for + * @param string $table the name of the table to check for + * * @return array [field_name] = comment + * + * @access public */ function PMA_getComments($db, $table = '') { @@ -535,9 +555,11 @@ function PMA_getComments($db, $table = '') /** * Gets the comment for a db * - * @access public - * @param string the name of the db to check for + * @param string $db the name of the db to check for + * * @return string comment + * + * @access public */ function PMA_getDbComment($db) { @@ -568,7 +590,7 @@ function PMA_getDbComment($db) * Gets the comment for a db * * @access public - * @param string the name of the db to check for + * * @return string comment */ function PMA_getDbComments() @@ -598,10 +620,12 @@ function PMA_getDbComments() /** * Set a database comment to a certain value. * - * @access public - * @param string $db the name of the db - * @param string $comment the value of the column + * @param string $db the name of the db + * @param string $comment the value of the column + * * @return boolean true, if comment-query was made. + * + * @access public */ function PMA_setDbComment($db, $comment = '') { @@ -642,10 +666,13 @@ function PMA_setDbComment($db, $comment = '') /** * Set a SQL history entry * - * @param string $db the name of the db - * @param string $table the name of the table - * @param string $username the username - * @param string $sqlquery the sql query + * @param string $db the name of the db + * @param string $table the name of the table + * @param string $username the username + * @param string $sqlquery the sql query + * + * @return nothing + * * @access public */ function PMA_setHistory($db, $table, $username, $sqlquery) @@ -681,27 +708,30 @@ function PMA_setHistory($db, $table, $username, $sqlquery) return; } - PMA_query_as_controluser(' - INSERT INTO + PMA_query_as_controluser( + 'INSERT INTO ' . PMA_backquote($cfgRelation['db']) . '.' . PMA_backquote($cfgRelation['history']) . ' (`username`, `db`, `table`, `timevalue`, `sqlquery`) - VALUES + VALUES (\'' . PMA_sqlAddSlashes($username) . '\', \'' . PMA_sqlAddSlashes($db) . '\', \'' . PMA_sqlAddSlashes($table) . '\', NOW(), - \'' . PMA_sqlAddSlashes($sqlquery) . '\')'); + \'' . PMA_sqlAddSlashes($sqlquery) . '\')' + ); } // end of 'PMA_setHistory()' function /** * Gets a SQL history entry * - * @param string $username the username + * @param string $username the username + * * @return array list of history items + * * @access public */ function PMA_getHistory($username) @@ -729,7 +759,10 @@ function PMA_getHistory($username) * deletes entries that exceeds $cfg['QueryHistoryMax'], oldest first, for the * given user * - * @param string $username the username + * @param string $username the username + * + * @return nothing + * * @access public */ function PMA_purgeHistory($username) @@ -751,20 +784,21 @@ function PMA_purgeHistory($username) LIMIT ' . $GLOBALS['cfg']['QueryHistoryMax'] . ', 1'; if ($max_time = PMA_DBI_fetch_value($search_query, 0, 0, $GLOBALS['controllink'])) { - PMA_query_as_controluser(' - DELETE FROM + PMA_query_as_controluser( + 'DELETE FROM ' . PMA_backquote($cfgRelation['db']) . '.' . PMA_backquote($cfgRelation['history']) . ' WHERE `username` = \'' . PMA_sqlAddSlashes($username) . '\' - AND `timevalue` <= \'' . $max_time . '\''); + AND `timevalue` <= \'' . $max_time . '\'' + ); } } // end of 'PMA_purgeHistory()' function /** * Prepares the dropdown for one mode * - * @param array $foreign the keys and values for foreigns - * @param string $data the current data of the dropdown - * @param string $mode the needed mode + * @param array $foreign the keys and values for foreigns + * @param string $data the current data of the dropdown + * @param string $mode the needed mode * * @return array the