vlucas
PDO treats all values as strings
When you have an entity that has integers or booleans, you currently can't set them.
PDO binds all values by default as strings, so in the query you get something like:
INSERT INTO table WHERE num='1'
The 1 has quotes, treating it like a string, but num is an int field.
You can read about this issue from PDOStatement::execute() http://www.php.net/manual/en/pdostatement.execute.php
This seems to end up making Spot unusable except for string fields.
This is a critical issue. I will have to set aside some time to fix this ASAP. It should just be a bind type switch based on the column type or something. I'm not sure what to do about ad-hoc queries run though Spot though. maybe switched based on the actual variable type?
That is one option, you could also do a read of the table structure and cache that so you know what type to insert it as, you could also let the user specify how to bind each item in the query.
Also, I think it is hilarious that neither of us have come across this before. :P
It is kinda funny... and tremendously sad. The main reason is that MySQL doesn't complain, and PHP auto-converts types without complaining either. The main thing I noticed a few months ago was that all the values coming out of Spot are always strings, regardless of whether or not they are set as INTs or FLOATs in the "fields" array. The only reason I noticed is because of a JSON API I was building and testing types on as I went. I plan to abstract out the type system a little more to ensure the values are always the right types, and this issue just seems like another strong case for that.
I did some work on this very issue in one of the projects I'm currently co-authoring for a PHP MasterClass course I'm taking.
Please check out the this particular file : https://github.com/mikaelg/imputation/blob/b1527ef67002f5723873e9c2ed74b932afc293f6/dirmaster/http/common/classes/Entity.php
Especially the valueMatchesPropertyType() function around line 90.
Classes that extend this Entity class, MUST have a static array which holds the types and mandatory-ness, for example:
Array("type" =>"integer", "mandatory" => true), "lastname" => Array("type" =>"string", "mandatory" => true), "firstname" => Array("type" =>"string", "mandatory" => true), "gender" => Array("type" =>"string", "mandatory" => true), "emailaddresses" => Array("type" =>"\ArrayObject", "mandatory" => false), "addresses" => Array("type" =>"\ArrayObject", "mandatory" => false), "status" => Array("type" =>"string", "mandatory" => false), ); ... ... } > ? > The Entity class - as a parent - controls setting of the variables through a __set() method which first checks if the value matches the property's type through $ccls::$fields and throws an exception if it doesn't. I wrote some unit tests to try to inject "wrong" types into the properties, but it seems that it holds up pretty well. Best regards, Marv P.S.: Eventhough the class is called Entity, it's not an Entity design pattern, I just thought that was the best name for a datastructure like that.
@marvelade Thanks for the input and example code. I plan on doing something like that internally in the PDO driver to bind the different types with different methods. On the actual Entity, I plan on handling it using a new type system. I added a new wiki page called "Roadmap" that outlines my proposed approach: https://github.com/vlucas/spot/wiki/Roadmap
Refactoring the PDO driver for every database type is an option, but makes your code less portable/reusable... Wouldnt it be easier and more flexibele to extend the \PDO class ? Just my .02$ :)
Bumping this. Just ran across the issue again and it is definitely not something we can easily work around. Which solution are we going with here?
I think it would be relatively little work to create a SpotPDO class that extends \PDO and overrides the methods that handle the intake of user data, checking it (and throwing exceptions if need be) before calling their parent.
@vlucas: if you have your hands full a.t.m. I think I can give it a shot, but it's your baby, so you decide :)
The PDO binding is portable, and can be put in the PDO adapter abstract class. There just needs to be a little bit of work making that method and then a big switch statement or something to bind the field "type" to the correct PDO::PARAM_<TYPE>. http://www.php.net/manual/en/pdostatement.bindparam.php
If I wrote the whole type system now (on the "Roadmap" wiki page), I might include a special method for the type class to handle it on it's own. What do you guys think?
@TheSavior : can you post some example code that will reproduce the error with an "expected result:" / "actual result:" ? @vlucas : maybe things can be made more foolproof by also checking the column types with a DESCRIBE query. Then, using a big switch/case you could just bluntly convert the input. Bluntly being ok, IMHO, because if you require a SMALLINT and someone tries to insert "ABC", it's the coders fault that it gets turned into 0. (they would've had chances enough to show/throw an error to the user before the "ABC" made it's way to the DAL)... No?
I just completed the initial type system. I didn't add any default validations in the initial type handlers yet (especially needed around date types), but I wanted to let you guys see where I was headed with it (on the "types" branch): https://github.com/vlucas/Spot/compare/types
This doesn't solve the actual PDO binding yet, but it does ensure all values are casted to their proper types upon setting and getting the value on/off the Entity object. This is the first step to a value type switch that the PDO adapters could then key off of.
The new type system was merged into master, which now paves the way for a proper PDO bind switch based on variable type (the new type system ensures the set variables are of the defined type). I'm not going to be able to do this myself for at least a few weeks because I'll be out speaking at a conference in Amsterdam June 5-10th and preparing my presentations in the meantime. So if someone wants to crack open the PDO abstract adapter and take a shot at the switch statement binding different types, it would be much appreciated.
This comment has a good example of what I was thinking of, for reference: http://www.php.net/manual/en/pdostatement.bindvalue.php#104939
I'm leaving on holiday next week and have tons of work to do before that so it's not very likely that I would find time to look in to it...
Would love to come and listen to your talk about REST next friday in Amsterdam, but due to aforementioned planning madness I won't be able to make it (I'm from Belgium near Antwerp ;) ).
grtz!
I had seen this done in another ORM/AR library but of course I cannot remember which one it was now. It had a bunch of type sniffing checks to automatically figure out which PDO::PARAM_* to use, but I think I like the idea of using a value based on the type plugin system in place for set/get. Seems like it would be less to code than a big loop on every single parameter
