From cfb8e9a25115f6b8bd62718be7d55b036883b17a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marcin=20Cie=C5=9Blak?= Date: Fri, 9 Mar 2012 17:24:57 +0000 Subject: [PATCH] Handle PostgreSQL transaction errors and improve schema detection * Introduce $wgDebugDBTransactions facility to help figure out what's going on with transactions. Currently PostgreSQL only. PostgresTransactionState can be easily be made more general to trace all sorts of state machinery. * Improve r113408: we don't need to full reconnect on error, rollback is enough. Rolling back breaks search_path, as PostgreSQL can manage sessions settings under transaction therefore we need to improve schema sniffing introduced in r82674 * Introduce few schema handling functions. This could probably be generalized for other databases like DB2 and Oracle. * Fix bug 15816 - Add a switch for SETting the search_path We try to avoid touching search_path at all unless really necessary. Even in this case we append MediaWiki core schema to the front of the list. * No longer add $wgDBmwschema to PostgreSQL role search_path in the installer. This is no longer necessary as setting schema on connect should ReallyWorkNow(tm). * Get rid as much as possible of $wgDBmwschema and bring us one step closer to fix bug 16794 (wgSharedDB support). All references to core MediaWiki schema in PostgreSQL specific code should now use Database::getCoreSchema() unless we know what we are doing. Followup-To: r113408 r82674 --- RELEASE-NOTES-1.20 | 2 + includes/DefaultSettings.php | 5 + includes/db/Database.php | 2 +- includes/db/DatabasePostgres.php | 289 ++++++++++++++++++++--- includes/installer/PostgresInstaller.php | 14 +- includes/installer/PostgresUpdater.php | 13 +- 6 files changed, 264 insertions(+), 61 deletions(-) diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20 index ea774819dd..1172f878bd 100644 --- a/RELEASE-NOTES-1.20 +++ b/RELEASE-NOTES-1.20 @@ -22,6 +22,7 @@ production. * (bug 27619) Remove preference option to display broken links as link? * (bug 34896) Update jQuery JSON plugin to v2.3 (2011-09-17) * (bug 34302) Add CSS classes to email fields in user preferences +* Introduced $wgDebugDBTransactions to trace transaction status (currently PostgreSQL only) === Bug fixes in 1.20 === * (bug 30245) Use the correct way to construct a log page title. @@ -42,6 +43,7 @@ production. Special:MyPage and Special:MyTalk * (bug 34929) Show the correct diff when a section edit is rejected by the spam filter +* (bug 15816) Add a switch for SETting the search_path (Postgres) === API changes in 1.20 === * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API. diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 827a094012..e214b7b302 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -4059,6 +4059,11 @@ $wgDebugRawPage = false; */ $wgDebugComments = false; +/** + * Extensive database transaction state debugging + */ +$wgDebugDBTransactions = false; + /** * Write SQL queries to the debug log */ diff --git a/includes/db/Database.php b/includes/db/Database.php index e60f3875cb..47eb596d6a 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -784,7 +784,7 @@ abstract class DatabaseBase implements DatabaseType { * @return bool */ function isWriteQuery( $sql ) { - return !preg_match( '/^(?:SELECT|BEGIN|COMMIT|SET|SHOW|\(SELECT)\b/i', $sql ); + return !preg_match( '/^(?:SELECT|BEGIN|ROLLBACK|COMMIT|SET|SHOW|\(SELECT)\b/i', $sql ); } /** diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index 1f1d7ed1b4..0155f282e0 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -16,8 +16,6 @@ class PostgresField implements Field { * @return null|PostgresField */ static function fromText( $db, $table, $field ) { - global $wgDBmwschema; - $q = <<tableName( $table, 'raw' ); $res = $db->query( sprintf( $q, - $db->addQuotes( $wgDBmwschema ), + $db->addQuotes( $db->getCoreSchema() ), $db->addQuotes( $table ), $db->addQuotes( $field ) ) @@ -97,6 +95,84 @@ SQL; } +/** + * Used to debug transaction processing + * Only used if $wgDebugDBTransactions is true + * + * @since 1.20 + * @ingroup Database + */ +class PostgresTransactionState { + + static $WATCHED = array( + array( + "desc" => "Connection state changed from %s -> %s\n", + "states" => array( + PGSQL_CONNECTION_OK => "OK", + PGSQL_CONNECTION_BAD => "BAD" + ) + ), + array( + "desc" => "Transaction state changed from %s -> %s\n", + "states" => array( + PGSQL_TRANSACTION_IDLE => "IDLE", + PGSQL_TRANSACTION_ACTIVE => "ACTIVE", + PGSQL_TRANSACTION_INTRANS => "TRANS", + PGSQL_TRANSACTION_INERROR => "ERROR", + PGSQL_TRANSACTION_UNKNOWN => "UNKNOWN" + ) + ) + ); + + public function __construct( $conn ) { + $this->mConn = $conn; + $this->update(); + $this->mCurrentState = $this->mNewState; + } + + public function update() { + $this->mNewState = array( + pg_connection_status( $this->mConn ), + pg_transaction_status( $this->mConn ) + ); + } + + public function check() { + global $wgDebugDBTransactions; + $this->update(); + if ( $wgDebugDBTransactions ) { + if ( $this->mCurrentState !== $this->mNewState ) { + $old = reset( $this->mCurrentState ); + $new = reset( $this->mNewState ); + foreach ( self::$WATCHED as $watched ) { + if ($old !== $new) { + $this->log_changed($old, $new, $watched); + } + $old = next( $this->mCurrentState ); + $new = next( $this->mNewState ); + + } + } + } + $this->mCurrentState = $this->mNewState; + } + + protected function describe_changed( $status, $desc_table ) { + if( isset( $desc_table[$status] ) ) { + return $desc_table[$status]; + } else { + return "STATUS " . $status; + } + } + + protected function log_changed( $old, $new, $watched ) { + wfDebug(sprintf($watched["desc"], + $this->describe_changed( $old, $watched["states"] ), + $this->describe_changed( $new, $watched["states"] )) + ); + } +} + /** * @ingroup Database */ @@ -136,9 +212,8 @@ class DatabasePostgres extends DatabaseBase { } function hasConstraint( $name ) { - global $wgDBmwschema; $SQL = "SELECT 1 FROM pg_catalog.pg_constraint c, pg_catalog.pg_namespace n WHERE c.connamespace = n.oid AND conname = '" . - pg_escape_string( $this->mConn, $name ) . "' AND n.nspname = '" . pg_escape_string( $this->mConn, $wgDBmwschema ) ."'"; + pg_escape_string( $this->mConn, $name ) . "' AND n.nspname = '" . pg_escape_string( $this->mConn, $this->mConn->getCoreSchema() ) ."'"; $res = $this->doQuery( $SQL ); return $this->numRows( $res ); } @@ -177,10 +252,6 @@ class DatabasePostgres extends DatabaseBase { $connectVars['port'] = $port; } $this->connectString = $this->makeConnectionString( $connectVars, PGSQL_CONNECT_FORCE_NEW ); - $this->reOpen(); - } - - function reOpen() { $this->close(); $this->installErrorHandler(); $this->mConn = pg_connect( $this->connectString ); @@ -194,6 +265,7 @@ class DatabasePostgres extends DatabaseBase { } $this->mOpened = true; + $this->mTransactionState = new PostgresTransactionState( $this->mConn ); global $wgCommandLineMode; # If called from the command-line (e.g. importDump), only show errors @@ -207,12 +279,7 @@ class DatabasePostgres extends DatabaseBase { $this->query( "SET standard_conforming_strings = on", __METHOD__ ); global $wgDBmwschema; - if ( $this->schemaExists( $wgDBmwschema ) ) { - $safeschema = $this->addIdentifierQuotes( $wgDBmwschema ); - $this->doQuery( "SET search_path = $safeschema" ); - } else { - $this->doQuery( "SET search_path = public" ); - } + $this->determineCoreSchema( $wgDBmwschema ); return $this->mConn; } @@ -248,17 +315,20 @@ class DatabasePostgres extends DatabaseBase { } protected function doQuery( $sql ) { + global $wgDebugDBTransactions; if ( function_exists( 'mb_convert_encoding' ) ) { $sql = mb_convert_encoding( $sql, 'UTF-8' ); } + $this->mTransactionState->check(); $this->mLastResult = pg_query( $this->mConn, $sql ); - $this->mAffectedRows = null; // use pg_affected_rows(mLastResult) + $this->mTransactionState->check(); + $this->mAffectedRows = null; return $this->mLastResult; } function reportQueryError( $error, $errno, $sql, $fname, $tempIgnore = false ) { + /* Transaction stays in the ERROR state until rolledback */ $this->rollback( __METHOD__ ); - $this->reOpen(); parent::reportQueryError( $error, $errno, $sql, $fname, $tempIgnore ); } @@ -502,7 +572,7 @@ class DatabasePostgres extends DatabaseBase { $tempsql .= '(' . $this->makeList( $row ) . ')'; if ( $ignore ) { - pg_query( $this->mConn, "SAVEPOINT $ignore" ); + $this->doQuery( "SAVEPOINT $ignore" ); } $tempres = (bool)$this->query( $tempsql, $fname, $ignore ); @@ -510,9 +580,9 @@ class DatabasePostgres extends DatabaseBase { if ( $ignore ) { $bar = pg_last_error(); if ( $bar != false ) { - pg_query( $this->mConn, "ROLLBACK TO $ignore" ); + $this->doQuery( $this->mConn, "ROLLBACK TO $ignore" ); } else { - pg_query( $this->mConn, "RELEASE $ignore" ); + $this->doQuery( $this->mConn, "RELEASE $ignore" ); $numrowsinserted++; } } @@ -527,7 +597,7 @@ class DatabasePostgres extends DatabaseBase { } else { // Not multi, just a lone insert if ( $ignore ) { - pg_query($this->mConn, "SAVEPOINT $ignore"); + $this->doQuery( "SAVEPOINT $ignore" ); } $sql .= '(' . $this->makeList( $args ) . ')'; @@ -535,9 +605,9 @@ class DatabasePostgres extends DatabaseBase { if ( $ignore ) { $bar = pg_last_error(); if ( $bar != false ) { - pg_query( $this->mConn, "ROLLBACK TO $ignore" ); + $this->doQuery( "ROLLBACK TO $ignore" ); } else { - pg_query( $this->mConn, "RELEASE $ignore" ); + $this->doQuery( "RELEASE $ignore" ); $numrowsinserted++; } } @@ -597,7 +667,7 @@ class DatabasePostgres extends DatabaseBase { } $olde = error_reporting( 0 ); $numrowsinserted = 0; - pg_query( $this->mConn, "SAVEPOINT $ignore"); + $this->doQuery( "SAVEPOINT $ignore" ); } $sql = "INSERT INTO $destTable (" . implode( ',', array_keys( $varMap ) ) . ')' . @@ -614,9 +684,9 @@ class DatabasePostgres extends DatabaseBase { if( $ignore ) { $bar = pg_last_error(); if( $bar != false ) { - pg_query( $this->mConn, "ROLLBACK TO $ignore" ); + $this->doQuery( "ROLLBACK TO $ignore" ); } else { - pg_query( $this->mConn, "RELEASE $ignore" ); + $this->doQuery( "RELEASE $ignore" ); $numrowsinserted++; } $olde = error_reporting( $olde ); @@ -702,10 +772,8 @@ class DatabasePostgres extends DatabaseBase { } function listTables( $prefix = null, $fname = 'DatabasePostgres::listTables' ) { - global $wgDBmwschema; - $eschema = $this->addQuotes( $wgDBmwschema ); + $eschema = $this->addQuotes( $this->getCoreSchema() ); $result = $this->query( "SELECT tablename FROM pg_tables WHERE schemaname = $eschema", $fname ); - $endArray = array(); foreach( $result as $table ) { @@ -723,6 +791,48 @@ class DatabasePostgres extends DatabaseBase { return wfTimestamp( TS_POSTGRES, $ts ); } + + /* + * Posted by cc[plus]php[at]c2se[dot]com on 25-Mar-2009 09:12 + * to http://www.php.net/manual/en/ref.pgsql.php + * + * Parsing a postgres array can be a tricky problem, he's my + * take on this, it handles multi-dimensional arrays plus + * escaping using a nasty regexp to determine the limits of each + * data-item. + * + * This should really be handled by PHP PostgreSQL module + * + * @since 1.20 + * @param text string: postgreql array returned in a text form like {a,b} + * @param output string + * @param limit int + * @param offset int + * @return string + */ + + function pg_array_parse( $text, &$output, $limit = false, $offset = 1 ) { + if( false === $limit ) { + $limit = strlen( $text )-1; + $output = array(); + } + if( '{}' != $text ) + do { + if ( '{' != $text{$offset} ) { + preg_match( "/(\\{?\"([^\"\\\\]|\\\\.)*\"|[^,{}]+)+([,}]+)/", + $text, $match, 0, $offset ); + $offset += strlen( $match[0] ); + $output[] = ( '"' != $match[1]{0} + ? $match[1] + : stripcslashes( substr( $match[1], 1, -1 ) ) ); + if ( '},' == $match[3] ) + return $output; + } else + $offset = $this->pg_array_parse( $text, $output[], $limit, $offset+1 ); + } while ( $limit > $offset ); + return $output; + } + /** * Return aggregated value function call */ @@ -737,6 +847,114 @@ class DatabasePostgres extends DatabaseBase { return '[http://www.postgresql.org/ PostgreSQL]'; } + + /** + * Return current schema (executes SELECT current_schema()) + * Needs transaction + * + * @since 1.20 + * @return string return default schema for the current session + */ + function getCurrentSchema() { + $res = $this->query( "SELECT current_schema()", __METHOD__); + $row = $this->fetchRow( $res ); + return $row[0]; + } + + /** + * Return list of schemas which are accessible without schema name + * This is list does not contain magic keywords like "$user" + * Needs transaction + * + * @seealso getSearchPath() + * @seealso setSearchPath() + * @since 1.20 + * @return array list of actual schemas for the current sesson + */ + function getSchemas() { + $res = $this->query( "SELECT current_schemas(false)", __METHOD__); + $row = $this->fetchRow( $res ); + $schemas = array(); + /* PHP pgsql support does not support array type, "{a,b}" string is returned */ + return $this->pg_array_parse($row[0], $schemas); + } + + /** + * Return search patch for schemas + * This is different from getSchemas() since it contain magic keywords + * (like "$user"). + * Needs transaction + * + * @since 1.20 + * @return array how to search for table names schemas for the current user + */ + function getSearchPath() { + $res = $this->query( "SHOW search_path", __METHOD__); + $row = $this->fetchRow( $res ); + /* PostgreSQL returns SHOW values as strings */ + return explode(",", $row[0]); + } + + function setSearchPath( $search_path ) { + /** + * Update search_path, values should already be sanitized + * Values may contain magic keywords like "$user" + * @since 1.20 + * + * @param array list of schemas to be searched by default + */ + $this->query( "SET search_path = " . implode(", ", $search_path) ); + } + + /** + * Determine default schema for MediaWiki core + * Adjust this session schema search path if desired schema exists + * and is not alread there. + * + * We need to have name of the core schema stored to be able + * to query database metadata. + * + * This will be also called by the installer after the schema is created + * + * @since 1.20 + * @param desired_schema string + */ + function determineCoreSchema( $desired_schema ) { + $this->begin( __METHOD__ ); + if ( $this->schemaExists( $desired_schema ) ) { + if ( in_array( $desired_schema, $this->getSchemas() ) ) { + $this->mCoreSchema = $desired_schema; + wfDebug("Schema \"" . $desired_schema . "\" already in the search path\n"); + } else { + /** + * Apped our schema (e.g. 'mediawiki') in front + * of the search path + * Fixes bug 15816 + */ + $search_path = $this->getSearchPath(); + array_unshift( $search_path, + $this->addIdentifierQuotes( $desired_schema )); + $this->setSearchPath( $search_path ); + wfDebug("Schema \"" . $desired_schema . "\" added to the search path\n"); + } + } else { + $this->mCoreSchema = $this->getCurrentSchema(); + wfDebug("Schema \"" . $desired_schema . "\" not found, using current \"". $this->mCoreSchema ."\"\n"); + } + /* Commit SET otherwise it will be rollbacked on error or IGNORE SELECT */ + $this->commit( __METHOD__ ); + } + + /** + * Return schema name fore core MediaWiki tables + * + * @since 1.20 + * @return string core schema name + */ + function getCoreSchema() { + return $this->mCoreSchema; + } + /** * @return string Version information from the database */ @@ -763,12 +981,11 @@ class DatabasePostgres extends DatabaseBase { * @return bool */ function relationExists( $table, $types, $schema = false ) { - global $wgDBmwschema; if ( !is_array( $types ) ) { $types = array( $types ); } if ( !$schema ) { - $schema = $wgDBmwschema; + $schema = $this->getCoreSchema(); } $table = $this->tableName( $table, 'raw' ); $etable = $this->addQuotes( $table ); @@ -795,8 +1012,6 @@ class DatabasePostgres extends DatabaseBase { } function triggerExists( $table, $trigger ) { - global $wgDBmwschema; - $q = <<query( sprintf( $q, - $this->addQuotes( $wgDBmwschema ), + $this->addQuotes( $this->getCoreSchema() ), $this->addQuotes( $table ), $this->addQuotes( $trigger ) ) @@ -819,22 +1034,20 @@ SQL; } function ruleExists( $table, $rule ) { - global $wgDBmwschema; $exists = $this->selectField( 'pg_rules', 'rulename', array( 'rulename' => $rule, 'tablename' => $table, - 'schemaname' => $wgDBmwschema + 'schemaname' => $this->getCoreSchema() ) ); return $exists === $rule; } function constraintExists( $table, $constraint ) { - global $wgDBmwschema; $SQL = sprintf( "SELECT 1 FROM information_schema.table_constraints ". "WHERE constraint_schema = %s AND table_name = %s AND constraint_name = %s", - $this->addQuotes( $wgDBmwschema ), + $this->addQuotes( $this->getCoreSchema() ), $this->addQuotes( $table ), $this->addQuotes( $constraint ) ); diff --git a/includes/installer/PostgresInstaller.php b/includes/installer/PostgresInstaller.php index 544fe85177..cd83c463ee 100644 --- a/includes/installer/PostgresInstaller.php +++ b/includes/installer/PostgresInstaller.php @@ -431,10 +431,6 @@ class PostgresInstaller extends DatabaseInstaller { $conn = $status->value; $dbName = $this->getVar( 'wgDBname' ); - //$schema = $this->getVar( 'wgDBmwschema' ); - //$user = $this->getVar( 'wgDBuser' ); - //$safeschema = $conn->addIdentifierQuotes( $schema ); - //$safeuser = $conn->addIdentifierQuotes( $user ); $exists = $conn->selectField( '"pg_catalog"."pg_database"', '1', array( 'datname' => $dbName ), __METHOD__ ); @@ -466,14 +462,8 @@ class PostgresInstaller extends DatabaseInstaller { } } - // If we created a user, alter it now to search the new schema by default - if ( $this->getVar( '_CreateDBAccount' ) ) { - $conn->query( "ALTER ROLE $safeuser SET search_path = $safeschema, public", - __METHOD__ ); - } - // Select the new schema in the current connection - $conn->query( "SET search_path = $safeschema" ); + $conn->determineCoreSchema( $schema ); return Status::newGood(); } @@ -493,10 +483,8 @@ class PostgresInstaller extends DatabaseInstaller { } $conn = $status->value; - //$schema = $this->getVar( 'wgDBmwschema' ); $safeuser = $conn->addIdentifierQuotes( $this->getVar( 'wgDBuser' ) ); $safepass = $conn->addQuotes( $this->getVar( 'wgDBpassword' ) ); - //$safeschema = $conn->addIdentifierQuotes( $schema ); // Check if the user already exists $userExists = $conn->roleExists( $this->getVar( 'wgDBuser' ) ); diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index 9f4b760ec5..5cee04a834 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -270,7 +270,6 @@ class PostgresUpdater extends DatabaseUpdater { } protected function describeTable( $table ) { - global $wgDBmwschema; $q = <<db->query( sprintf( $q, $this->db->addQuotes( $table ), - $this->db->addQuotes( $wgDBmwschema ) ) ); + $this->db->addQuotes( $this->db->getCoreSchema() ) ) ); if ( !$res ) { return null; } @@ -295,8 +294,6 @@ END; } function describeIndex( $idx ) { - global $wgDBmwschema; - // first fetch the key (which is a list of columns ords) and // the table the index applies to (an oid) $q = <<db->query( sprintf( $q, - $this->db->addQuotes( $wgDBmwschema ), + $this->db->addQuotes( $this->db->getCoreSchea() ), $this->db->addQuotes( $idx ) ) ); @@ -346,7 +343,6 @@ END; } function fkeyDeltype( $fkey ) { - global $wgDBmwschema; $q = <<db->query( sprintf( $q, - $this->db->addQuotes( $wgDBmwschema ), + $this->db->addQuotes( $this->db->getCoreSchema() ), $this->db->addQuotes( $fkey ) ) ); @@ -367,7 +363,6 @@ END; } function ruleDef( $table, $rule ) { - global $wgDBmwschema; $q = <<db->query( sprintf( $q, - $this->db->addQuotes( $wgDBmwschema ), + $this->db->addQuotes( $this->db->getCoreSchema() ), $this->db->addQuotes( $table ), $this->db->addQuotes( $rule ) ) -- 2.20.1