Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable support for __set. #1622

Merged
merged 2 commits into from
Jan 10, 2019
Merged

Conversation

nowackipawel
Copy link
Contributor

I wrote it and works like a charm. Slightly more efficient, because I some of array_key_exists don't exist anymore ;).
But... I noticed an issue when I am trying to save this entity. Property which is set to null is excluded from INSERT what should cause problem if default column value is not set to null but different value - in that case we cannot save null during insert.

In my case should be:

INSERT INTO `t_table_search` (`dcs_identifier`, `dcs_conditions`, `dcs_dsc_id`) VALUES ('e77d630b7be1484cb4b7b4f8dbc15c45', '{\"v-school\":[\"1\"],\"v-center\":[\"44\"]}', NULL) // dcs_dsc_id = null;

it is:

INSERT INTO `t_table_search` (`dcs_identifier`, `dcs_conditions`) VALUES ('e77d630b7be1484cb4b7b4f8dbc15c45', '{\"v-school\":[\"1\"],\"v-center\":[\"44\"]}')// no null for dcs_dsc_id

I think it is because of entity value of property in _original is the same as actual value of property (i.e.: dcs_dsc_id == null)

object(App\Entities\entityname)#40 (7) {
  ["dcs_identifier":protected]=>
  string(32) "e77d630b7be1484cb4b7b4f8dbc15c45"
  ["dcs_conditions":protected]=>
  string(52) "{"v-school":["1"],"v-center":["44"]}"
  ["dcs_dsc_id":protected]=>
  NULL
  ["dcs_updated_at":protected]=>
  NULL
  ["_options":protected]=>
  array(3) {
    ["casts"]=>
    array(3) {
      ["dcs_identifier"]=>
      string(6) "string"
      ["dcs_conditions"]=>
      string(10) "json-array"
      ["dcs_dsc_id"]=>
      string(11) "?json-array"
    }
    ["dates"]=>
    array(2) {
      [0]=>
      NULL
      [1]=>
      string(14) "dcs_updated_at"
    }
    ["datamap"]=>
    array(0) {
    }
  }
  ["_original":protected]=>
  array(4) {
    ["dcs_identifier"]=>
    NULL
    ["dcs_conditions"]=>
    NULL
    ["dcs_dsc_id"]=>
    NULL
    ["dcs_updated_at"]=>
    NULL
  }
  ["_cast":"CodeIgniter\Entity":private]=>
  bool(true)

I saw PR month or two ago which was changing entity saving but IMHO it should be turned ON for update or replace but should NOT work for INSERT.

I wrote it and works like a charm. Slightly more efficient, because I  some of  array_key_exists don't exist anymore ;). 
But... I noticed an issue when I am trying to save this entity. Property which is set to null is excluded from INSERT what should cause problem if default column value is not set to null but different value - in that case we cannot save null during insert.

In my case should be:
```
INSERT INTO `t_table_search` (`dcs_identifier`, `dcs_conditions`, `dcs_dsc_id`) VALUES ('e77d630b7be1484cb4b7b4f8dbc15c45', '{\"v-school\":[\"1\"],\"v-center\":[\"44\"]}', NULL) // dcs_dsc_id = null;
```
it is:
```
INSERT INTO `t_table_search` (`dcs_identifier`, `dcs_conditions`) VALUES ('e77d630b7be1484cb4b7b4f8dbc15c45', '{\"v-school\":[\"1\"],\"v-center\":[\"44\"]}')// no null for dcs_dsc_id
```

 I think it is because of entity value of property in _original is the same as actual value of property (i.e.: dcs_dsc_id == null)


```object(App\Entities\entityname)codeigniter4#40 (7) {
  ["dcs_identifier":protected]=>
  string(32) "e77d630b7be1484cb4b7b4f8dbc15c45"
  ["dcs_conditions":protected]=>
  string(52) "{"driving-school":["1"],"examination-center":["44"]}"
  ["dcs_dsc_id":protected]=>
  NULL
  ["dcs_updated_at":protected]=>
  NULL
  ["_options":protected]=>
  array(3) {
    ["casts"]=>
    array(3) {
      ["dcs_identifier"]=>
      string(6) "string"
      ["dcs_conditions"]=>
      string(10) "json-array"
      ["dcs_dsc_id"]=>
      string(11) "?json-array"
    }
    ["dates"]=>
    array(2) {
      [0]=>
      NULL
      [1]=>
      string(14) "dcs_updated_at"
    }
    ["datamap"]=>
    array(0) {
    }
  }
  ["_original":protected]=>
  array(4) {
    ["dcs_identifier"]=>
    NULL
    ["dcs_conditions"]=>
    NULL
    ["dcs_dsc_id"]=>
    NULL
    ["dcs_updated_at"]=>
    NULL
  }
  ["_cast":"CodeIgniter\Entity":private]=>
  bool(true)```


I saw PR month or two ago which was changing entity saving but IMHO it should be turned ON for update or replace but should NOT work for INSERT.
system/Entity.php Outdated Show resolved Hide resolved
@nowackipawel
Copy link
Contributor Author

@lonnieezell : is it ready to merge?

@lonnieezell lonnieezell merged commit 3003609 into codeigniter4:develop Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants