Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: db->query returning ResultInterface object for failed queries, doesn't return boolean for write queries #4069

Closed
sneakyimp opened this issue Jan 5, 2021 · 1 comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer

Comments

@sneakyimp
Copy link
Contributor

Describe the bug
If you put your CI4 project in production mode (i.e., CI_ENVIRONMENT=production) then the BaseConnection::query() function will return a ResultInterface object with resultID=false when you run a bad query. This appears to be due to line 662 in system/Database/BaseConnection.php. It is perhaps easy to have overlooked this problem because an exception is usually thrown if you have DBDebug on.

The comments in the source code do not provide much insight into how the BaseConnection::query() function should behave. It's also confusing that it can return numerous different data types: BaseResult|Query|false. According to the db query documentation, this return value is not correct:

The query() function returns a database result object when “read” type queries are run which you can use to show your results. When “write” type queries are run it simply returns TRUE or FALSE depending on success or failure...

I also don't see any way this function could return true under any circumstances. I would also ask if it's wise to be constructing ResultInterface classes when resultID is a boolean value? The var name resultID suggests a resource variable. Perhaps we should revisit the various result classes and make sure we take into account the possibility of a boolean value for resultID?

There should probably include a comprehensive class reference for Database\ConnectionInterface in the documentation somewhere in the documentation. If there is, I haven't been able to find it.

Finally, another minor complication is that mysqli_query actually doesn't return a boolean value for an OPTIMIZE TABLE query, despite what the PHP documentation says:

Returns false on failure. For successful SELECT, SHOW, DESCRIBE or EXPLAIN queries mysqli_query() will return a mysqli_result object. For other successful queries mysqli_query() will return true.

For an OPTIMIZE TABLE query, mysqli_query returns a mysqli_result object -- whether the optimization succeeds or fails. See #4059.

CodeIgniter 4 version
The current develop branch of CodeIgniter 4 has this problem (as of this writing)

Affected module(s)
Database\BaseConnection.php
and anything that utilizes it

Expected behavior, and steps to reproduce if appropriate
According to the docs $query should be false here:

$query = $db->query("SELECT * FROM table_does_not_exist");

To reproduce this problem, set CI_ENVIRONMENT=production, configure your test db credentials, and add this function to a controller then run it:

	function test3() {
		$db = \Config\Database::connect("tests");
		echo get_class($db) . "\n";

		// intentionally bad query		
		$query = $db->query("SELECT * FROM table_does_not_exist");
		var_dump($query);
	}

The output:

=== MySQLi (truncated output for brevity) ===
object(CodeIgniter\Database\MySQLi\Result)#47 (7) {
  ["connID"]=>
  object(mysqli)#45 (19) {
    ...
  }
  ["resultID"]=>
  bool(false)
  ["resultArray"]=>
  array(0) {
  }
  ["resultObject"]=>
  array(0) {
  }
  ["customResultObject"]=>
  array(0) {
  }
  ["currentRow"]=>
  int(0)
  ["rowData"]=>
  NULL
}

=== SQLite3 ===
object(CodeIgniter\Database\SQLite3\Result)#48 (8) {
  ["numRows":protected]=>
  NULL
  ["connID"]=>
  object(SQLite3)#45 (0) {
  }
  ["resultID"]=>
  bool(false)
  ["resultArray"]=>
  array(0) {
  }
  ["resultObject"]=>
  array(0) {
  }
  ["customResultObject"]=>
  array(0) {
  }
  ["currentRow"]=>
  int(0)
  ["rowData"]=>
  NULL
}

=== SQLSRV ===
object(CodeIgniter\Database\SQLSRV\Result)#46 (8) {
  ["rowOffset":"CodeIgniter\Database\SQLSRV\Result":private]=>
  int(0)
  ["connID"]=>
  resource(114) of type (SQL Server Connection)
  ["resultID"]=>
  bool(false)
  ["resultArray"]=>
  array(0) {
  }
  ["resultObject"]=>
  array(0) {
  }
  ["customResultObject"]=>
  array(0) {
  }
  ["currentRow"]=>
  int(0)
  ["rowData"]=>
  NULL
}

Context

  • OS: Ubuntu: 18.04
  • Web server: n/a, run via CLI
  • PHP version: 7.2.24
@sneakyimp sneakyimp added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 5, 2021
@MGatner MGatner added the database Issues or pull requests that affect the database layer label May 18, 2021
@kenjis
Copy link
Member

kenjis commented Oct 13, 2021

Related #4632 (comment)

@kenjis kenjis closed this as completed Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them database Issues or pull requests that affect the database layer
Projects
None yet
Development

No branches or pull requests

3 participants