JJJ
commented
3 years ago Please excuse the git commit --ammend spasms I just pushed here. Embarrassing. Anyways...
I tested this PR inside of Sugar Calendar (obvs.) to make sure it's performing as intended with all null value columns. Not just integers, decimals, and date times, but any that have allow_null set to not false.
I believe this ends up being the best approach, because the validation callbacks can be overridden, which would skip over any bespoke null value handling. While allow_null is a property of the Column, it really is a construct of the Query class, specifically the validate_item() method. That method simply needs to always allow null if null is allowed. It does not need any additional validation.
The decision I've made in this PR (that I suspect is most open to discussion or future change) is what to do when trying to save a null value to a column when allow_null is false. I've opted to fail the entire database write, similar to if any other validation had failed.
(Berlin still fails silently everywhere when stuff like this happens, which is a separate issue IMO.)
We could, instead, simply unset the $item[ $key ], which would force MySQL to fallback to the defined default value of the column in the table. The reason I did not like that idea, is it seems more helpful to keep and trust everything as it's defined in PHP and Berlin (either user supplied, or via Query::default_item()), otherwise what's the point?
Thoughts & feedback encouraged. ❤️
ashleyfae
Up until now, the "Column::allow_null" flag was only partially implemented, working sometimes for "datetime" columns, but not for any others.
This commit ensures that, in the "Query::validate_item" method, null values for all item keys are allowed when "allow_null" is true for that column, and returns false early when attempting to save a null value to an item key when it is not allowed.
This commit also removes the special datetime, column type, null value handling that was added as part of #22, since it is now explicitly globally allowed (when allowed) in the global validator.
This commit also modifies "Column::sanitize_default" to only allow null defaults when "allow_null" is true, adds explicit support for strings and integers, and adds a @todo regarding other column types that may have their own unique default values (datetime, decimal, timestamp, and others come to mind).
Fixes #76.