Persistence\Array_ not allowing zero id, throwing error

[mkrecek234]

1. Create a model from an array with $m = new Model(new Array_($t)); where $t= [ ['name' => 'January', 'sales' => 20000, 'purchases' => 10000], ['name' => 'February', 'sales' => 23000, 'purchases' => 12000]]

2. Iterate through model with foreach ($model as $row) { }

Atk4\Data throws an error on iteration as it does not accept id to be zero in \Atk4\Data\Field Line 311.

$value = (int) $value; if ($this->required && empty($value)) { throw new ValidationException([$this->name => 'Must not be a zero'], $this->getOwner()); }

@mvorisek Probably due to stricter type validation, but that way Persistence\Array_ is broken.

[mvorisek]

Correct me if I am wrong, but default's model ID field is required to be a non-zero by default.

I expect the same issue with SQL persistence.

To verify if this is a bug with array persistence, set required/mandator for ID field to false.

[mkrecek234]

If id is set not to be required, sure it does not throw an Exception @mvorisek. Array Persistence however automatically assigns the id to match the array index, so starts with 0. To me, 0 is not null, and also SQL databases make a difference: NULL (not filled out) whereas 0 (filled out). Validation for integer thus should check for is_null($value) and not empty($value).

[mvorisek]

Then is seems absolutely unrelated to me with array persistence.

See #899 and related issues.

[mkrecek234]

It is not unrelated. If we decide not to treat a 0 id as a filled value and treat it differently than null, then Persistence\Array_ is not working, as it assigns logically the first record to have an id=0. @mvorisek

[mvorisek]

Please check the PR above and the difference between notNull/notEmpty. ID is designed to be nonEmpty by default in https://github.com/atk4/data/blob/3.0.0/src/Model.php#L420

You can always modify it by $m->getField($m->id_field)->required = false.

Is this a solution for you?

[mkrecek234]

@mvorisek This does not make sense, sure the id of a Persistence\Array_ model HAS to be required. And the way Array_ is setup is that it automatically assigns the id to match the array index, so starts with 0. It is implemented in Array_.
Required means "not null" and not "not zero" - every professional database makes a difference in that regard: Zero indeed fulfills a mandatory field. NULL not.

So if we all come to the conclusion that 0 is treated as null, then Array_ needs a change. Right now, it cannot work as it is assigning id=0 to the first element of the data array.

By the way, if you create a model with Array_ based on this array [['id' => 1, 'name' => 'Entry 0'], ['id' => 2, ...]] the Validation would then through the error `Uncaught Atk4\Data\Exception: Row constains ID column, but it does not match the row ID

[mvorisek]

Michael, see the PR above that intends exactly the correct handling of notNull/notEmpty. For historical reasons, it is named mandatory/required, maybe we can name it later better, but of course, atk4 must handle this well. And it will.

So if we all come to the conclusion that 0 is treated as null
It is not what I was, I just pointed out, that some bad developers do so.

Discord discussion:

Now, your issue is about ID to be allowed to be empty. My question is and was, why you can not allow it to be empty manually for your usecase.

You can easily init your array to start with one by specifying the first key to be 1: [1 => [], [], []], all other keys will be +1 of the previous key, this is how php arrays are compiled.

[['id' => 1, 'name' => 'Entry 0'], ['id' => 2, ...]] can be therefore written as [1 => ['id' => 1, 'name' => 'Entry 0'], ['id' => 2, ...]].

Is your issue about ignoring array keys if id column is specified? If so, this can be improved.

[mkrecek234]

We should not overcomplicate things for the developer:
If you want to create a model from an array (with array persistence), then it should be EASY:

Just do $m = new Model(new Array_([['name' => 'Peter'], ['age' => 12]])); - why should any developer bother about how to assign which id?

Persistence\Array_ could in your case just map array[0] to [1 => ['name' => 'Peter'], ['age' => 12]]] - BUT today Array_ persistence maps it to id=0.

So let's keep it simple, and not build too many traps for developers.

[mkrecek234]

Once again, my issue is NOT to allow id to be empty. I would prefer a required id to be allowed to be 0 (zero), not just null. But if you don't agree, then see my comment above, that Array_ should map automatically element array[0] to id=1.

[mvorisek]

Michael, I would like to help you, but I think you are not fully aware of the possible negative consequences I mentioned.

Also, please accept also 0 is empty in integer world. In php, in SQL, almost everywhere.

I do not want to spend more time in this discussion. If you think you have a clever idea I am missing, please submit ts as a PR.

[mkrecek234]

I think you don't get my point: If we agree to treat 0 identical to null for integer, THEN Persistence\Array_ should map $data[0] to id=1 - today it maps it to id=0 which causes an error.

[mvorisek]

Is the only issue you have the seeding of the array persistence and IFF the keys are a list (0, 1, ...n-1) AND id col is unspecified, THEN insert it with ID 1...n?

[mkrecek234]

Yes, now you got me ;-)

…

Am 19.10.2021 um 15:32 schrieb Michael Voříšek ***@***.***>: Internal representation is unimportant. Is the only issue you have the seeding the array persistence and IFF the keys as list (0, 1, ...n-1) AND id col is unspecified, THEN insert it with ID 1...n? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#907 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG2X37ST72XY6WBNSX726ETUHVXOXANCNFSM5GI3H6GQ>.