From da9e4b52a54035cfb0f583755cf20cb5186f54cb Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 19 Feb 2020 00:34:41 -0800 Subject: [PATCH] rdbms: Add multi-statement query support to Database Add Database::queryMulti(), which will execute an array of queries as a batch with minimal roundtrips. SQLite fallbacks to looping through each statement and invoking doQuery(). Add QueryStatus class to reduce complexity in Database. Rewrite doQuery() as doSingleStatementQuery(). Change-Id: I3d51083e36ab06fcc1d94558e51b38e106f71bb9 --- autoload.php | 1 + includes/libs/rdbms/database/Database.php | 495 ++++++++++++------ .../libs/rdbms/database/DatabaseMysqlBase.php | 25 +- .../libs/rdbms/database/DatabaseMysqli.php | 76 ++- .../libs/rdbms/database/DatabasePostgres.php | 77 ++- .../libs/rdbms/database/DatabaseSqlite.php | 23 +- .../libs/rdbms/database/utils/QueryStatus.php | 66 +++ .../includes/Revision/RevisionStoreDbTest.php | 9 +- .../includes/db/DatabaseTestHelper.php | 60 +-- .../includes/db/DatabaseMysqlTest.php | 183 +++++++ .../libs/rdbms/database/DatabaseSQLTest.php | 50 +- .../libs/rdbms/database/DatabaseTest.php | 12 +- 12 files changed, 803 insertions(+), 274 deletions(-) create mode 100644 includes/libs/rdbms/database/utils/QueryStatus.php diff --git a/autoload.php b/autoload.php index 64739bfef52..41034bcc699 100644 --- a/autoload.php +++ b/autoload.php @@ -1851,6 +1851,7 @@ $wgAutoloadLocalClasses = [ 'Wikimedia\\Rdbms\\PostgresBlob' => __DIR__ . '/includes/libs/rdbms/encasing/PostgresBlob.php', 'Wikimedia\\Rdbms\\PostgresField' => __DIR__ . '/includes/libs/rdbms/field/PostgresField.php', 'Wikimedia\\Rdbms\\PostgresResultWrapper' => __DIR__ . '/includes/libs/rdbms/database/resultwrapper/PostgresResultWrapper.php', + 'Wikimedia\\Rdbms\\QueryStatus' => __DIR__ . '/includes/libs/rdbms/database/utils/QueryStatus.php', 'Wikimedia\\Rdbms\\ResultWrapper' => __DIR__ . '/includes/libs/rdbms/database/resultwrapper/ResultWrapper.php', 'Wikimedia\\Rdbms\\SQLiteField' => __DIR__ . '/includes/libs/rdbms/field/SQLiteField.php', 'Wikimedia\\Rdbms\\SchemaBuilder' => __DIR__ . '/includes/libs/rdbms/dbal/SchemaBuilder.php', diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 857fc876fd6..9df14a7de1c 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -964,7 +964,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware abstract protected function closeConnection(); /** - * Run a query and return a DBMS-dependent wrapper or boolean + * Run a query and return a QueryStatus instance with the query result information * * This is meant to handle the basic command of actually sending a query to the * server via the driver. No implicit transaction, reconnection, nor retry logic @@ -977,19 +977,45 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware * to doQuery() such that an immediately subsequent call to lastError()/lastErrno() * meaningfully reflects any error that occurred during that public query method call. * - * For SELECT queries, this returns either: - * - a) An IResultWrapper describing the query results + * For SELECT queries, the result field contains either: + * - a) A driver-specific IResultWrapper describing the query results * - b) False, on any query failure * - * For non-SELECT queries, this returns either: - * - a) A driver-specific value/resource, only on success + * For non-SELECT queries, the result field contains either: + * - a) A driver-specific IResultWrapper, only on success * - b) True, only on success (e.g. no meaningful result other than "OK") * - c) False, on any query failure * - * @param string $sql SQL query - * @return IResultWrapper|bool An IResultWrapper, or true on success; false on failure + * @param string $sql Single-statement SQL query + * @return QueryStatus + * @since 1.39 */ - abstract protected function doQuery( $sql ); + abstract protected function doSingleStatementQuery( string $sql ): QueryStatus; + + /** + * Execute a batch of query statements, aborting remaining statements if one fails + * + * @see Database::doQuery() + * + * @stable to override + * @param string[] $sqls Non-empty map of (statement ID => SQL statement) + * @return array Map of (statement ID => QueryStatus) + * @since 1.39 + */ + protected function doMultiStatementQuery( array $sqls ): array { + $qsByStatementId = []; + + $aborted = false; + foreach ( $sqls as $statementId => $sql ) { + $qs = $aborted + ? new QueryStatus( false, 0, 'Query aborted', 0 ) + : $this->doSingleStatementQuery( $sql ); + $qsByStatementId[$statementId] = $qs; + $aborted = ( $qs->res === false ); + } + + return $qsByStatementId; + } /** * Determine whether a query writes to the DB. When in doubt, this returns true. @@ -1207,147 +1233,245 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware : false; } - public function query( $sql, $fname = __METHOD__, $flags = self::QUERY_NORMAL ) { + public function query( $sql, $fname = __METHOD__, $flags = 0 ) { $flags = (int)$flags; // b/c; this field used to be a bool - // Double check that the SQL query is appropriate in the current context and is - // allowed for an outside caller (e.g. does not break session/transaction tracking). + + // Make sure that this caller is allowed to issue this query statement $this->assertQueryIsCurrentlyAllowed( $sql, $fname ); // Send the query to the server and fetch any corresponding errors - list( $ret, $err, $errno, $errFlags ) = $this->executeQuery( $sql, $fname, $flags ); - if ( $ret === false ) { + /** @var QueryStatus $qs */ + $qs = $this->executeQuery( $sql, $fname, $flags, $sql ); + + // Handle any errors that occurred + if ( $qs->res === false ) { // An error occurred; log and report it as needed. Errors that corrupt the state of // the transaction/session cannot be silenced from the client. $ignore = ( $this->fieldHasBit( $flags, self::QUERY_SILENCE_ERRORS ) && - !$this->fieldHasBit( $errFlags, self::ERR_ABORT_SESSION ) && - !$this->fieldHasBit( $errFlags, self::ERR_ABORT_TRX ) + !$this->fieldHasBit( $qs->flags, self::ERR_ABORT_SESSION ) && + !$this->fieldHasBit( $qs->flags, self::ERR_ABORT_TRX ) ); - $this->reportQueryError( $err, $errno, $sql, $fname, $ignore ); + // Throw an error unless both the ignore flag was set and a rollback is not needed + $this->reportQueryError( $qs->message, $qs->code, $sql, $fname, $ignore ); } - return $ret; + return $qs->res; } /** - * Execute a query, retrying it if there is a recoverable connection loss + * Run a batch of SQL query statements and return the results * - * This is similar to query() except: - * - It does not prevent all non-ROLLBACK queries if there is a corrupted transaction - * - It does not disallow raw queries that are supposed to use dedicated IDatabase methods - * - It does not throw exceptions for common error cases + * @see Database::query() * - * This is meant for internal use with Database subclasses. - * - * @param string $sql Original SQL statement query + * @param string[] $sqls Map of (statement ID => SQL statement) * @param string $fname Name of the calling function - * @param int $flags Bit field of class QUERY_* constants - * @return array An n-tuple of: - * - mixed|bool: An object, resource, or true on success; false on failure - * - string: The result of calling lastError() - * - int: The result of calling lastErrno() - * - int: Bit field of ERR_* class constants - * @throws DBUnexpectedError + * @param int $flags Bit field of IDatabase::QUERY_* constants + * @param string $summarySql Virtual SQL for profiling (e.g. "UPSERT INTO TABLE 'x'") + * @return array Ordered map of (statement ID => QueryStatus) + * @since 1.39 */ - final protected function executeQuery( $sql, $fname, $flags ) { - $this->assertHasConnectionHandle(); - - if ( $this->isWriteQuery( $sql, $flags ) ) { - // Do not treat temporary table writes as "meaningful writes" since they are only - // visible to one session and are not permanent. Profile them as reads. Integration - // tests can override this behavior via $flags. - $pseudoPermanent = $this->fieldHasBit( $flags, self::QUERY_PSEUDO_PERMANENT ); - $tempTableChanges = $this->getTempTableWrites( $sql, $pseudoPermanent ); - $isPermWrite = !$tempTableChanges; - foreach ( $tempTableChanges as list( $tmpType ) ) { - $isPermWrite = $isPermWrite || ( $tmpType !== self::TEMP_NORMAL ); - } - // Permit temporary table writes on replica DB connections - // but require a writable primary DB connection for any persistent writes. - if ( $isPermWrite ) { - $this->assertIsWritablePrimary(); - // DBConnRef uses QUERY_REPLICA_ROLE to enforce replica roles for raw SQL queries - if ( $this->fieldHasBit( $flags, self::QUERY_REPLICA_ROLE ) ) { - throw new DBReadOnlyRoleError( - $this, - "Cannot write; target role is DB_REPLICA" - ); - } - } - } else { - // No permanent writes in this query - $isPermWrite = false; - // No temporary tables written to either - $tempTableChanges = []; + public function queryMulti( array $sqls, string $fname, int $flags, string $summarySql ) { + // Make sure that this caller is allowed to issue these query statements + foreach ( $sqls as $sql ) { + $this->assertQueryIsCurrentlyAllowed( $sql, $fname ); + } + + // Send the query statements to the server and fetch the results + $statusByStatementId = $this->executeQuery( $sqls, $fname, $flags, $summarySql ); + // @phan-suppress-next-line PhanTypeSuspiciousNonTraversableForeach + foreach ( $statusByStatementId as $statementId => $qs ) { + if ( $qs->res === false ) { + // An error occurred; log and report it as needed. Errors that corrupt the state of + // the transaction/session cannot be silenced from the client. + $ignore = ( + $this->fieldHasBit( $flags, self::QUERY_SILENCE_ERRORS ) && + !$this->fieldHasBit( $qs->flags, self::ERR_ABORT_SESSION ) && + !$this->fieldHasBit( $qs->flags, self::ERR_ABORT_TRX ) + ); + $this->reportQueryError( + $qs->message, + $qs->code, + $sqls[$statementId], + $fname, + $ignore + ); + } + } + + return $statusByStatementId; + } + + /** + * Execute a set of queries without enforcing public (non-Database) caller restrictions + * + * Retry it if there is a recoverable connection loss (e.g. no important state lost) + * + * This does not precheck for transaction/session state errors or critical section errors + * + * @see Database::query() + * @see Database::querMulti() + * + * @param string|string[] $sqls SQL statment or (statement ID => SQL statement) map + * @param string $fname Name of the calling function + * @param int $flags Bit field of class QUERY_* constants + * @param string $summarySql Actual/simplified SQL for profiling + * @return QueryStatus|array QueryStatus (when given a string statement) + * or ordered map of (statement ID => QueryStatus) (when given an array of statements) + * @throws DBUnexpectedError + * @since 1.34 + */ + final protected function executeQuery( $sqls, $fname, $flags, $summarySql ) { + if ( is_array( $sqls ) ) { + // Query consists of an atomic match of statements + $multiMode = true; + $statementsById = $sqls; + } else { + // Query consists of a single statement + $multiMode = false; + $statementsById = [ '*' => $sqls ]; + } + + $this->assertHasConnectionHandle(); + + $hasPermWrite = false; + $cStatementsById = []; + $tempTableChangesByStatementId = []; + foreach ( $statementsById as $statementId => $sql ) { + if ( $this->isWriteQuery( $sql, $flags ) ) { + $verb = $this->getQueryVerb( $sql ); + // Temporary table writes are not "meaningful" writes, since they are only + // visible to one (ephermal) session, so treat them as reads instead. This + // can be overriden during integration testing via $flags. For simplicity, + // disallow CREATE/DROP statements during multi queries, avoiding the need + // to speculatively track whether a table will be temporary at query time. + if ( $multiMode && in_array( $verb, [ 'CREATE', 'DROP' ] ) ) { + throw new DBUnexpectedError( + $this, + "Cannot issue CREATE/DROP as part of multi-queries" + ); + } + $pseudoPermanent = $this->fieldHasBit( $flags, self::QUERY_PSEUDO_PERMANENT ); + $tempTableChanges = $this->getTempTableWrites( $sql, $pseudoPermanent ); + $isPermWrite = !$tempTableChanges; + foreach ( $tempTableChanges as list( $tmpType ) ) { + $isPermWrite = $isPermWrite || ( $tmpType !== self::TEMP_NORMAL ); + } + // Permit temporary table writes on replica connections, but require a writable + // master connection for writes to persistent tables. Note that this + if ( $isPermWrite ) { + $this->assertIsWritablePrimary(); + // DBConnRef uses QUERY_REPLICA_ROLE to enforce replica roles during query() + if ( $this->fieldHasBit( $flags, self::QUERY_REPLICA_ROLE ) ) { + throw new DBReadOnlyRoleError( + $this, + "Cannot write; target role is DB_REPLICA" + ); + } + } + $hasPermWrite = $hasPermWrite || $isPermWrite; + } else { + // No temporary tables written to either + $tempTableChanges = []; + } + $tempTableChangesByStatementId[$statementId] = $tempTableChanges; + // Add agent and calling method comments to the SQL + $cStatementsById[$statementId] = $this->makeCommentedSql( $sql, $fname ); } - // Add agent and calling method comments to the SQL - $commentedSql = $this->makeCommentedSql( $sql, $fname ); // Whether a silent retry attempt is left for recoverable connection loss errors $retryLeft = !$this->fieldHasBit( $flags, self::QUERY_NO_RETRY ); + $firstStatement = reset( $statementsById ); $cs = $this->commenceCriticalSection( __METHOD__ ); do { // Start a DBO_TRX wrapper transaction as needed (throw an error on failure) - if ( $this->beginIfImplied( $sql, $fname, $flags ) ) { - // Since begin() was called, any connection loss already handled + if ( $this->beginIfImplied( $firstStatement, $fname, $flags ) ) { + // Since begin() was called, any connection loss was already handled $retryLeft = false; } - // Send the query to the server, fetching any results and corresponding errors - // Send the query to the server, fetching any results and corresponding errors. - // Silently retry the query if the error was just a recoverable connection loss. - list( $ret, $err, $errno, $errflags ) = $this->executeQueryAttempt( - $sql, - $commentedSql, - $isPermWrite, + // Send the query statements to the server and fetch any results. Retry all the + // statements if the error was a recoverable connection loss on the first statement. + // To reduce the risk of running queries twice, do not retry the statements if there + // is a connection error during any of the subsequent statements. + $statusByStatementId = $this->attemptQuery( + $statementsById, + $cStatementsById, $fname, - $flags + $summarySql, + $hasPermWrite, + $multiMode ); - // Silently retry the query if the error was just a recoverable connection loss - if ( !$retryLeft ) { - break; - } - $retryLeft = false; - } while ( $this->fieldHasBit( $errflags, self::ERR_RETRY_QUERY ) ); + } while ( + // Query had at least one statement + ( $firstQs = reset( $statusByStatementId ) ) && + // An error occurred that can be recovered from via query retry + $this->fieldHasBit( $firstQs->flags, self::ERR_RETRY_QUERY ) && + // The retry has not been exhausted (consume it now) + $retryLeft && !( $retryLeft = false ) + ); - // Register creation and dropping of temporary tables - $this->registerTempWrites( $ret, $tempTableChanges ); + foreach ( $statusByStatementId as $statementId => $qs ) { + // Register creation and dropping of temporary tables + $this->registerTempWrites( $qs->res, $tempTableChangesByStatementId[$statementId] ); + } $this->completeCriticalSection( __METHOD__, $cs ); - return [ $ret, $err, $errno, $errflags ]; + return $multiMode ? $statusByStatementId : $statusByStatementId['*']; } /** - * Wrapper for doQuery() that handles a single SQL statement query attempt + * Query method wrapper handling profiling, logging, affected row count tracking, and + * automatic reconnections (without retry) on query failure due to connection loss + * + * Note that this does not handle DBO_TRX logic. * * This method handles profiling, debug logging, reconnection and the tracking of: * - write callers * - last write time * - affected row count of the last write * - whether writes occurred in a transaction + * - last successful query time (confirming that the connection was not dropped) * - * This method does *not* handle DBO_TRX transaction logic *nor* query retries. + * @see doSingleStatementQuery() + * @see doMultiStatementQuery() * - * @param string $sql Original SQL statement query - * @param string $commentedSql SQL statement query with debugging/trace comment - * @param bool $isPermWrite Whether the query is a (non-temporary table) write + * @param string[] $statementsById Map of (statement ID => SQL statement) + * @param string[] $cStatementsById Map of (statement ID => commented SQL statement) * @param string $fname Name of the calling function - * @param int $flags Bit field of class QUERY_* constants - * @return array An n-tuple of: - * - mixed|bool: An object, resource, or true on success; false on failure - * - string: The result of calling lastError() - * - int: The result of calling lastErrno() - * - int: Bit field of ERR_* class constants + * @param string $summarySql Actual/simplified SQL for profiling + * @param bool $hasPermWrite Whether any of the queries write to permanent tables + * @param bool $multiMode Whether this is for an anomic statement batch + * @return array Map of (statement ID => statement result) * @throws DBUnexpectedError */ - private function executeQueryAttempt( $sql, $commentedSql, $isPermWrite, $fname, $flags ) { + private function attemptQuery( + array $statementsById, + array $cStatementsById, + string $fname, + string $summarySql, + bool $hasPermWrite, + bool $multiMode + ) { + // Treat empty multi-statement query lists as no query at all + if ( !$statementsById ) { + return []; + } + // Transaction attributes before issuing this query $priorSessInfo = $this->getCriticalSessionInfo(); - // Keep track of whether the transaction has write queries pending - if ( $isPermWrite ) { + // Get the transaction-aware SQL string used for profiling + $prefix = ( $this->topologyRole === self::ROLE_STREAMING_MASTER ) ? 'query-m: ' : 'query: '; + $generalizedSql = new GeneralizedSql( $summarySql, $prefix ); + + $startTime = microtime( true ); + $ps = $this->profiler ? ( $this->profiler )( $generalizedSql->stringify() ) : null; + $this->lastQuery = $summarySql; + $this->affectedRowCount = null; + if ( $hasPermWrite ) { $this->lastWriteTime = microtime( true ); $this->transactionManager->transactionWritingIn( $this->getServerName(), @@ -1355,95 +1479,109 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware ); } - $prefix = ( $this->topologyRole === self::ROLE_STREAMING_MASTER ) ? 'query-m: ' : 'query: '; - $generalizedSql = new GeneralizedSql( $commentedSql, $prefix ); + if ( $multiMode ) { + $qsByStatementId = $this->doMultiStatementQuery( $cStatementsById ); + } else { + $qsByStatementId = [ '*' => $this->doSingleStatementQuery( $cStatementsById['*'] ) ]; + } - $startTime = microtime( true ); - $ps = $this->profiler ? ( $this->profiler )( $generalizedSql->stringify() ) : null; - $this->affectedRowCount = null; - $this->lastQuery = $sql; - $ret = $this->doQuery( $commentedSql ); - $lastError = $this->lastError(); - $lastErrno = $this->lastErrno(); - $this->affectedRowCount = $this->affectedRows(); unset( $ps ); // profile out (if set) $queryRuntime = max( microtime( true ) - $startTime, 0.0 ); - $errflags = self::ERR_NONE; + $lastStatus = end( $qsByStatementId ); + // Use the last affected row count for consistency with lastErrno()/lastError() + $this->affectedRowCount = $lastStatus->rowsAffected; + // Compute the total number of rows affected by all statements in the query + $totalAffectedRowCount = 0; + foreach ( $qsByStatementId as $qs ) { + $totalAffectedRowCount += $qs->rowsAffected; + } - if ( $ret !== false ) { + if ( $lastStatus->res !== false ) { $this->lastPing = $startTime; - // Keep track of whether the transaction has write queries pending - if ( $isPermWrite && $this->trxLevel() ) { + if ( $hasPermWrite && $this->trxLevel() ) { $this->transactionManager->updateTrxWriteQueryReport( - $this->getQueryVerb( $sql ), + $summarySql, $queryRuntime, - $this->affectedRows(), + $totalAffectedRowCount, $fname ); } - } elseif ( $this->isConnectionError( $lastErrno ) ) { - // Connection lost before or during the query... - // Determine how to proceed given the lost session state - $connLossFlag = $this->assessConnectionLoss( $sql, $queryRuntime, $priorSessInfo ); - // Update session state tracking and try to reestablish a connection - $reconnected = $this->replaceLostConnection( $lastErrno, __METHOD__ ); - // Check if important server-side session-level state was lost - if ( $connLossFlag >= self::ERR_ABORT_SESSION ) { - $sessionError = $this->getQueryException( $lastError, $lastErrno, $sql, $fname ); - $this->transactionManager->setSessionError( $sessionError ); - } - // Check if important server-side transaction-level state was lost - if ( $connLossFlag >= self::ERR_ABORT_TRX ) { - $trxError = $this->getQueryException( $lastError, $lastErrno, $sql, $fname ); - $this->transactionManager->setTransactionError( $trxError ); - } - // Check if the query should be retried (having made the reconnection attempt) - if ( $connLossFlag === self::ERR_RETRY_QUERY ) { - $errflags |= ( $reconnected ? self::ERR_RETRY_QUERY : self::ERR_ABORT_QUERY ); - } else { - $errflags |= $connLossFlag; - } - } elseif ( $this->isKnownStatementRollbackError( $lastErrno ) ) { - // Query error triggered a server-side statement-only rollback... - $errflags |= self::ERR_ABORT_QUERY; - if ( $this->trxLevel() ) { - // Allow legacy callers to ignore such errors via QUERY_IGNORE_DBO_TRX and - // try/catch. However, a deprecation notice will be logged on the next query. - $cause = [ $lastError, $lastErrno, $fname ]; - $this->transactionManager->setTrxStatusIgnoredCause( $cause ); - } - } else { - // Some other error occurred during the query... - if ( $this->trxLevel() ) { + } + + $errflags = 0; + $numRowsReturned = 0; + $numRowsAffected = 0; + if ( !$multiMode && $lastStatus->res === false ) { + $lastSql = end( $statementsById ); + $lastError = $lastStatus->message; + $lastErrno = $lastStatus->code; + if ( $this->isConnectionError( $lastErrno ) ) { + // Connection lost before or during the query... + // Determine how to proceed given the lost session state + $connLossFlag = $this->assessConnectionLoss( + $lastSql, + $queryRuntime, + $priorSessInfo + ); + // Update session state tracking and try to reestablish a connection + $reconnected = $this->replaceLostConnection( $lastErrno, __METHOD__ ); + // Check if important server-side session-level state was lost + if ( $connLossFlag >= self::ERR_ABORT_SESSION ) { + $ex = $this->getQueryException( $lastError, $lastErrno, $lastSql, $fname ); + $this->transactionManager->setSessionError( $ex ); + } + // Check if important server-side transaction-level state was lost + if ( $connLossFlag >= self::ERR_ABORT_TRX ) { + $ex = $this->getQueryException( $lastError, $lastErrno, $lastSql, $fname ); + $this->transactionManager->setTransactionError( $ex ); + } + // Check if the query should be retried (having made the reconnection attempt) + if ( $connLossFlag === self::ERR_RETRY_QUERY ) { + $errflags |= ( $reconnected ? self::ERR_RETRY_QUERY : self::ERR_ABORT_QUERY ); + } else { + $errflags |= $connLossFlag; + } + } elseif ( $this->isKnownStatementRollbackError( $lastErrno ) ) { + // Query error triggered a server-side statement-only rollback... + $errflags |= self::ERR_ABORT_QUERY; + if ( $this->trxLevel() ) { + // Allow legacy callers to ignore such errors via QUERY_IGNORE_DBO_TRX and + // try/catch. However, a deprecation notice will be logged on the next query. + $cause = [ $lastError, $lastErrno, $fname ]; + $this->transactionManager->setTrxStatusIgnoredCause( $cause ); + } + } elseif ( $this->trxLevel() ) { + // Some other error occurred during the query, within a transaction... // Server-side handling of errors during transactions varies widely depending on // the RDBMS type and configuration. There are several possible results: (a) the // whole transaction is rolled back, (b) only the queries after BEGIN are rolled // back, (c) the transaction is marked as "aborted" and a ROLLBACK is required // before other queries are permitted. For compatibility reasons, pessimistically // require a ROLLBACK query (not using SAVEPOINT) before allowing other queries. - $trxError = $this->getQueryException( $lastError, $lastErrno, $sql, $fname ); - $this->transactionManager->setTransactionError( $trxError ); + $ex = $this->getQueryException( $lastError, $lastErrno, $lastSql, $fname ); + $this->transactionManager->setTransactionError( $ex ); $errflags |= self::ERR_ABORT_TRX; } else { + // Some other error occurred during the query, without a transaction... $errflags |= self::ERR_ABORT_QUERY; } } - - if ( $sql === self::PING_QUERY ) { - $this->lastRoundTripEstimate = $queryRuntime; + foreach ( $qsByStatementId as $qs ) { + $qs->flags = $errflags; + $numRowsReturned += $qs->rowsReturned; + $numRowsAffected += $qs->rowsAffected; } - $numRows = 0; - if ( $ret instanceof IResultWrapper ) { - $numRows = $ret->numRows(); + if ( !$multiMode && $statementsById['*'] === self::PING_QUERY ) { + $this->lastRoundTripEstimate = $queryRuntime; } $this->transactionManager->recordQueryCompletion( $generalizedSql, $startTime, - $isPermWrite, - $isPermWrite ? $this->affectedRows() : $numRows, + $hasPermWrite, + $hasPermWrite ? $numRowsAffected : $numRowsReturned, $this->getServerName() ); @@ -1453,7 +1591,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware "{method} [{runtime}s] {db_server}: {sql}", $this->getLogContext( [ 'method' => $fname, - 'sql' => $sql, + 'sql' => $summarySql, 'domain' => $this->getDomainID(), 'runtime' => round( $queryRuntime, 3 ), 'db_log_category' => 'query' @@ -1461,12 +1599,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware ); } - if ( !is_bool( $ret ) && $ret !== null && !( $ret instanceof IResultWrapper ) ) { - throw new DBUnexpectedError( $this, - static::class . '::doQuery() should return an IResultWrapper' ); - } - - return [ $ret, $lastError, $lastErrno, $errflags ]; + return $qsByStatementId; } /** @@ -1510,14 +1643,22 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } /** - * Error out if the DB is not in a valid state for a query via query() + * Check if the given query is appropriate to run in a public context + * + * The caller is assumed to come from outside Database. + * In order to keep the DB handle's session state tracking in sync, certain queries + * like "USE", "BEGIN", "COMMIT", and "ROLLBACK" must not be issued directly from + * outside callers. Such commands should only be issued through dedicated methods + * like selectDomain(), begin(), commit(), and rollback(), respectively. + * + * This also checks if the session state tracking was corrupted by a prior exception. * * @param string $sql * @param string $fname * @throws DBUnexpectedError * @throws DBTransactionStateError */ - private function assertQueryIsCurrentlyAllowed( $sql, $fname ) { + private function assertQueryIsCurrentlyAllowed( string $sql, string $fname ) { $verb = $this->getQueryVerb( $sql ); if ( $verb === 'USE' ) { @@ -1526,6 +1667,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware if ( $verb === 'ROLLBACK' ) { // Whole transaction rollback is used for recovery + // @TODO: T269161; prevent "BEGIN"/"COMMIT"/"ROLLBACK" from outside callers return; } @@ -1708,19 +1850,24 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } /** - * Report a query error. Log the error, and if neither the object ignore - * flag nor the $ignoreErrors flag is set, throw a DBQueryError. + * Report a query error + * + * If $ignore is set, emit a DEBUG level log entry and continue, + * otherwise, emit an ERROR level log entry and throw an exception. * * @param string $error * @param int|string $errno * @param string $sql * @param string $fname - * @param bool $ignore + * @param bool $ignore Whether to just log an error rather than throw an exception * @throws DBQueryError */ public function reportQueryError( $error, $errno, $sql, $fname, $ignore = false ) { if ( $ignore ) { - $this->queryLogger->debug( "SQL ERROR (ignored): $error", [ 'db_log_category' => 'query' ] ); + $this->queryLogger->debug( + "SQL ERROR (ignored): $error", + [ 'db_log_category' => 'query' ] + ); } else { throw $this->getQueryExceptionAndLog( $error, $errno, $sql, $fname ); } @@ -2709,7 +2856,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $sqlCondition = $this->makeKeyCollisionCondition( [ $row ], $identityKey ); $this->delete( $table, [ $sqlCondition ], $fname ); $affectedRowCount += $this->affectedRows(); - // Now insert the row + // Insert the new row $this->insert( $table, $row, $fname ); $affectedRowCount += $this->affectedRows(); } @@ -2727,8 +2874,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware * @param array[] $rows Non-empty list of rows * @param string[] $uniqueKey List of columns that define a single unique index * @return string + * @since 1.38 */ - private function makeKeyCollisionCondition( array $rows, array $uniqueKey ) { + protected function makeKeyCollisionCondition( array $rows, array $uniqueKey ) { if ( !$rows ) { throw new DBUnexpectedError( $this, "Empty row array" ); } elseif ( !$uniqueKey ) { @@ -2812,11 +2960,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $affectedRowCount += $this->affectedRows(); } } - $this->endAtomic( $fname ); } catch ( DBError $e ) { $this->cancelAtomic( $fname ); throw $e; } + $this->endAtomic( $fname ); + // Set the affected row count for the whole operation $this->affectedRowCount = $affectedRowCount; } diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php index 16b8dc3b7dc..3f737c4315a 100644 --- a/includes/libs/rdbms/database/DatabaseMysqlBase.php +++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php @@ -178,9 +178,9 @@ abstract class DatabaseMysqlBase extends Database { $flags = self::QUERY_NO_RETRY | self::QUERY_CHANGE_TRX; // Avoid using query() so that replaceLostConnection() does not throw // errors if the transaction status is STATUS_TRX_ERROR - list( $ret, $err, $errno ) = $this->executeQuery( $sql, __METHOD__, $flags ); - if ( $ret === false ) { - $this->reportQueryError( $err, $errno, $sql, __METHOD__ ); + $qs = $this->executeQuery( $sql, __METHOD__, $flags, $sql ); + if ( $qs->res === false ) { + $this->reportQueryError( $qs->message, $qs->code, $sql, __METHOD__ ); } } } catch ( RuntimeException $e ) { @@ -210,12 +210,10 @@ abstract class DatabaseMysqlBase extends Database { } if ( $database !== $this->getDBname() ) { - $sql = 'USE ' . $this->platform->addIdentifierQuotes( $database ); - list( $res, $err, $errno ) = - $this->executeQuery( $sql, __METHOD__, self::QUERY_IGNORE_DBO_TRX ); - - if ( $res === false ) { - $this->reportQueryError( $err, $errno, $sql, __METHOD__ ); + $sql = 'USE ' . $this->addIdentifierQuotes( $database ); + $qs = $this->executeQuery( $sql, __METHOD__, self::QUERY_IGNORE_DBO_TRX, $sql ); + if ( $qs->res === false ) { + $this->reportQueryError( $qs->message, $qs->code, $sql, __METHOD__ ); return false; // unreachable } } @@ -446,6 +444,8 @@ abstract class DatabaseMysqlBase extends Database { } /** + * Escape special characters in a string for use in an SQL statement + * * @param string $s * @return mixed */ @@ -1051,7 +1051,6 @@ abstract class DatabaseMysqlBase extends Database { protected function doFlushSession( $fname ) { $flags = self::QUERY_CHANGE_LOCKS | self::QUERY_NO_RETRY; - // Note that RELEASE_ALL_LOCKS() is not supported well enough to use here. // https://mariadb.com/kb/en/release_all_locks/ $releaseLockFields = []; @@ -1061,9 +1060,9 @@ abstract class DatabaseMysqlBase extends Database { } if ( $releaseLockFields ) { $sql = 'SELECT ' . implode( ',', $releaseLockFields ); - list( $res, $err, $errno ) = $this->executeQuery( $sql, __METHOD__, $flags ); - if ( $res === false ) { - $this->reportQueryError( $err, $errno, $sql, $fname, true ); + $qs = $this->executeQuery( $sql, __METHOD__, $flags, $sql ); + if ( $qs->res === false ) { + $this->reportQueryError( $qs->message, $qs->code, $sql, $fname, true ); } } } diff --git a/includes/libs/rdbms/database/DatabaseMysqli.php b/includes/libs/rdbms/database/DatabaseMysqli.php index ef0403359b2..dc6da3621f7 100644 --- a/includes/libs/rdbms/database/DatabaseMysqli.php +++ b/includes/libs/rdbms/database/DatabaseMysqli.php @@ -40,16 +40,62 @@ use Wikimedia\IPUtils; * @see Database */ class DatabaseMysqli extends DatabaseMysqlBase { - /** - * @param string $sql - * @return MysqliResultWrapper|bool - */ - protected function doQuery( $sql ) { + protected function doSingleStatementQuery( string $sql ): QueryStatus { AtEase::suppressWarnings(); $res = $this->getBindingHandle()->query( $sql ); AtEase::restoreWarnings(); - return $res instanceof mysqli_result ? new MysqliResultWrapper( $this, $res ) : $res; + return new QueryStatus( + $res instanceof mysqli_result ? new MysqliResultWrapper( $this, $res ) : $res, + $this->affectedRows(), + $this->lastError(), + $this->lastErrno() + ); + } + + protected function doMultiStatementQuery( array $sqls ): array { + $qsByStatementId = []; + + $conn = $this->getBindingHandle(); + // Clear any previously left over result + while ( $conn->more_results() && $conn->next_result() ) { + $mysqliResult = $conn->store_result(); + $mysqliResult->free(); + } + + $combinedSql = implode( ";\n", $sqls ); + $conn->multi_query( $combinedSql ); + + reset( $sqls ); + do { + $mysqliResult = $conn->store_result(); + $statementId = key( $sqls ); + if ( $statementId !== null ) { + // Database uses "true" for succesfull queries without result sets + if ( $mysqliResult === false ) { + $res = ( $conn->errno === 0 ); + } elseif ( $mysqliResult instanceof mysqli_result ) { + $res = new MysqliResultWrapper( $this, $mysqliResult ); + } else { + $res = $mysqliResult; + } + $qsByStatementId[$statementId] = new QueryStatus( + $res, + $conn->affected_rows, + $conn->error, + $conn->errno + ); + next( $sqls ); + } + // For error handling, continue regardless of next_result() returning false + } while ( $conn->more_results() && ( $conn->next_result() || true ) ); + // Fill in status for statements aborted due to prior statement failure + while ( ( $statementId = key( $sqls ) ) !== null ) { + $qsByStatementId[$statementId] = new QueryStatus( false, 0, 'Query aborted', 0 ); + next( $sqls ); + } + + return $qsByStatementId; } /** @@ -131,18 +177,12 @@ class DatabaseMysqli extends DatabaseMysqlBase { return $ok ? $mysqli : null; } - /** - * @return bool - */ protected function closeConnection() { $conn = $this->getBindingHandle(); return $conn->close(); } - /** - * @return int - */ public function insertId() { $conn = $this->getBindingHandle(); @@ -154,15 +194,12 @@ class DatabaseMysqli extends DatabaseMysqlBase { */ public function lastErrno() { if ( $this->conn instanceof mysqli ) { - return $this->conn->errno; + return (int)$this->conn->errno; } else { return mysqli_connect_errno(); } } - /** - * @return int - */ protected function fetchAffectedRowCount() { $conn = $this->getBindingHandle(); @@ -175,17 +212,12 @@ class DatabaseMysqli extends DatabaseMysqlBase { */ protected function mysqlError( $conn = null ) { if ( $conn === null ) { - return mysqli_connect_error(); + return (string)mysqli_connect_error(); } else { return $conn->error; } } - /** - * Escapes special characters in a string for use in an SQL statement - * @param string $s - * @return string - */ protected function mysqlRealEscapeString( $s ) { $conn = $this->getBindingHandle(); diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php index f710aa56359..97489387f0e 100644 --- a/includes/libs/rdbms/database/DatabasePostgres.php +++ b/includes/libs/rdbms/database/DatabasePostgres.php @@ -193,28 +193,72 @@ class DatabasePostgres extends Database { !preg_match( '/^SELECT\s+pg_(try_|)advisory_\w+\(/', $sql ); } - /** - * @param string $sql - * @return bool|IResultWrapper - */ - public function doQuery( $sql ) { + public function doSingleStatementQuery( string $sql ): QueryStatus { $conn = $this->getBindingHandle(); $sql = mb_convert_encoding( $sql, 'UTF-8' ); - // Clear previously left over PQresult - while ( $res = pg_get_result( $conn ) ) { - pg_free_result( $res ); + // Clear any previously left over result + while ( $priorRes = pg_get_result( $conn ) ) { + pg_free_result( $priorRes ); } + if ( pg_send_query( $conn, $sql ) === false ) { throw new DBUnexpectedError( $this, "Unable to post new query to PostgreSQL\n" ); } - $this->lastResultHandle = pg_get_result( $conn ); - if ( pg_result_error( $this->lastResultHandle ) ) { - return false; + + // Newer PHP versions use PgSql\Result instead of resource variables + // https://www.php.net/manual/en/function.pg-get-result.php + $pgRes = pg_get_result( $conn ); + $this->lastResultHandle = $pgRes; + $res = pg_result_error( $pgRes ) ? false : $pgRes; + + return new QueryStatus( + is_bool( $res ) ? $res : new PostgresResultWrapper( $this, $conn, $res ), + $this->affectedRows(), + $this->lastError(), + $this->lastErrno() + ); + } + + protected function doMultiStatementQuery( array $sqls ): array { + $qsByStatementId = []; + + $conn = $this->getBindingHandle(); + // Clear any previously left over result + while ( $pgResultSet = pg_get_result( $conn ) ) { + pg_free_result( $pgResultSet ); } - return $this->lastResultHandle ? - new PostgresResultWrapper( $this, $this->getBindingHandle(), $this->lastResultHandle ) : false; + $combinedSql = mb_convert_encoding( implode( ";\n", $sqls ), 'UTF-8' ); + pg_send_query( $conn, $combinedSql ); + + reset( $sqls ); + while ( ( $pgResultSet = pg_get_result( $conn ) ) !== false ) { + $this->lastResultHandle = $pgResultSet; + + $statementId = key( $sqls ); + if ( $statementId !== null ) { + if ( pg_result_error( $pgResultSet ) ) { + $res = false; + } else { + $res = new PostgresResultWrapper( $this, $conn, $pgResultSet ); + } + $qsByStatementId[$statementId] = new QueryStatus( + $res, + pg_affected_rows( $pgResultSet ), + (string)pg_result_error( $pgResultSet ), + pg_result_error_field( $pgResultSet, PGSQL_DIAG_SQLSTATE ) + ); + } + next( $sqls ); + } + // Fill in status for statements aborted due to prior statement failure + while ( ( $statementId = key( $sqls ) ) !== null ) { + $qsByStatementId[$statementId] = new QueryStatus( false, 0, 'Query aborted', 0 ); + next( $sqls ); + } + + return $qsByStatementId; } protected function dumpError() { @@ -510,7 +554,6 @@ __INDEXATTR__; throw $e; } $this->endAtomic( "$fname (outer)" ); - // Set the affected row count for the whole operation $this->affectedRowCount = $affectedRowCount; } @@ -1264,9 +1307,9 @@ SQL; // https://www.postgresql.org/docs/9.1/functions-admin.html $sql = "pg_advisory_unlock_all()"; - list( $res, $err, $errno ) = $this->executeQuery( $sql, __METHOD__, $flags ); - if ( $res === false ) { - $this->reportQueryError( $err, $errno, $sql, $fname, true ); + $qs = $this->executeQuery( $sql, __METHOD__, $flags, $sql ); + if ( $qs->res === false ) { + $this->reportQueryError( $qs->message, $qs->code, $sql, $fname, true ); } } diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index 8a982ebe923..9d90eed6c46 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -28,6 +28,7 @@ use LockManager; use NullLockManager; use PDO; use PDOException; +use PDOStatement; use RuntimeException; use Wikimedia\Rdbms\Platform\ISQLPlatform; use Wikimedia\Rdbms\Platform\SqlitePlatform; @@ -379,18 +380,18 @@ class DatabaseSqlite extends Database { ); } - /** - * @param string $sql - * @return IResultWrapper|bool - */ - protected function doQuery( $sql ) { - $res = $this->getBindingHandle()->query( $sql ); - if ( $res === false ) { - return false; - } + protected function doSingleStatementQuery( string $sql ): QueryStatus { + $conn = $this->getBindingHandle(); - $this->lastAffectedRowCount = $res->rowCount(); - return new SqliteResultWrapper( $res ); + $res = $conn->query( $sql ); + $this->lastAffectedRowCount = $res ? $res->rowCount() : 0; + + return new QueryStatus( + $res instanceof PDOStatement ? new SqliteResultWrapper( $res ) : $res, + $res ? $res->rowCount() : 0, + $this->lastError(), + $this->lastErrno() + ); } protected function doSelectDomain( DatabaseDomain $domain ) { diff --git a/includes/libs/rdbms/database/utils/QueryStatus.php b/includes/libs/rdbms/database/utils/QueryStatus.php new file mode 100644 index 00000000000..d3979c47f3e --- /dev/null +++ b/includes/libs/rdbms/database/utils/QueryStatus.php @@ -0,0 +1,66 @@ +res = $res; + $this->rowsReturned = ( $res instanceof IResultWrapper ) ? $res->numRows() : 0; + $this->rowsAffected = $affected; + $this->message = $error; + $this->code = $errno; + $this->flags = 0; + } +} diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTest.php b/tests/phpunit/includes/Revision/RevisionStoreDbTest.php index de896a7c4d4..78b77271e98 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTest.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTest.php @@ -152,8 +152,13 @@ class RevisionStoreDbTest extends MediaWikiIntegrationTestCase { */ private function getDatabaseMock( array $params ) { $db = $this->getMockBuilder( DatabaseSqlite::class ) - ->onlyMethods( [ 'select', 'doQuery', 'open', 'closeConnection', 'isOpen' ] ) - ->setConstructorArgs( [ $params ] ) + ->onlyMethods( [ + 'select', + 'doSingleStatementQuery', + 'open', + 'closeConnection', + 'isOpen' + ] )->setConstructorArgs( [ $params ] ) ->getMock(); $db->method( 'select' )->willReturn( new FakeResultWrapper( [] ) ); diff --git a/tests/phpunit/includes/db/DatabaseTestHelper.php b/tests/phpunit/includes/db/DatabaseTestHelper.php index 66447c3833f..5801bd020e1 100644 --- a/tests/phpunit/includes/db/DatabaseTestHelper.php +++ b/tests/phpunit/includes/db/DatabaseTestHelper.php @@ -6,6 +6,7 @@ use Psr\Log\NullLogger; use Wikimedia\Rdbms\Database; use Wikimedia\Rdbms\DatabaseDomain; use Wikimedia\Rdbms\FakeResultWrapper; +use Wikimedia\Rdbms\QueryStatus; use Wikimedia\Rdbms\TransactionProfiler; use Wikimedia\RequestTimeout\RequestTimeout; @@ -28,13 +29,11 @@ class DatabaseTestHelper extends Database { */ protected $lastSqls = []; - /** @var array List of row arrays */ - protected $nextResult = []; + /** @var array Stack of result maps */ + protected $nextResMapQueue = []; /** @var array|null */ - protected $nextError = null; - /** @var array|null */ - protected $lastError = null; + protected $lastResMap = null; /** * @var string[] Array of tables to be considered as existing by tableExist() @@ -100,19 +99,17 @@ class DatabaseTestHelper extends Database { /** * @param mixed $res Use an array of row arrays to set row result - */ - public function forceNextResult( $res ) { - $this->nextResult = $res; - } - - /** * @param int $errno Error number * @param string $error Error text * @param array $options * - isKnownStatementRollbackError: Return value for isKnownStatementRollbackError() */ - public function forceNextQueryError( $errno, $error, $options = [] ) { - $this->nextError = [ 'errno' => $errno, 'error' => $error ] + $options; + public function forceNextResult( $res, $errno = 0, $error = '', $options = [] ) { + $this->nextResMapQueue[] = [ + 'res' => $res, + 'errno' => $errno, + 'error' => $error + ] + $options; } protected function addSql( $sql ) { @@ -182,16 +179,16 @@ class DatabaseTestHelper extends Database { } public function lastErrno() { - return $this->lastError ? $this->lastError['errno'] : -1; + return $this->lastResMap ? $this->lastResMap['errno'] : -1; } public function lastError() { - return $this->lastError ? $this->lastError['error'] : 'test'; + return $this->lastResMap ? $this->lastResMap['error'] : 'test'; } protected function isKnownStatementRollbackError( $errno ) { - return ( $this->lastError['errno'] ?? 0 ) === $errno - ? ( $this->lastError['isKnownStatementRollbackError'] ?? false ) + return ( $this->lastResMap['errno'] ?? 0 ) === $errno + ? ( $this->lastResMap['isKnownStatementRollbackError'] ?? false ) : false; } @@ -232,24 +229,25 @@ class DatabaseTestHelper extends Database { $this->forcedAffectedCountQueue = $counts; } - protected function doQuery( $sql ) { + protected function doSingleStatementQuery( string $sql ): QueryStatus { $sql = preg_replace( '< /\* .+? \*/>', '', $sql ); $this->addSql( $sql ); - if ( $this->nextError ) { - $this->lastError = $this->nextError; - $this->nextError = null; - return false; + if ( $this->nextResMapQueue ) { + $this->lastResMap = array_shift( $this->nextResMapQueue ); + if ( !$this->lastResMap['errno'] && $this->forcedAffectedCountQueue ) { + $this->affectedRowCount = array_shift( $this->forcedAffectedCountQueue ); + } + } else { + $this->lastResMap = [ 'res' => [], 'errno' => 0, 'error' => '' ]; } + $res = $this->lastResMap['res']; - $res = $this->nextResult; - $this->nextResult = []; - $this->lastError = null; - - if ( $this->forcedAffectedCountQueue ) { - $this->affectedRowCount = array_shift( $this->forcedAffectedCountQueue ); - } - - return new FakeResultWrapper( $res ); + return new QueryStatus( + is_bool( $res ) ? $res : new FakeResultWrapper( $res ), + $this->affectedRows(), + $this->lastError(), + $this->lastErrno() + ); } } diff --git a/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php b/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php index 2c212b060c4..7ab9f358ea6 100644 --- a/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php +++ b/tests/phpunit/integration/includes/db/DatabaseMysqlTest.php @@ -240,4 +240,187 @@ class DatabaseMysqlTest extends \MediaWikiIntegrationTestCase { return $conn; } + + /** + * @dataProvider provideQueryMulti + * @covers Wikimedia\Rdbms\Database::queryMulti + */ + public function testQueryMulti( + array $sqls, + int $flags, + string $summarySql, + ?array $resMap, + ?array $exception + ) { + $row = $this->conn->query( "SELECT 3 as test", __METHOD__ )->fetchObject(); + $this->assertNotFalse( $row ); + $this->assertSame( '3', $row->test ); + + if ( is_array( $resMap ) ) { + $qsMap = $this->conn->queryMulti( $sqls, __METHOD__, $flags, $summarySql ); + + reset( $resMap ); + foreach ( $qsMap as $qs ) { + if ( is_iterable( $qs->res ) ) { + $this->assertArrayEquals( current( $resMap ), iterator_to_array( $qs->res ) ); + } else { + $this->assertSame( current( $resMap ), $qs->res ); + } + next( $resMap ); + } + } else { + [ $class, $message, $code ] = $exception; + + try { + $this->conn->queryMulti( $sqls, __METHOD__, $flags, $summarySql ); + $this->fail( "No DBError thrown" ); + } catch ( DBError $e ) { + $this->assertInstanceOf( $class, $e ); + $this->assertStringContainsString( $message, $e->getMessage() ); + $this->assertSame( $code, $e->errno ); + } + } + } + + public static function provideQueryMulti() { + return [ + [ + [ + 'SELECT 1 AS v, 2 AS x', + '(SELECT 1 AS v) UNION ALL (SELECT 2 AS v) UNION ALL (SELECT 3 AS v)', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released' + ], + Database::QUERY_IGNORE_DBO_TRX, + 'COMPOSITE QUERY 1', + [ + [ + (object)[ 'v' => '1', 'x' => '2' ] + ], + [ + (object)[ 'v' => '1' ], + (object)[ 'v' => '2' ], + (object)[ 'v' => '3' ] + ], + [ + (object)[ 'free' => '1' ] + ], + [ + (object)[ 'released' => null ] + ] + ], + null + ], + [ + [ + 'SELECT 1 AS v, 2 AS x', + 'SELECT UNION PARSE_ERROR ()', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released' + ], + 0, + 'COMPOSITE QUERY 2', + null, + [ DBQueryError::class, 'You have an error in your SQL syntax', 1064 ] + ], + [ + [ + 'SELECT UNION PARSE_ERROR ()', + 'SELECT 1 AS v, 2 AS x', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released' + ], + 0, + 'COMPOSITE QUERY 3', + null, + [ DBQueryError::class, 'You have an error in your SQL syntax', 1064 ] + ], + [ + [ + 'SELECT 1 AS v, 2 AS x', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released', + 'SELECT UNION PARSE_ERROR ()', + ], + 0, + 'COMPOSITE QUERY 4', + null, + [ DBQueryError::class, 'You have an error in your SQL syntax', 1064 ] + ], + [ + [], + 0, + 'COMPOSITE QUERY 5', + [], + null + ], + [ + [ + 'SELECT 1 AS v, 2 AS x', + 'SELECT UNION PARSE_ERROR ()', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released' + ], + Database::QUERY_SILENCE_ERRORS, + 'COMPOSITE QUERY 2I', + [ + [ + (object)[ 'v' => '1', 'x' => '2' ] + ], + false, + false, + false + ], + null + ], + [ + [ + 'SELECT UNION PARSE_ERROR IGNORE ()', + 'SELECT 1 AS v, 2 AS x', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released' + ], + Database::QUERY_SILENCE_ERRORS, + 'COMPOSITE QUERY 3I', + [ + false, + false, + false, + false + ], + null + ], + [ + [ + 'SELECT 1 AS v, 2 AS x', + 'SELECT IS_FREE_LOCK("unused_lock") AS free', + 'SELECT RELEASE_LOCK("unused_lock") AS released', + 'SELECT UNION PARSE_ERROR ()', + ], + Database::QUERY_SILENCE_ERRORS, + 'COMPOSITE QUERY 4I', + [ + [ + (object)[ 'v' => '1', 'x' => '2' ] + ], + [ + (object)[ 'free' => '1' ] + ], + [ + (object)[ 'released' => null ] + ], + false + ], + null + ], + [ + [], + Database::QUERY_SILENCE_ERRORS, + 'COMPOSITE QUERY 5I', + [], + null + ] + ]; + } + } diff --git a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php index 095287f6e64..c354a6d876d 100644 --- a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php +++ b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php @@ -46,6 +46,45 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { $this->assertEquals( $sqlText, $db->getLastSqls() ); } + /** + * @dataProvider provideQueryMulti + * @covers Wikimedia\Rdbms\Database::queryMulti + */ + public function testQueryMulti( array $sqls, string $summarySql, array $resTriples ) { + $lastSql = null; + reset( $sqls ); + foreach ( $resTriples as [ $res, $errno, $error ] ) { + $this->database->forceNextResult( $res, $errno, $error ); + if ( $lastSql !== null && $errno ) { + $lastSql = current( $sqls ); + } + next( $sqls ); + } + $lastSql = $lastSql ?? end( $sqls ); + $this->database->queryMulti( $sqls, __METHOD__, 0, $summarySql ); + $this->assertLastSql( implode( '; ', $sqls ) ); + } + + public static function provideQueryMulti() { + return [ + [ + [ + 'SELECT 1 AS v', + 'UPDATE page SET page_size=0 WHERE page_id=42', + 'DELETE FROM page WHERE page_id=999', + 'SELECT page_id FROM page LIMIT 3' + ], + 'COMPOSITE page QUERY', + [ + [ [ [ 'v' => 1 ] ], 0, '' ], + [ true, 0, '' ], + [ true, 0, '' ], + [ [ [ 'page_id' => 42 ], [ 'page_id' => 1 ], [ 'page_id' => 11 ] ], 0, '' ] + ] + ] + ]; + } + /** * @dataProvider provideSelect * @covers Wikimedia\Rdbms\Database::select @@ -2321,7 +2360,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { */ public function testImplicitTransactionRollback() { $doError = function () { - $this->database->forceNextQueryError( 666, 'Evilness' ); + $this->database->forceNextResult( false, 666, 'Evilness' ); try { $this->database->delete( 'error', '1', __CLASS__ . '::SomeCaller' ); $this->fail( 'Expected exception not thrown' ); @@ -2381,9 +2420,12 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase { }; $doError = function () { - $this->database->forceNextQueryError( 666, 'Evilness', [ - 'isKnownStatementRollbackError' => true, - ] ); + $this->database->forceNextResult( + false, + 666, + 'Evilness', + [ 'isKnownStatementRollbackError' => true ] + ); try { $this->database->delete( 'error', '1', __CLASS__ . '::SomeCaller' ); $this->fail( 'Expected exception not thrown' ); diff --git a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseTest.php index 512de0c7b67..7469d677f98 100644 --- a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseTest.php +++ b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseTest.php @@ -14,6 +14,7 @@ use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\IResultWrapper; use Wikimedia\Rdbms\LBFactorySingle; use Wikimedia\Rdbms\Platform\SQLPlatform; +use Wikimedia\Rdbms\QueryStatus; use Wikimedia\Rdbms\TransactionManager; use Wikimedia\RequestTimeout\CriticalSectionScope; use Wikimedia\TestingAccessWrapper; @@ -452,7 +453,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { static $abstractMethods = [ 'fetchAffectedRowCount', 'closeConnection', - 'doQuery', + 'doSingleStatementQuery', 'fieldInfo', 'getSoftwareLink', 'getServerVersion', @@ -483,10 +484,19 @@ class DatabaseTest extends PHPUnit\Framework\TestCase { }; $wdb->currentDomain = DatabaseDomain::newUnspecified(); $wdb->platform = new SQLPlatform( new AddQuoterMock() ); + // Info used for logging/errors + $wdb->connectionParams = [ + 'host' => 'localhost', + 'user' => 'testuser' + ]; $db->method( 'getServer' )->willReturn( '*dummy*' ); $db->setTransactionManager( new TransactionManager() ); + $qs = new QueryStatus( false, 0, '', 0 ); + $qs->res = true; + $db->method( 'doSingleStatementQuery' )->willReturn( $qs ); + return $db; }