I came across an issue today involving the ORM package and my database structure. I have an unsigned integer column in my database called payment_hold_period that holds the number of seconds that a payment is held for. This column is allowed null values as we do different things with this field when it is null.
When an integer value is supplied for this field, this is how long the payment is held for before we pay it out to the client. For example, 86400 means 1 day, 604800 means 1 week. If we enter 0 for this field, this means that payouts are not held whatsoever and the client gets paid out right away. Our default hold period is 1209600 (2 weeks) which is stored in a configuration file. If the payment_hold_period contains a null value, then we use the default value of 1209600 from the config file. The null setting is the case for the majority of our clients, so if we change the value in the config, we don't have to update the database since we check for null values to grab the default setting.
In the other 2 cases, we enter an integer value (0 to whatever amount of seconds) to represent how long we wait before paying a client out. The problem I'm running into is the following:
As mentioned, most clients have a null value for the payment_hold_period, which means that they wait 1209600 seconds (2 weeks) before we release their payment. Now if I update this value to anything other than 0, the database updates without any issues. However, if I want to completely bypass the payment hold period for a client, I enter in a value of 0 (for 0 seconds). The problem is, FuelPHP's ORM will not let a null value get updated with 0 when I set the data type to "int" in the model.
The issues lies in the update() method of the model.php file on line 1533 (fuel/orm 1.8.1.2). The code doesn't do a strict check when the data type is set to "int". So essentially, Fuel ORM thinks that 0 and null are the same and never updates the database in the situation above. I 100% need to cast this column as an integer value because we use strict checking in our codebase, otherwise Fuel defaults all values from this column as a string and my code breaks.
I took a look at the fuel/orm 1.8.2 branch and the 1.9 branch, and the non-strict code checks are still in both branches. Is there a specific reason why integer data types don't get strict checked on updates? I see this as a logical flaw in the code as 0 and null are not the same values.
As a follow up to this, I tried using 'tinyint' and 'bigint' data types for the column and it works as expected. My data is also returned correctly as integers and not strings. The issue seems to lie specifically with 'int' data types only.
If you want type checks and/or conversions, you need to use the Typing observer, and the correct type definitions in your model, optionally including default values.
The ORM itself doesn't do any typing whatsoever, so if conversions happen, it is done by the RDBMS engine itself.
I am using the typing observer and my data_type is set to 'int'. All of my models use the typing observer as it comes in super handy for auto-unserialization and timestamps to date objects. The problem is with Fuel's ORM not doing a strict check specifically for 'int' data types in the file mentioned above. As soon as I change it to a strict check, everything works as expected. It has nothing to do the database system itself.
Alternatively, if I use any of the type mappings for 'int' such as tinyint, smallint, mediumint, bigint, or even 'integer', the non-strict check gets bypassed and works as expected.
To bypass this issue, I simply set my data type to 'integer' and it works perfectly fine. This is something you should look at though as ORM doesn't handle all integer data types the same way.
I'm trying to understand why that is there, since the typing observer runs before that code, so all values should already have the correct type. The check also fails to deal with all other int formats (as you have noticed).
At the same time, is_changed() also checks "bool" that way, not really consistent.
I have to look at this in more detail, I need to full understand the consequences / impact of changing this.
Thanks for the reply, Harro. If you have any updates on this, please keep me posted. It seems like inconsistent logic to me why this would only affect 'int' data types and not the other integer data types.
Donate
