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

boolean values update issues #107

Closed
tristanbsn opened this issue Feb 16, 2022 · 10 comments
Closed

boolean values update issues #107

tristanbsn opened this issue Feb 16, 2022 · 10 comments
Labels
question Further information is requested

Comments

@tristanbsn
Copy link

With Postgresql database, update are not handled correctly in some cases.

Working :

  • old value : false => new value : true
  • old value : true => new value : NULL
  • old value : NULL => new value : true
  • old value : NULL => new value : false

Not working :

  • old value : true => new value : false
  • old value : false => new value : NULL

After some search I found that there is an issue in the update(array $row, array $current) function of src/Loaders/InsertUpdate.php file.
This section use loose comparison between old values array and new values array :
if ($row == array_intersect_key($current, $row)) { return; }

The $current array return boolean from database as a PHP Boolean while the $row array take string('true', 't', 'false', 'f') or NULL value to define booleans.

This prevent database update when it should be.

@ecourtial ecourtial added the bug Something isn't working label Feb 16, 2022
@ecourtial
Copy link
Collaborator

Thank you @tristanbsn we are going to look at it.

@ecourtial
Copy link
Collaborator

@tristanbsn could you clarify this point please, I am not sure to understand 🙇

... while the $row array take string('true', 't', 'false', 'f') or NULL value to define booleans.

@tristanbsn
Copy link
Author

@ecourtial In my case I use a transformer with a callback that take a date as source and transform it as string 'false' if date < current_date, 'true' otherwise :
return (date($row->get($column)) < date('Y-m-d')) ? 'false' : 'true';

This value get stored in a Boolean field in my PostgreSQL database.

@tristanbsn
Copy link
Author

To clarify the issue is mainly caused by the loose comparision in the update(array $row, array $current) function that cause unwanted behaviors because of thoses cases :

  • true == "false" return true
  • true == "f" return true
  • false == "false" return false
  • false == "f" return false
  • false == NULL return true

In my case if the value is true in my PostegreSQL database, it cannot be updated to false because this line prevent the update to be triggered :
if ($row == array_intersect_key($current, $row)) { return; }

@ecourtial
Copy link
Collaborator

Thank you. I will look at it today.

@tristanbsn
Copy link
Author

Thank you, I made a quick patch if it can help you :

    /**
     * Execute the update statement.
     */
    protected function update(array $row, array $current): void
    {
        if (false === $this->doUpdates) {
            return;
        }

        if (null === $this->update) {
            $this->prepareUpdate($row);
        }

        // CURRENT VERSION
        /*
            if ($row == array_intersect_key($current, $row)) {
                return;
            }
        */

        // CORRECTED VERSION
        $correctedRow = $row;

        foreach ($correctedRow as $key => $value) {
            if (isset($current[$key]) && gettype($current[$key]) === "boolean" && gettype($value) === "string" && in_array($value, ["true", "t", "false", "f"])) {
                $correctedRow[$key] = in_array($value, ["true", "t"]) ? true : false;
            }
        }

        if ($correctedRow == array_intersect_key($current, $row)) {
            return;
        }
        // END CORRECTED VERSION

        if ($this->timestamps) {
            $row['updated_at'] = $this->time;
        }

        $this->update->execute($row);
    }

@ecourtial
Copy link
Collaborator

After a quick look, I am still not sure to understand 😅

This section use loose comparison between old values array and new values array :
if ($row == array_intersect_key($current, $row)) { return; }

So here the comparison is between keys of the array.
Hence despite the fact that we should have === instead of == at line 188, we are going to fix that on rigour sake, it won't change anything because as we can see in the documentation of PHP:

The two keys from the key => value pairs are considered equal only if (string) $key1 === (string) $key2 . In other words a strict type check is executed so the string representation must be the same.

So if we would like to really strictly compare keys, we could use a callback. But I am not sure why we should do that, because the keys of the array are supposed to represent columns names, not values.

Let me know if I am missing something, I would be very glad to help you.

@ecourtial ecourtial added question Further information is requested and removed bug Something isn't working labels Feb 18, 2022
@tristanbsn
Copy link
Author

if ($row == array_intersect_key($current, $row)) { return; } compare not only the keys but also the values of the array.
In my situation,

$row = ["date_naissance" => "1962-06-18", "actif" => "false"]
$current = ["date_naissance" => "1962-06-18", "actif" => true]

the comparison is true because "false" == true is true and this cause the update to not be triggered.

@ecourtial
Copy link
Collaborator

My bad indeed. Easily reproduced... after your explanations.
So I created a PR, could you check it is ok for you, and then I will merge and tag a new version.

@tristanbsn
Copy link
Author

It works for me, thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants