Field defaults not used

[Arzaroth]

Hi,

First of all, thanks for this ORM. I needed something simple, non-intrusive, without annotations, and this delivers.

So this issue is fairly simple, in the documentation (http://phpdatamapper.com/docs/entities/), we can see the 'status' field is using "'default' => 0" in its definition. so one would assume it's building an Entity with such fields sets values according to this field if existing.

While this is the case when "'value' => 0" is set in the field definition, using 'default' doesn't seem to do much.

My understanding would be 'default' sets field value on insert if value is missing (i.e. NULL) and required, and 'value' sets it on entity build.

AFAIK, Entity::initFields() takes 'value' into account, and 'default' is used in Entity\Manager::fields() [206-207] to populate Entity\Manager::fieldDefaultValues, which is later used in Mapper::get() [518], but doesn't set any value on create.

So my question is fairly simple: is 'default' really relevant, since in almost any use case, 'value' does the trick? It seems there is, as of now, no point in using it. And if it's not, my guess is it shouldn't be referenced in the documentation.

[Arzaroth]

I'll correct myself, seems I was to quick on the draw. To make use of 'default', one has to use Mapper::get() without argument, to build a new Entity, which wasn't intuitive to me. Is it a design choice? I would say my question still stands.

[FlipEverything]

I think that's the intended behaviour. With the value parameter you can set the value of a new entity to a PHP return value like new \Datetime or $_SESSION['something'] or somefunction() etc. The default field is used with migrations to set the default value in the database and if you build a new entity it will be used to set the property of the new object to that, if it was not provided.

[Arzaroth]

if you build a new entity it will be used to set the property of the new object to that, if it was not provided

That's the problem: it doesn't.

Consider this stupid example:

namespace Entity;

class Post extends \Spot\Entity {
    [...]
    public static function fields() {
        return array(
            'id'    => array('type' => 'integer', 'primary' => true),
            'title' => array('type' => 'string', 'required' => true, 'value' => 'Title'),
            'text'  => array('type' => 'text', 'required' => true, 'default' => ''),
        );
    }
}

Then:

$entity = $mapper->build(array(
    'id' => 1
));

What should be the value for $entity->text? In spot2 current state, it's NULL. To apply default, you have to do:

$entity = $mapper->get(array(
    'id' => 1
));

$entity->_data["text"] is still NULL, and $entity->_dataModified["text"] is "", as expected (but $entity->_dataModified["id"] is then NULL in this example, because the record was not found)

[marcelloh]

In my mapper there is getEmptyRecord() An import part of it will create an emty entity based on defaults:

$dataRecord = $this->get(false);
$dataRecord = $dataRecord->toArray();

[FlipEverything]

I think that you have to save the entity to have the default values filled in. I haven't got any problems with it in the past but currently I can't test it for you.

[marcelloh]

You don't have to save it. But getting an entity with an id assumes it will get it from the database, so there is absolutely no need to have fields replaces by its default. The saved database row is alwas the thruth in case of a get(id). But if you want a fresh entity (new databaserecord), you get it by: mapper->get(false) and there is will use the default values as configured in the entity.

[tuupola]

The default option sets the value of DEFAULT when migrating table. The value of DEFAULT is used if INSERT or REPLACE query does not include a value for the given column. However since Spot uses full INSERT with NULL values when the DEFAULT is actually never used.

https://dev.mysql.com/doc/refman/5.7/en/data-type-defaults.html

Let's assume I have the following entity.

class Test extends \Spot\Entity
{
    protected static $table = "tests";

    public static function fields()
    {
        return [
            "id" => ["type" => "integer", "unsigned" => true, "primary" => true, "autoincrement" => true],

            "foo" => ["type" => "string", "length" => 32, "default" => "hello"],
            "bar" => ["type" => "string", "length" => 32, "default" => "hello", "notnull" => false],
            "pop" => ["type" => "string", "length" => 32, "value" => "hello"],
            "baz" => ["type" => "string", "length" => 32, "value" => "hello", "notnull" => false],

            "created_at"   => ["type" => "datetime", "value" => new \DateTime()],
            "updated_at"   => ["type" => "datetime", "value" => new \DateTime()]
        ];
    }
}

Migration query will look like this:

CREATE TABLE `tests` 
    `id` INT UNSIGNED AUTO_INCREMENT NOT NULL, 
    `foo` VARCHAR(32) DEFAULT 'hello', 
    `bar` VARCHAR(32) DEFAULT 'hello', 
    `pop` VARCHAR(32) DEFAULT NULL, 
    `baz` VARCHAR(32) DEFAULT NULL, 
    `created_at` DATETIME DEFAULT NULL, 
    `updated_at` DATETIME DEFAULT NULL, 
PRIMARY KEY(`id`) ;

Now lets create empty object and save it:

$test = new App\Test;
$spot->mapper("App\Test")->save($test);

Query will look like this:

INSERT INTO `tests` 
    (`foo`, `bar`, `pop`, `baz`, `created_at`, `updated_at`) 
VALUES 
    (NULL, NULL, 'hello', 'hello', '2016-12-22 07:17:10', '2016-12-22 07:17:10')

mysql> select * from tests;
+----+------+------+-------+-------+---------------------+---------------------+
| id | foo  | bar  | pop   | baz   | created_at          | updated_at          |
+----+------+------+-------+-------+---------------------+---------------------+
|  1 | NULL | NULL | hello | hello | 2016-12-22 07:17:10 | 2016-12-22 07:17:10 |
+----+------+------+-------+-------+---------------------+---------------------+
1 row in set (0.00 sec)

Now lets try a query without NULL values:

INSERT INTO `tests` 
    (`created_at`, `updated_at`) 
VALUES 
    ('2016-12-22 07:17:10', '2016-12-22 07:17:10')

mysql> select * from tests;
+----+-------+-------+-------+-------+---------------------+---------------------+
| id | foo   | bar   | pop   | baz   | created_at          | updated_at          |
+----+-------+-------+-------+-------+---------------------+---------------------+
|  1 | NULL  | NULL  | hello | hello | 2016-12-22 07:17:10 | 2016-12-22 07:17:10 |
|  2 | hello | hello | NULL  | NULL  | 2016-12-22 07:17:10 | 2016-12-22 07:17:10 |
+----+-------+-------+-------+-------+---------------------+---------------------+
2 rows in set (0.00 sec)

tl;dr "default" => "something" is not used by Spot, it is an SQL construct. Use "value" => "something" instead. The former might however be useful if you alter data also outside of Spot.

[Arzaroth]

Thank you for your reply. I assumed that much but a confirmation from an experienced user is nice. However, it felt counter-intuitive to me having to set "default" and "value" to the same value in most cases, if I were to use migration.

Considering your example, as I said earlier, a user not familiar with Spot would expect both foo and bar fields to be populated with default value (i.e. "hello") on Entity build. I would argue most of the time, it's what you want. So in this example, we are "forced" to set both "default" and "value" to "hello".

I'll close the issue, since the question "Does "default" should be used to populate Entity on build?" is out of scope.

[vlucas]

@Arzaroth I do understand your confusion. It does seem like the name default would imply the entity would be loaded with that value. I think this is an unfortunate case of the SQL conventions and naming already being there first, though I think it could be argued that the default in spot should do both things - make the SQL DEFAULT in addition to setting that value on the entity by default when you first load it. Thoughts?

[Arzaroth]

I'll re-open the issue so we can talk about this here then.

According to my limited experience with Spot, I'd say they are a few things to consider:

• Backward compatibility. It could break existing apps which relied on the fact default did not behave this way (setting NULL instead), for whatever reasons.
• Utility of value. When would you use value over default if the change was made? I can think of some marginal examples, but most of the time, value would be irrelevant.
• Priority. If both value and default are set, which is the correct one to use on Entity build? Logic says value, but the documentation will have to be very clear about that.
• Consistency. Right now, Mapper::get(false) returns a new Entity, with _dataModified populated using both value and default. If default was to be used in Entity build, should Mapper::get(false) still behave this way?

The implementation itself would be pretty straightforward, being for instance:

diff --git a/lib/Entity.php b/lib/Entity.php
index db3f3a9..78276ff 100644
--- a/lib/Entity.php
+++ b/lib/Entity.php
@@ -101,7 +101,7 @@ abstract class Entity implements EntityInterface, \JsonSerializable
         $fields = static::fields();
         foreach ($fields as $field => $opts) {
             if (!isset($this->_data[$field])) {
-                $this->_data[$field] = isset($opts['value']) ? $opts['value'] : null;
+                $this->_data[$field] = isset($opts['value']) ? $opts['value'] : (isset($opts['default']) ? $opts['default'] : null);
             }
         }

The way I see it, rather than changing the way default and value work, being clear in the documentation regarding their purposes. Now that I dwelved a bit more in Spot, current implementation makes sense.

[FlipEverything]

Sorry, my bad! :) I thought Spot sets the default values for the entity, but I forgot that my own application does it for me. (And in some cases I set the "value" to the "default" value redundantly. Which is pointless I think.) I remember now that I had this kind of rage/confusion when started using Spot. I don't mind the modification you made. I think it's reasonable and it should be enough to set one of the parameters (default), value is not needed in the most cases. But If you specify the value by hand it should be the primary choice.

[charlesswanson]

I just started using Spot (it looks like exactly what I want!). However, this really confuses me...

I have a table with columns: id, title, color. I want to be able to run

$mapper->create([
    'title' => 'Something'
]);

and have an entry with a new id and a default color of 'red'. However, Spot is inserting null for color. I've tried using 'value' => 'red' and 'default' => 'red' for the color field with no luck.

Is this expected?

[charlesswanson]

Sorry, user error...setting value to red does what I want.

[nebulousGirl]

I can think of one case for which the separation of "default" and "value" is important and it is when setting a default that changes frequently like dates: "value" => new \DateTime. Using "default" in this case would result in unwanted migrations for each field which have this kind of default each time a new migration is run.

This doesn't mean we can't populate the entity with "default", but that "value" has a use and should not be removed.

[tuupola]

I think the current behaviour is correct. This is more of a documentation problem.

[FlipEverything]

I think that @Arzaroth's modification is better than the current implementation.

[Arzaroth]

Resolved with #229 being merged.