cakephp: An empty string should be NULL

If someone is interested in a work-around would be:

<?php

namespace App\Model\Behavior;

use Cake\Event\Event;
use Cake\ORM\Association;
use Cake\ORM\Behavior;
use Cake\ORM\Table;
use Cake\Utility\Hash;

class PreserveNullBehavior extends Behavior
{
    /**
     * @param \Cake\Event\Event $event
     * @param \ArrayObject $data
     * @param \ArrayObject $options
     * @return void
     */
    public function beforeMarshal(Event $event, \ArrayObject $data, \ArrayObject $options)
    {
        $data = $this->_process($data, $this->_table);
    }

    /**
     * @param \ArrayObject|array $data
     * @param \Cake\ORM\Table $table
     * @return \ArrayObject|array
     */
    protected function _process($data, Table $table)
    {
        $associations = [];
        foreach ($table->associations() as $association) { /* @var $association \Cake\ORM\Association */
            $associations[$association->property()] = $association->name();
        }

        foreach ($data as $key => $value) {
            if (array_key_exists($key, $associations)) {
                $data[$key] = $this->_process($data[$key], $table->association($associations[$key])->target());
                continue;
            }
            $nullable = Hash::get((array)$table->schema()->column($key), 'null');
            if ($nullable !== true) {
                continue;
            }
            if (is_string($value) && $value === '') {
                $data[$key] = null;
            }
        }

        return $data;
    }
}

This is especially important with foreign keys (which are supposed to be nullable if optional), but also with string fields and alike. If an empty string is posted one would want to have null here saved in most cases. Otherwise you risk data inconsistency and false results on find().

You either make it not nullable and can use != ‘’ for checking (which I did in 2.x always), or you use null and IS/ISNOT null check. But having to allow both means you will end up with both in your app (from different ways of input) - and as a consequence you have always conditions like where field != '' AND field IS NOT NULL etc. if you ever forget to add both you miss out some records on finding, which introduces bugs. I have seen so much of that error prone code everywhere in apps…

👍 For a more core internal solution in the long run, but I am fine with any behavior opt-in for 3.3/3.4.

Ran into this. Agree that if DB schema has ‘allow null’ on a field and that entity sets that field is an empty string, it should be set to null in the DB, not and empty string.

My issue use case:

I have a form where the ‘email’ is optional, but if it’s provided then it must be unique (enforced via the isUnique buildRules).

When folks leave that field blank, is should be ‘NULL’ in the DB, but right now it gets inserted as an empty string. So when a user tries to add a new record with no email provided they are unable, as they get error with ‘record already exists’ for the email field…

OK, so I released the Nullable behavior in Shim 1.4.0 as proof of concept and along with some tests that show the usually expected behavior. With the 3.3.7 core release those tests will then all pass I assume. People interested in getting their DB clean can already use that right away.

We should probably now find a way to get those fixes into the core somehow.

@ionas Yes it is and the behavior shouldn’t be changed as @markstory already mentioned.

What about nullable INT and BOOLEAN fields? How would 0 and '' be handled there?

Found my own answer: char(36) casts “” to NULL, varchar(36) leaves “” as “”.

Actually, BoolType and empty string (nullable meaning of empty=>true coming from form) also behaves wrong

I agree, for BoolType empty string should be marshalled to null not false. We can change this in 4.0.

But for StringType auto converting empty string to null is not something that core should do. Those who need this behavior can easily handle it at app level with a behavior.

I also found a fixture issue here due to some testing with nullable columns: https://github.com/cakephp/cakephp/issues/9686

Thanks @steefaan Based on your input I made a test case here for all kind of types: https://github.com/dereuromark/cakephp-shim/commit/e8794cf2340502129dace06d96593c23136f3bf2#diff-e4ba1b1125865b7f834ad66a4059c6f0R35 Looks like this behavior approach indeed fixes the issue for both bool and string type.

I even tried to add back the empty string as possible nullish value - when explicitly set as default value in the schema: https://github.com/dereuromark/cakephp-shim/pull/9 But maybe this is too much? I wonder why is not really working out? The schema default is still null here for some reason.

@josegonzalez You are missing the whole point. allowEmpty false would only create the mess and inconsistency we just talked about the last 10+ comments 😃

It should be, but I wanted to make sure you didn’t want to change that behavior as well.

Isn’t a nullable int with empty '' coming in already stored as null? Same for bool with incoming ''? That would be the “intuitive” expectation. Because in that case someone put the form field to “empty=>true” which means optional, which means nullable.

0 and 1 for bool should be treated as always, same for 0 for int.