Is remove always supposed to return true?

[OscarGodson]

I'm trying to figure out if remove was actually successful but it seems like I can pretty much pass anything to it and it'll say everything went well. Unless the DB is down, no errs are fired and the 2nd "data" param just always returns true. Or, maybe this is a normal thing for adapters to do?

[mde]

Yes, if there's no error, you'll get a true back. In the case of a SQL DELETE, you're removing according to some query criteria -- there's no way to know if any rows matched your search without doing multiple queries. We could do a count first, but two queries for every removal seems inefficient. Do you have thoughts on this?

[OscarGodson]

Ah, hmmm. As I said I'm still new to SQL. Feel like I'm learning it pretty fast now that I'm doing it all day, but I had just assumed it'd error out if nothing was found. In this case, I'm not sure what the majority of people do. You'd have way more experience in that realm :) so far everywhere I do a remove I first do a first call and if that returns nothing I return a 404 User Not Found to the client, but I don't know if thats standard or not. It works for our API tho.

[OscarGodson]

Oh, but, if that was a super common pattern (Model.first() and only if data Model.remove()) I may suggest a method that does both in one call. If I'm doing it wrong then I'd stick with the current setup

[mde]

I'm not a SQL expert, but with SQL a SELECT and a REMOVE with conditions work essentially the same. If you SELECT with conditions that don't match any entries in the DB, it returns nothing. If you DELETE with conditions that match nothing, it deletes nothing. Think of it like an if statement. If your if statement doesn't return true, the code in that block doesn't execute -- it doesn't mean there's something wrong. (The fact that it's all based on sets also makes bulk updates and deletes easy.)

As far as the standard for REST APIs, and removal of a non-existing item, I'm not sure -- REST is super-minimal, and leaves a lot of edge cases unspecified. 404 is probably fine in the case where you try to remove something that isn't there in the first place. Creating a special method to do that might be problematic however in that for the non-relational DBs, you already have to do an initial fetch based on your conditions to get the id(s) of the things to remove. This is basically the opposite side to your problem with SQL adapters.

HAVING SAID ALL THIS, after doing a little sleuthing around, it appears that Postgres (and perhaps other SQLs as well) are capable of returning a value for the affected rows after an UPDATE or DELETE. I can have a look and see how easy that would be to plug into our true/false return value -- and I should add that a pull request for this would be happily accepted. :D

[fourpixels]

Any news on this one? There is 'affectedRows' on most SQL libraries, so I would be happy to have this true result of the operation :)

[mde]

Sorry, I'm totally underwater with my new job -- I'd happily merge a PR that implements this.