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

ActiveRow problem with update of primary key #57

Closed
mdjimy opened this issue Mar 12, 2015 · 2 comments
Closed

ActiveRow problem with update of primary key #57

mdjimy opened this issue Mar 12, 2015 · 2 comments

Comments

@mdjimy
Copy link

mdjimy commented Mar 12, 2015

Hi,
recently, I've been exploring source code of some database entities and I've noticed, that in method update of object ActiveRow is one small bug. Well to be precise, it's not a bug, but I think It's not a valid behavior of method too.

The code of method looks like this:

public function update($data) {
    $selection = $this->table->createSelectionInstance()->wherePrimary($this->getPrimary());
    if ($selection->update($data)) {
        selection->select('*');
        if (($row = $selection->fetch()) === FALSE) {
            throw new Nette\InvalidStateException('Database refetch failed; row does not exist!');
        }
        $this->data = $row->data;
        return TRUE;
    else {
        return FALSE;
    }
}

How you can see, if I’ll be updating primary key of ActiveRow, the primary key will be in database updated (if I don’t have defined any references on it), but I’ll end up with an exception “Database fetch failed;…” because in the query is used an old value of the primary key.

I think that a better behavior this method will be the one from below:

  1. To disable update of a primary column(s), and throw an exception before update in DB
  2. Or to fetch updated row for from DB on updated key (my option)

If you don't have anything against this, I'd like to resolve this issue & create pull request. (Because this term at school I have a subject called Open-Source Programming in which I have to join some running open source project & resolve some issues.) :D

Thanks for your attention,
and I'll look forward to your reply.

@dg
Copy link
Member

dg commented Mar 13, 2015

Better is to fetch updated row.

@mdjimy
Copy link
Author

mdjimy commented Mar 13, 2015

I thought so... :)

Could you please assign me to this issue @dg? I'd like to solve it (as part of my seminar work at school), if you don't have anything against it. ;)

And thank you for your response.

@dg dg closed this as completed in e057c95 Mar 26, 2015
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

No branches or pull requests

2 participants