Report zero-row statements as successful in Connection::safeQuery()

htdocs/class/database/manager.php

@@ -329,8 +329,7 @@ public function deleteTables($tables)
         $deleted = array();
         // DBAL 4: connection is established lazily on first query
         foreach ($tables as $key => $val) {
-            //was: if (!$this->db->query("DROP TABLE " . $this->db->prefix($key))) {
-            if (!$this->db->query("DROP TABLE " . $this->db->prefix($val))) {
+            if ($this->db->query("DROP TABLE " . $this->db->prefix($val))) {
                 $deleted[] = $val;
             }
         }

htdocs/class/database/mysqldatabase.php

@@ -223,7 +223,12 @@
     public function getAffectedRows()
     {
         $this->deprecated();
-        return $this->lastResult->rowCount();
+        $result = $this->lastResult;
+        if ($result instanceof \Doctrine\DBAL\Result) {
+            return $result->rowCount();
+        }
+        // Connection::query() returns the count for writes, or true when no rows changed
+        return is_numeric($result) ? (int) $result : 0;
     }
 
     /**

tests/unit/class/database/managerTest.php

@@ -25,4 +25,27 @@ public function test___publicProperties()
             $this->assertTrue($prop->isPublic());
         }
     }
+
+    public function testDeleteTablesReturnsOnlyDroppedTables()
+    {
+        $manager = (new ReflectionClass($this->myclass))->newInstanceWithoutConstructor();
+        // DROP TABLE succeeds (true, no rows affected) for one table and fails (null) for the other
+        $manager->db = new class {
+            public $dropped = array();
+
+            public function prefix($table)
+            {
+                return 'x_' . $table;
+            }
+
+            public function query($sql)
+            {
+                $this->dropped[] = $sql;
+                return (false !== strpos($sql, 'x_missing')) ? null : true;
+            }
+        };
+
+        $this->assertSame(array('items'), $manager->deleteTables(array('items', 'missing')));
+        $this->assertSame(array('DROP TABLE x_items', 'DROP TABLE x_missing'), $manager->db->dropped);
+    }
 }

tests/unit/class/database/mysqldatabaseTest.php

@@ -32,4 +32,26 @@ public function test_genId()
         $x = $instance->genId($sequence);
         $this->assertSame(0, $x);
     }
+
+    #[\PHPUnit\Framework\Attributes\RequiresPhpExtension('pdo_sqlite')]
+    public function testGetAffectedRowsFollowsConnectionQuery()
+    {
+        $instance = new $this->myclass();
+        $instance->conn = new \Xoops\Core\Database\Connection(
+            array('driver' => 'pdo_sqlite', 'memory' => true),
+            new \Doctrine\DBAL\Driver\PDO\SQLite\Driver(),
+            new Configuration()
+        );
+        $instance->conn->setSafe(true);
+
+        // DDL changes no rows: the query succeeds and reports 0 affected rows
+        $this->assertTrue($instance->queryF('CREATE TABLE items (id INTEGER)'));
+        $this->assertSame(0, $instance->getAffectedRows());
+
+        $this->assertSame(2, $instance->queryF('INSERT INTO items VALUES (1), (2)'));
+        $this->assertSame(2, $instance->getAffectedRows());
+
+        $this->assertFalse($instance->queryF('INSERT INTO missing VALUES (1)'));
+        $this->assertSame(0, $instance->getAffectedRows());
+    }
 }

tests/unit/xoopsLib/Xmf/Database/TablesSchemaTest.php

@@ -132,10 +132,6 @@ public function testExecuteQueueRendersCreateTableAndRunsItForced(): void
 
     public function testExecuteQueueAcceptsDdlThatAffectsNoRows(): void
     {
-        $this->markTestIncomplete(
-            'Existing 2.6 bug: Connection::safeQuery() returns null when a statement affects 0 rows, '
-            . 'which every CREATE TABLE does, so Tables::executeQueue() reports a successful CREATE as failed.'
-        );
         $tables = $this->queueCreateTable(self::itemsSchema());
 
         $this->assertTrue($tables->executeQueue(true));
@@ -188,12 +184,12 @@ public function query($sql)
     {
         $this->queries[] = $sql;
         // like Connection::safeQuery(): force lasts for one statement, and a
-        // non-SELECT returns its affected-row count, or null when that is 0
+        // non-SELECT returns its affected-row count, or true when that is 0
         $this->force = false;
         if ($this->fail) {
             $result = false;
         } elseif (stripos(ltrim($sql), 'select') !== 0) {
-            $result = $this->affectedRows ?: null;
+            $result = $this->affectedRows ?: true;
         } else {
             // only x_items exists; every other table comes back empty
             $rows = [];

tests/unit/xoopsLib/Xoops/Core/Database/ConnectionSafeQueryTest.php

@@ -0,0 +1,102 @@
+<?php
+require_once(__DIR__ . '/../../../../init_new.php');
+
+use Doctrine\DBAL\Configuration;
+use Doctrine\DBAL\Driver\PDO\SQLite\Driver;
+use Doctrine\DBAL\Result;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\RequiresPhpExtension;
+use Xoops\Core\Database\Connection;
+
+/**
+ * safeQuery() return values, against an in-memory SQLite database
+ */
+#[CoversClass(Connection::class)]
+#[RequiresPhpExtension('pdo_sqlite')]
+class ConnectionSafeQueryTest extends \PHPUnit\Framework\TestCase
+{
+    /** @var Connection */
+    private $db;
+
+    protected function setUp(): void
+    {
+        $this->db = new Connection(['driver' => 'pdo_sqlite', 'memory' => true], new Driver(), new Configuration());
+        $this->db->setSafe(true);
+    }
+
+    public function testDdlThatChangesNoRowsReportsSuccess(): void
+    {
+        $this->assertTrue($this->db->query('CREATE TABLE items (id INTEGER, title TEXT)'));
+    }
+
+    public function testWriteReturnsAffectedRowCount(): void
+    {
+        $this->db->query('CREATE TABLE items (id INTEGER, title TEXT)');
+
+        $this->assertSame(2, $this->db->query("INSERT INTO items VALUES (1, 'a'), (2, 'b')"));
+        $this->assertSame(1, $this->db->query("UPDATE items SET title = 'c' WHERE id = 2"));
+    }
+
+    public function testWriteMatchingNoRowsReportsSuccess(): void
+    {
+        $this->db->query('CREATE TABLE items (id INTEGER, title TEXT)');
+
+        $this->assertTrue($this->db->query("UPDATE items SET title = 'x' WHERE id = 99"));
+        $this->assertTrue($this->db->safeQuery('DELETE FROM items'));
+    }
+
+    public function testSelectReturnsResult(): void
+    {
+        $this->db->query('CREATE TABLE items (id INTEGER)');
+
+        $result = $this->db->query('SELECT * FROM items');
+
+        $this->assertInstanceOf(Result::class, $result);
+        $this->assertFalse($result->fetchAssociative());
+    }
+
+    public function testFailedStatementReturnsNull(): void
+    {
+        $this->assertNull($this->db->query('CREATE TABLE broken ('));
+        $this->assertNull($this->db->query('SELECT * FROM missing_table'));
+    }
+
+    public function testTablesExecuteQueueCreatesTableThroughConnection(): void
+    {
+        // SQLite has no INFORMATION_SCHEMA, so seed the definition addTable() would have built
+        $tables = (new \ReflectionClass(\Xmf\Database\Tables::class))->newInstanceWithoutConstructor();
+        $state = [
+            'db'     => $this->db,
+            'tables' => ['fresh' => [
+                'name'    => 'x_fresh',
+                'options' => '',
+                'columns' => [['name' => 'id', 'attributes' => 'INTEGER NOT NULL']],
+                'keys'    => ['PRIMARY' => ['columns' => '`id`', 'unique' => true]],
+                'create'  => true,
+            ]],
+            'queue'  => [['createtable' => 'fresh']],
+        ];
+        foreach ($state as $name => $value) {
+            (new \ReflectionProperty(\Xmf\Database\Tables::class, $name))->setValue($tables, $value);
+        }
+
+        $this->assertTrue($tables->executeQueue(true));
+        $this->assertSame(
+            'x_fresh',
+            $this->db->query("SELECT name FROM sqlite_master WHERE name = 'x_fresh'")->fetchOne()
+        );
+    }
+
+    public function testWriteIsRefusedWhenNotSafe(): void
+    {
+        $this->db->query('CREATE TABLE items (id INTEGER)');
+        $this->db->setSafe(false);
+
+        $this->assertNull($this->db->query('INSERT INTO items VALUES (1)'));
+        $this->assertInstanceOf(Result::class, $this->db->query('SELECT * FROM items'));
+
+        $this->db->setForce(true);
+        $this->assertSame(1, $this->db->query('INSERT INTO items VALUES (1)'));
+        $this->assertFalse($this->db->getForce(), 'force applies to one statement');
+    }
+}

xoops_lib/Xoops/Core/Database/Connection.php

@@ -438,7 +438,9 @@ public function rollBack(): void
      * @param array  $params The query parameters
      * @param array  $types  The parameter types
      *
-     * @return Result|int|string|null Result for SELECT, affected rows for DML, null on failure
+     * @return Result|int|string|true|null Result for SELECT; the affected row count for other
+     *                                    statements, or true when one succeeded without changing
+     *                                    rows (CREATE TABLE, an UPDATE matching nothing); null on failure
      */
     public function query(string $sql, array $params = [], array $types = [])
     {
@@ -455,7 +457,9 @@ public function query(string $sql, array $params = [], array $types = [])
      * @param array  $params The query parameters
      * @param array  $types  The parameter types
      *
-     * @return Result|int|string|null Result for SELECT, affected rows for DML, null on failure
+     * @return Result|int|string|true|null Result for SELECT; the affected row count for other
+     *                                    statements, or true when one succeeded without changing
+     *                                    rows (CREATE TABLE, an UPDATE matching nothing); null on failure
      */
     public function safeQuery(string $sql, array $params = [], array $types = [])
     {
@@ -481,7 +485,11 @@ public function safeQuery(string $sql, array $params = [], array $types = [])
             $result = null;
         }
         $events->triggerEvent('core.database.query.end');
-        return $result ?: null;
+        // 0 affected rows is still a success; null must mean the statement failed
+        if ($result === 0 || $result === '0') {
+            return true;
+        }
+        return $result;
     }
 
     /**