phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.78k stars 1.96k forks source link

Inconsistent behavior with datatype casting in the result set given by ORM. #12707

Closed adhocore closed 7 years ago

adhocore commented 7 years ago

Expected and Actual Behavior

With Model::setup(['castOnHydrate' => true]); setting, no matter how the data is retrieved and used, it should always have proper data types as long as the result is generated from the Phalcon\Mvc\Model::query() or Phalcon\Mvc\Model\Manager->createBuilder(). Currently this behavior is working as expected with full select (select * from ...) but not with partial select (select col1, col2 from ...) or alias (select col1 as col from ...).

Describe what you are trying to achieve and what goes wrong.

The data types are lost in partial select or when aliases are used.

Sample code and output


Model::setup(['castOnHydrate' => true]);

class Products extends \Phalcon\Mvc\Model { /* @Primary @Identity @Column(type="integer", length=11, nullable=false) / public $id;

/** @Column(type="double", length=10, nullable=false) */
public $price;

public function initialize()
{
    $this->belongsTo('category_id', 'Categories', 'id');
}

}

$qb = Products::find(1); var_dump($qb[0]->toArray()['price']); // double(7.28)

$qb = Products::find(['id=1', 'columns' => 'id,price as old_price']); var_dump($qb[0]->toArray()['old_price']); // string(8) "7.280000"

$qb = Products::find(['id=1', 'columns' => 'id,price']); var_dump($qb[0]->toArray()['price']); // string(8) "7.280000"

$qb = Products::query()->where('id = 1')->execute(); var_dump($qb[0]->toArray()['price']); // double(7.28) var_dump($qb->toArray()[0]['price']); // string(8) "7.280000"

$qb = Products::query()->columns('price')->where('id = 1')->execute(); var_dump($qb[0]->toArray()['price']); // string(8) "7.280000"

$qb = $this->modelsManager->createBuilder() ->addFrom(Products::class, 'p') ->leftJoin(Models\Categories::class, 'p.category_id = c.id', 'c') ->where('p.id = 1') ->getQuery()->execute(); var_dump($qb[0]->toArray()['price']); // double(7.28)

$qb = $this->modelsManager->createBuilder() ->columns('p.id, p.price, c.id as cid') ->addFrom(Products::class, 'p') ->leftJoin(Categories::class, 'p.category_id = c.id', 'c') ->where('p.id = 1') ->getQuery()->execute(); var_dump($qb[0]->toArray()['price']); // string(8) "7.280000"


### Details

* Phalcon version: (`php --ri phalcon`) : 3.0.4
* PHP Version: (`php -v`) : PHP 7.0.16-1~dotdeb+8.1
* Operating System: Debian
* Installation type: installing via package manager
* Zephir version (if any):
* Server: Nginx
* Other related info (Database, table schema): MariaDB 10.1.21
Jurigag commented 7 years ago

Everything is fine. Just when you select columns not full model is returned - it's Row object which doesn't use castOnHydrate. Just install mysqlnd. The behaviour is consistent, once hydration happens - like when you have model your property is hydrated, otherwise you have result returned from database. You need to add:

PDO::ATTR_EMULATE_PREPARES   => false,
PDO::ATTR_STRINGIFY_FETCHES  => false,
PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',

On your db service parameters.

adhocore commented 7 years ago

Thanks @Jurigag, The results above are already after using mysqlnd and those PDO attrs. The point is even for partial select if it is queried from ORM, then should not the data types be consistent because not always do we need to have select *. Also the issue is not just with partial select but even with aliasing.

$qb = Products::find(['id=1', 'columns' => '*,price as p']);
var_dump($qb[0]->toArray()['p']); // string(8) "7.280000"

// the order of toArray produces different results too
$qb = Products::query()->where('id = 1')->execute();
var_dump($qb[0]->toArray()['price']); // double(7.28)
var_dump($qb->toArray()[0]['price']); // string(8) "7.280000"

The thing is we need strict types for JSON response and if there is something like castOnHydrate which works in some cases but not in other cases mean, we have to loop the resultset yet again and do the type casting. What do you suggest? Will it be an overhead to have this casting thing baked in ORM for all such cases?

Jurigag commented 7 years ago

I can tell you right now that they are not, i have similar use cases and i have correct types returned, idk maybe it's only about double but i doubt it. Is same happening for int? No it isn't like it works in some cases but not in other - castOnHydrate always works just only for full models, for partials not. I think yes, it would be overhead, Row object is supposed to be as simple as it can be. Just check for sure if you are using mysqlnd, just do

apt-get remove php-mysql
 apt-get install php-mysqlnd

Or php5-* if you are using php5.

Jurigag commented 7 years ago

Check out my example:

Table schema:

SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";
SET time_zone = "+00:00";

/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
/*!40101 SET NAMES utf8mb4 */;

CREATE TABLE `test` (
  `a` int(11) NOT NULL,
  `b` float NOT NULL,
  `c` double NOT NULL,
  `d` double NOT NULL,
  `id` int(11) NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

INSERT INTO `test` (`a`, `b`, `c`, `d`, `id`) VALUES
(1, 1.5, 1.5, 1.5, 1);

ALTER TABLE `test`
  ADD PRIMARY KEY (`id`);

ALTER TABLE `test`
  MODIFY `id` int(11) NOT NULL AUTO_INCREMENT, AUTO_INCREMENT=2;
/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;

Php code:

use Phalcon\Di\FactoryDefault;

$di = new FactoryDefault();
$di->set('db',function(){
   $adapter = new \Phalcon\Db\Adapter\Pdo\Mysql([
       'host'     => 'localhost',
       'username' => 'root',
       'password' => '',
       'dbname'   => 'phalcon_test',
       'options'  => [
           PDO::ATTR_EMULATE_PREPARES   => false,
           PDO::ATTR_STRINGIFY_FETCHES  => false,
           PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
       ],
       'charset'  => 'utf8',
   ]);

   return $adapter;
});

class Test extends \Phalcon\Mvc\Model
{

}

var_dump(Test::findFirst()->toArray());
var_dump(Test::findFirst(['columns'=>'a,b,c,d'])->toArray());
var_dump($test = Test::find()->toArray());
var_dump($test = Test::find(['columns'=>'a,b,c,d'])->toArray());

Returns:

/vagrant/www/test.php:28:
array (size=5)
  'a' => int 1
  'b' => float 1.5
  'c' => float 1.5
  'd' => float 1.5
  'id' => int 1
/vagrant/www/test.php:29:
array (size=4)
  'a' => int 1
  'b' => float 1.5
  'c' => float 1.5
  'd' => float 1.5
/vagrant/www/test.php:30:
array (size=1)
  0 => 
    array (size=5)
      'a' => int 1
      'b' => float 1.5
      'c' => float 1.5
      'd' => float 1.5
      'id' => int 1
/vagrant/www/test.php:31:
array (size=1)
  0 => 
    array (size=4)
      'a' => int 1
      'b' => float 1.5
      'c' => float 1.5
      'd' => float 1.5

I don't even have castOnHydrate set, you need to use mysqlnd to have correct values returned from database.

adhocore commented 7 years ago

@Jurigag thanks for the elaboration. That's great but we have schema with decimal(10, 4) types and not float or double and these cant be changed for some reasons. It doesn't seem to work with mysql decimal type.

Jurigag commented 7 years ago

Still, not framework problem. As i wrote you above, behaviour is really consistent - you got type casting only on full models, on Row object you got types returned directly from database.

Jurigag commented 7 years ago

Decimal is returned as string to prevent the decimal from becoming an unreliable float in PHP. Still, there is nothing bugged or missing in framework. Cast on hydrate it's making it float, that's the only difference but as i already wrote - cast on hydrate works only on full models.