R::exec(..) improper handling of false in bindings

[Jemt]

Hi,

The following example works: return R::exec("UPDATE mytable SET archived = ? WHERE id = ?", array(true, 68));

The following does not: return R::exec("UPDATE mytable SET archived = ? WHERE id = ? AND archived = ?", array(true, 68, false);

Error thrown: Uncaught [22007] - SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: ''

The error is wrong - the problem has nothing to do with a datetime format. The archived column is defined as tinyint(1) unsigned. The problem relates to how false is handled in bindings.

https://github.com/gabordemooij/redbean/blob/274c426583d91b20a1e39968b2e20969ef83cd1d/RedBeanPHP/Driver/RPDO.php#L139 This line calls canBeTreatedAsInt which returns true for a boolean value of true but returns false for a boolean value of false. So while a boolean with a value of false should be treated as an int, it is not.

Using 1 and 0 as a replacement for true and false allows us to work around the problem, although I strongly prefer true/false.

Sample script to reproduce bug:

<?php

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

require_once(dirname(__FILE__) . "/vendor/autoload.php");
use RedBeanPHP\R;

R::setup("mysql:host=xxx.xxx.xxx.xxx;port=3306;dbname=DB", "USER", "PASS");

$p = R::dispense("person");
$p["name"] = "Mr. Test";
$p["archived"] = false;
R::store($p);

R::exec("UPDATE person SET name = ? WHERE archived = ?", array("Mr. Demo", false));

?>

Fatal error: Uncaught [22007] - SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: '' trace: #0 /Users/tester/www/Demo/Server/vendor/gabordemooij/redbean/RedBeanPHP/Driver/RPDO.php(625): RedBeanPHP\Driver\RPDO->runQuery('UPDATE person S...', Array) #1 /Users/tester/www/Demo/Server/vendor/gabordemooij/redbean/RedBeanPHP/Adapter/DBAdapter.php(107): RedBeanPHP\Driver\RPDO->Execute('UPDATE person S...', Array) #2 /Users/tester/www/Demo/Server/vendor/gabordemooij/redbean/RedBeanPHP/Facade.php(158): RedBeanPHP\Adapter\DBAdapter->exec('UPDATE person S...', Array) #3 /Users/tester/www/Demo/Server/vendor/gabordemooij/redbean/RedBeanPHP/Facade.php(1223): RedBeanPHP\Facade::query('exec', 'UPDATE person S...', Array) #4 /Users/tester/www/Demo/Server/test.php(17): RedBeanPHP\Facade::exec('UPDATE person S...', Array) #5 {main} thrown in /Users/tester/www/Demo/Server/vendor/gabordemooij/redbean/RedBeanPHP/Driver/RPDO.php on line 206

It works fine for a DELETE statement:

$res = R::exec("DELETE FROM person WHERE archived = ?", array(false));

[Jemt]

I have updated the initial bug report with additional details.

[Jemt]

Turns out the use of false when retrieving data doesn't work with SQLite.

<?php

ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

require_once(dirname(__FILE__) . "/vendor/autoload.php");
use RedBeanPHP\R;

R::setup("sqlite:" . dirname(__FILE__) . "/database.db");

$p = R::dispense("person");
$p["name"] = "Mr. Test";
$p["archived"] = false;
R::store($p);

$res = R::findOne("person", "name LIKE ? AND archived = ?", array("Mr. Test", false)); // replace false with 0 to fix it

var_dump($res);

?>

As a more generic solution I'm now patching all binding parameters like so: $this->fixBindingParameters(array(...))

private function fixBindingParameters(array $params)
{
    $fixedParams = array();
    foreach ($params as $param)
    {
        if (is_bool($param) === true)
        {
            $fixedParams[] = $param === true ? 1 : 0;
        }
        else
        {
            $fixedParams[] = $param;
        }
    }
    return $fixedParams;
}

[gabordemooij]

Thank you for reporting, I will look into this.

[gabordemooij]

As far as I can see, there seem to be two issues:

1. RedBeanPHP does not work with MySQL strict mode (https://redbeanphp.com/index.php?p=/requirements), this solves the strange errors in MySQL.

2. As for FALSE-values becoming empty strings, this is because RedBeanPHP simply takes the string representation of the value and let the database compare it. In MySQL ''==0 but in SQLite not. This depends on the database and it is caused by the fact that databases do not have a native boolean data type (and even if they have them they have several problems associated with them). If you want to enforce a cast to an integer type use: array( FALSE, \PDO::PARAM_INT ) as described in: https://redbeanphp.com/index.php?p=/finding, you can also use the functions: pstr() and pint(). However I can see that this might still not be ideal, so I added a toggle to convert FALSE automatically to 0: AQueryWriter::treatFalseBindingsAsInt( TRUE ); - you only have to call this static method once (https://github.com/gabordemooij/redbean/pull/924). Unfortunately I cannot make this the default behavior for SQLite as many people may rely on the existing behavior in some way or another and I rather not break their stuff.

[Jemt]

@gabordemooij Thank you for providing an explanation and solution.

I must have overlooked the fact that RedBeanPHP doesn't support Strict Mode. However, it has been working wonderfully for more than 6 months on our current project.

The documentation takes a rather radical approach, recommending that we clear sql_mode of everything, not just strict modes which are STRICT_TRANS_TABLES and STRICT_ALL_TABLES, as far as I know.

In our case we are working on a managed database with the following modes enabled (SELECT @@sql_mode):

REAL_AS_FLOAT,PIPES_AS_CONCAT,ANSI_QUOTES,IGNORE_SPACE,ONLY_FULL_GROUP_BY,ANSI,STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION

I am wondering if disabling or removing STRICT_TRANS_TABLES and STRICT_ALL_TABLES would suffice for RedBeanPHP to work, or if we have to clear all selected SQL modes ?

[gabordemooij]

I think removing the strict_tables clauses should be okay. The documentation is a bit blunt yes :-)

[Jemt]

Thanks you very much, @gabordemooij :-)