rdbms: fix Sqlite::tableExists() method to avoid STATUS_TRX_ERROR

Sqlite used the base implementation of trying a SELECT 1 query and
seeing if it failed. Instead, make it use the sqlite_master table.
Also remove the base version of that method since it would always
cause this problem and all subclasses have proper implementations.

Make LoadBalancerTest::assertWriteAllowed() more explicit and add
more assertions there.

Change-Id: I6c7b0bea8894c45dfe8931748d6687f0e5d1e101

diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 16e654f..ded1140 100644 (file)
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -1925,18 +1925,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
        }
 
-       public function tableExists( $table, $fname = __METHOD__ ) {
-               $tableRaw = $this->tableName( $table, 'raw' );
-               if ( isset( $this->sessionTempTables[$tableRaw] ) ) {
-                       return true; // already known to exist
-               }
-
-               $table = $this->tableName( $table );
-               $ignoreErrors = true;
-               $res = $this->query( "SELECT 1 FROM $table LIMIT 1", $fname, $ignoreErrors );
-
-               return (bool)$res;
-       }
+       abstract public function tableExists( $table, $fname = __METHOD__ );
 
        public function indexUnique( $table, $index ) {
                $indexInfo = $this->indexInfo( $table, $index );

diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 2125c70..2ebb1a9 100644 (file)
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -519,6 +519,19 @@ class DatabaseSqlite extends Database {
                return $this->lastAffectedRowCount;
        }
 
+       function tableExists( $table, $fname = __METHOD__ ) {
+               $tableRaw = $this->tableName( $table, 'raw' );
+               if ( isset( $this->sessionTempTables[$tableRaw] ) ) {
+                       return true; // already known to exist
+               }
+
+               $encTable = $this->addQuotes( $tableRaw );
+               $res = $this->query(
+                       "SELECT 1 FROM sqlite_master WHERE type='table' AND name=$encTable" );
+
+               return $res->numRows() ? true : false;
+       }
+
        /**
         * Returns information about an index
         * Returns false if the index does not exist

diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php
index cf605c1..d6b43e5 100644 (file)
--- a/tests/phpunit/includes/db/LoadBalancerTest.php
+++ b/tests/phpunit/includes/db/LoadBalancerTest.php
@@ -190,32 +190,38 @@ class LoadBalancerTest extends MediaWikiTestCase {
 
        private function assertWriteAllowed( Database $db ) {
                $table = $db->tableName( 'some_table' );
+               // Trigger a transaction so that rollback() will remove all the tables.
+               // Don't do this for MySQL/Oracle as they auto-commit transactions for DDL
+               // statements such as CREATE TABLE.
+               $useAtomicSection = in_array( $db->getType(), [ 'sqlite', 'postgres', 'mssql' ], true );
                try {
                        $db->dropTable( 'some_table' ); // clear for sanity
+                       $this->assertNotEquals( $db::STATUS_TRX_ERROR, $db->trxStatus() );
 
-                       // Trigger DBO_TRX to create a transaction so the flush below will
-                       // roll everything here back in sqlite. But don't actually do the
-                       // code below inside an atomic section becaue MySQL and Oracle
-                       // auto-commit transactions for DDL statements like CREATE TABLE.
-                       $db->startAtomic( __METHOD__ );
-                       $db->endAtomic( __METHOD__ );
-
+                       if ( $useAtomicSection ) {
+                               $db->startAtomic( __METHOD__ );
+                       }
                        // Use only basic SQL and trivial types for these queries for compatibility
                        $this->assertNotSame(
                                false,
                                $db->query( "CREATE TABLE $table (id INT, time INT)", __METHOD__ ),
                                "table created"
                        );
+                       $this->assertNotEquals( $db::STATUS_TRX_ERROR, $db->trxStatus() );
                        $this->assertNotSame(
                                false,
                                $db->query( "DELETE FROM $table WHERE id=57634126", __METHOD__ ),
                                "delete query"
                        );
+                       $this->assertNotEquals( $db::STATUS_TRX_ERROR, $db->trxStatus() );
                } finally {
-                       // Drop the table to clean up, ignoring any error.
-                       $db->query( "DROP TABLE $table", __METHOD__, true );
-                       // Rollback the DBO_TRX transaction for sqlite's benefit.
+                       if ( !$useAtomicSection ) {
+                               // Drop the table to clean up, ignoring any error.
+                               $db->dropTable( 'some_table' );
+                       }
+                       // Rollback the atomic section for sqlite's benefit.
                        $db->rollback( __METHOD__, 'flush' );
+                       $this->assertNotEquals( $db::STATUS_TRX_ERROR, $db->trxStatus() );
                }
        }
 

diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
index c12882b..8f4aae3 100644 (file)
--- a/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
+++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseTest.php
@@ -440,6 +440,7 @@ class DatabaseTest extends PHPUnit\Framework\TestCase {
                        'numFields', 'numRows',
                        'open',
                        'strencode',
+                       'tableExists'
                ];
                $db = $this->getMockBuilder( Database::class )
                        ->disableOriginalConstructor()