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

save() fails silently if you have select()ed some fields, but not the id #203

Closed
ghost opened this issue May 20, 2014 · 6 comments
Closed

Comments

@ghost
Copy link

ghost commented May 20, 2014

Given the following table

CREATE TABLE `users` (
  `id` int(11) unsigned NOT NULL AUTO_INCREMENT,
  `email` varchar(254) NOT NULL DEFAULT '',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

With the data

INSERT INTO `users` (`id`, `email`)
VALUES ('1', 'foo@example.com');

And the following code

$user = ORM::for_table('users')
    ->where('id', 1)
    ->select('email')
    ->find_one()
;

$user->email = 'bar@example.com';

$success = $user->save();

$success is true, but the database will not have reflected this change (email will still be foo@example.com).

get_last_query() indicates that the following query is being executed

UPDATE `users` SET `email` = 'bar@example.com' WHERE `id` = ''

Notice that it could not find the id value to place in the query.

A work around is to remove the select() call altogether, or at least call select('id'), but I still think this is a bug.

@treffynnon
Copy link
Collaborator

This should probably throw an exception. Can you or anyone prepare a pull request for this issue? I don't have the time at the moment unfortunately.

@ghost
Copy link
Author

ghost commented Jun 21, 2014

@treffynnon should this be closed before #205 is merged or another solution committed?

@treffynnon
Copy link
Collaborator

It will be closed after #205 is merged. At the moment I don't see any issue
with merging your pull request - just gotta find the time to do it!

@ghost
Copy link
Author

ghost commented Jun 21, 2014

That's what I'm saying... you've closed it already.

@treffynnon
Copy link
Collaborator

Ah, I have? I got disturbed mid-way through working on it the other day so
probably didn't finish what I was I doing. Not to worry the pull request
acts as the open ticket for now anyway.

@ghost
Copy link
Author

ghost commented Jun 21, 2014

No worries.

treffynnon added a commit that referenced this issue Jun 21, 2014
Restrict null primary keys on update/delete, resolves #203
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

1 participant