Bug 4069 - query returns Result even when query result is FALSE

system/BaseModel.php

@@ -357,7 +357,7 @@ abstract protected function doFirst();
 	 *
 	 * @param array $data Data
 	 *
-	 * @return object|integer|string|false
+	 * @return integer|string|boolean
 	 */
 	abstract protected function doInsert(array $data);
 
@@ -407,7 +407,7 @@ abstract protected function doUpdateBatch(array $set = null, string $index = nul
 	 * @param integer|string|array|null $id    The rows primary key(s)
 	 * @param boolean                   $purge Allows overriding the soft deletes setting.
 	 *
-	 * @return object|boolean
+	 * @return string|boolean
 	 *
 	 * @throws DatabaseException
 	 */
@@ -666,11 +666,7 @@ public function save($data): bool
 		{
 			$response = $this->insert($data, false);
 
-			if ($response instanceof BaseResult)
-			{
-				$response = $response->resultID !== false;
-			}
-			elseif ($response !== false)
+			if ($response !== false)
 			{
 				$response = true;
 			}
@@ -708,7 +704,7 @@ public function getInsertID()
 	 * @param array|object|null $data     Data
 	 * @param boolean           $returnID Whether insert ID should be returned or not.
 	 *
-	 * @return BaseResult|object|integer|string|false
+	 * @return integer|string|boolean
 	 *
 	 * @throws ReflectionException
 	 */

system/Database/BaseBuilder.php

@@ -2252,7 +2252,7 @@ public function getCompiledInsert(bool $reset = true)
 	 *
 	 * @throws DatabaseException
 	 *
-	 * @return BaseResult|Query|false
+	 * @return Query|boolean
 	 */
 	public function insert(array $set = null, bool $escape = null)
 	{
@@ -2483,7 +2483,7 @@ public function update(array $set = null, $where = null, int $limit = null): boo
 
 			$result = $this->db->query($sql, $this->binds, false);
 
-			if ($result->resultID !== false)
+			if ($result !== false)
 			{
 				// Clear our binds so we don't eat up memory
 				$this->binds = [];
@@ -2835,7 +2835,7 @@ public function getCompiledDelete(bool $reset = true): string
 	 * @param integer $limit     The limit clause
 	 * @param boolean $resetData
 	 *
-	 * @return mixed
+	 * @return string|boolean
 	 * @throws DatabaseException
 	 */
 	public function delete($where = '', int $limit = null, bool $resetData = true)

system/Database/BaseConnection.php

@@ -612,7 +612,7 @@ abstract protected function execute(string $sql);
 	 * @param boolean $setEscapeFlags
 	 * @param string  $queryClass
 	 *
-	 * @return BaseResult|Query|false
+	 * @return BaseResult|Query|boolean
 	 *
 	 * @todo BC set $queryClass default as null in 4.1
 	 */
@@ -625,7 +625,6 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
 			$this->initialize();
 		}
 
-		$resultClass = str_replace('Connection', 'Result', get_class($this));
 		/**
 		 * @var Query $query
 		 */
@@ -682,7 +681,7 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
 				Events::trigger('DBQuery', $query);
 			}
 
-			return new $resultClass($this->connID, $this->resultID);
+			return false;
 		}
 
 		$query->setDuration($startTime);
@@ -696,7 +695,20 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
 		// If $pretend is true, then we just want to return
 		// the actual query object here. There won't be
 		// any results to return.
-		return $this->pretend ? $query : new $resultClass($this->connID, $this->resultID);
+		if ($this->pretend)
+		{
+			return $query;
+		}
+
+		// resultID is not false, so it must be successful
+		if ($this->isWriteType($sql))
+		{
+			return true;
+		}
+
+		// query is not write-type, so it must be read-type query; return QueryResult
+		$resultClass = str_replace('Connection', 'Result', get_class($this));
+		return new $resultClass($this->connID, $this->resultID);
 	}
 
 	//--------------------------------------------------------------------
@@ -1755,6 +1767,19 @@ public function resetDataCache()
 
 	//--------------------------------------------------------------------
 
+	/**
+	 * Determines if the statement is a write-type query or not.
+	 *
+	 * @param  string $sql
+	 * @return boolean
+	 */
+	public function isWriteType($sql): bool
+	{
+		return (bool) preg_match('/^\s*"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX|MERGE)\s/i', $sql);
+	}
+
+	//--------------------------------------------------------------------
+
 	/**
 	 * Returns the last error code and message.
 	 *

system/Database/BaseUtils.php

@@ -120,7 +120,7 @@ public function databaseExists(string $databaseName): bool
 	 * Optimize Table
 	 *
 	 * @param  string $tableName
-	 * @return mixed
+	 * @return boolean
 	 * @throws DatabaseException
 	 */
 	public function optimizeTable(string $tableName)
@@ -135,13 +135,8 @@ public function optimizeTable(string $tableName)
 		}
 
 		$query = $this->db->query(sprintf($this->optimizeTable, $this->db->escapeIdentifiers($tableName)));
-		if ($query !== false)
-		{
-			$query = $query->getResultArray();
-			return current($query);
-		}
 
-		return false;
+		return ($query !== false) ? true : false;
 	}
 
 	//--------------------------------------------------------------------

system/Database/ConnectionInterface.php

@@ -116,7 +116,6 @@ public function getPlatform(): string;
 	public function getVersion(): string;
 
 	//--------------------------------------------------------------------
-
 	/**
 	 * Orchestrates a query against the database. Queries must use
 	 * Database\Statement objects to store the query and build it.
@@ -128,7 +127,7 @@ public function getVersion(): string;
 	 * @param string $sql
 	 * @param mixed  ...$binds
 	 *
-	 * @return mixed
+	 * @return BaseResult|Query|boolean
 	 */
 	public function query(string $sql, $binds = null);
 
@@ -193,4 +192,12 @@ public function escape($str);
 	public function callFunction(string $functionName, ...$params);
 
 	//--------------------------------------------------------------------
+
+	/**
+	 * Determines if the statement is a write-type query or not.
+	 *
+	 * @param  string $sql
+	 * @return boolean
+	 */
+	public function isWriteType($sql): bool;
 }

system/Database/Postgre/Connection.php

@@ -609,4 +609,24 @@ protected function _transRollback(): bool
 	}
 
 	// --------------------------------------------------------------------
+
+	/**
+	 * Determines if a query is a "write" type.
+	 *
+	 * Overrides BaseConnection::isWriteType, adding additional read query types.
+	 *
+	 * @param  string $sql An SQL query string
+	 * @return boolean
+	 */
+	public function isWriteType($sql): bool
+	{
+		if (preg_match('#^(INSERT|UPDATE).*RETURNING\s.+(\,\s?.+)*$#is', $sql))
+		{
+			return false;
+		}
+
+		return parent::isWriteType($sql);
+	}
+
+	// --------------------------------------------------------------------
 }

system/Database/Query.php

@@ -280,10 +280,7 @@ public function getErrorMessage(): string
 	 */
 	public function isWriteType(): bool
 	{
-		return (bool) preg_match(
-			'/^\s*"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX)\s/i',
-			$this->originalQueryString
-		);
+		return $this->db->isWriteType($this->originalQueryString);
 	}
 
 	/**

system/Database/SQLSRV/Connection.php

@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\Database\SQLSRV;
 
+use CodeIgniter\Database\Query;
 use CodeIgniter\Database\BaseConnection;
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use Exception;
@@ -501,17 +502,6 @@ public function execute(string $sql)
 		return $stmt;
 	}
 
-	/**
-	 * Determines if a query is a "write" type.
-	 *
-	 * @param  string $sql An SQL query string
-	 * @return boolean
-	 */
-	public function isWriteType($sql)
-	{
-		return (bool) preg_match('/^\s*"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX|MERGE)\s/i', $sql);
-	}
-
 	/**
 	 * Returns the last error encountered by this connection.
 	 *
@@ -578,4 +568,27 @@ public function getVersion(): string
 
 		return isset($info['SQLServerVersion']) ? $this->dataCache['version'] = $info['SQLServerVersion'] : false;
 	}
+
+	// --------------------------------------------------------------------
+
+	/**
+	 * Determines if a query is a "write" type.
+	 *
+	 * Overrides BaseConnection::isWriteType, adding additional read query types.
+	 *
+	 * @param  string $sql An SQL query string
+	 * @return boolean
+	 */
+	public function isWriteType($sql): bool
+	{
+		if (preg_match('/^\s*"?(EXEC\s*sp_rename)\s/i', $sql))
+		{
+			return true;
+		}
+
+		return parent::isWriteType($sql);
+	}
+
+	// --------------------------------------------------------------------
+
 }

system/Database/SQLite3/Connection.php

@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\Database\SQLite3;
 
+use CodeIgniter\Database\Query;
 use CodeIgniter\Database\BaseConnection;
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use ErrorException;
@@ -488,20 +489,6 @@ protected function _transRollback(): bool
 
 	//--------------------------------------------------------------------
 
-	/**
-	 * Determines if the statement is a write-type query or not.
-	 *
-	 * @return boolean
-	 */
-	public function isWriteType($sql): bool
-	{
-		return (bool) preg_match(
-			'/^\s*"?(SET|INSERT|UPDATE|DELETE|REPLACE|CREATE|DROP|TRUNCATE|LOAD|COPY|ALTER|RENAME|GRANT|REVOKE|LOCK|UNLOCK|REINDEX)\s/i',
-			$sql);
-	}
-
-	//--------------------------------------------------------------------
-
 	/**
 	 * Checks to see if the current install supports Foreign Keys
 	 * and has them enabled.

system/Model.php

@@ -19,6 +19,7 @@
 use CodeIgniter\Database\ConnectionInterface;
 use CodeIgniter\Database\Exceptions\DatabaseException;
 use CodeIgniter\Database\Exceptions\DataException;
+use CodeIgniter\Database\Query;
 use CodeIgniter\Exceptions\ModelException;
 use CodeIgniter\I18n\Time;
 use CodeIgniter\Validation\ValidationInterface;
@@ -253,7 +254,7 @@ protected function doFirst()
 	 *
 	 * @param array $data Data
 	 *
-	 * @return BaseResult|integer|string|false
+	 * @return Query|boolean
 	 */
 	protected function doInsert(array $data)
 	{
@@ -278,7 +279,7 @@ protected function doInsert(array $data)
 		$result = $builder->insert();
 
 		// If insertion succeeded then save the insert ID
-		if ($result->resultID)
+		if ($result)
 		{
 			if (! $this->useAutoIncrement)
 			{
@@ -379,7 +380,7 @@ protected function doUpdateBatch(array $set = null, string $index = null, int $b
 	 * @param integer|string|array|null $id    The rows primary key(s)
 	 * @param boolean                   $purge Allows overriding the soft deletes setting.
 	 *
-	 * @return BaseResult|boolean
+	 * @return string|boolean
 	 *
 	 * @throws DatabaseException
 	 */

system/Test/Mock/MockConnection.php

@@ -48,7 +48,7 @@ public function shouldReturn(string $method, $return)
 	 * @param boolean $setEscapeFlags
 	 * @param string  $queryClass
 	 *
-	 * @return BaseResult|Query|false
+	 * @return BaseResult|Query|boolean
 	 *
 	 * @todo BC set $queryClass default as null in 4.1
 	 */
@@ -81,8 +81,14 @@ public function query(string $sql, $binds = null, bool $setEscapeFlags = true, s
 
 		$query->setDuration($startTime);
 
-		$resultClass = str_replace('Connection', 'Result', get_class($this));
+		// resultID is not false, so it must be successful
+		if ($query->isWriteType())
+		{
+			return true;
+		}
 
+		// query is not write-type, so it must be read-type query; return QueryResult
+		$resultClass = str_replace('Connection', 'Result', get_class($this));
 		return new $resultClass($this->connID, $this->resultID);
 	}