Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Rollback where.like and where.not_like

The real win with these chain methods is where.not, that takes care of
different scenarios in a graceful way, for instance when the given value
is nil.

    where("author.id != ?", author_to_ignore.id)
    where.not("author.id", author_to_ignore.id)

Both where.like and where.not_like compared to the SQL versions doesn't
seem to give us that much:

    Post.where("title LIKE 'ruby on%'")
    Post.where.like(title: 'ruby on%'")
    Post.where("title NOT LIKE 'ruby on%'")
    Post.where.not_like(title: 'ruby on%'")

Thus Rails is adding where.not, but not where.like/not_like and others.
  • Loading branch information...
commit 8d02afeaee8993bd0fde69687fdd9bf30921e805 1 parent 7b50dc5
@carlosantoniodasilva carlosantoniodasilva authored
View
5 activerecord/CHANGELOG.md
@@ -1,11 +1,10 @@
## Rails 4.0.0 (unreleased) ##
-* Allow `Relation#where` with no arguments to be chained with new query methods
- `not`, `like`, and `not_like`.
+* Allow `Relation#where` with no arguments to be chained with new `not` query method.
Example:
- Developer.where.not(name: 'Aaron').where.like(name: 'Takoyaki%')
+ Developer.where.not(name: 'Aaron')
*Akira Matsuda*
View
26 activerecord/lib/active_record/relation/query_methods.rb
@@ -47,32 +47,6 @@ def not(opts, *rest)
@scope.where_values += where_value
@scope
end
-
- # Returns a new relation expressing WHERE + LIKE condition
- # according to the conditions provided as a hash in the arguments.
- #
- # Book.where.like(title: "Rails%")
- # # SELECT * FROM books WHERE title LIKE 'Rails%'
- def like(opts, *rest)
- where_value = @scope.send(:build_where, opts, rest).map do |rel|
- Arel::Nodes::Matches.new(rel.left, rel.right)
- end
- @scope.where_values += where_value
- @scope
- end
-
- # Returns a new relation expressing WHERE + NOT LIKE condition
- # according to the conditions provided as a hash in the arguments.
- #
- # Conference.where.not_like(name: "%Kaigi")
- # # SELECT * FROM conferences WHERE name NOT LIKE '%Kaigi'
- def not_like(opts, *rest)
- where_value = @scope.send(:build_where, opts, rest).map do |rel|
- Arel::Nodes::DoesNotMatch.new(rel.left, rel.right)
- end
- @scope.where_values += where_value
- @scope
- end
end
Relation::MULTI_VALUE_METHODS.each do |name|
View
19 activerecord/test/cases/relation/where_chain_test.rb
@@ -62,29 +62,14 @@ def test_not_eq_with_array_parameter
assert_equal([expected], relation.where_values)
end
- def test_like
- expected = Arel::Nodes::Matches.new(Post.arel_table[:title], 'a%')
- relation = Post.where.like(title: 'a%')
- assert_equal([expected], relation.where_values)
- end
-
- def test_not_like
- expected = Arel::Nodes::DoesNotMatch.new(Post.arel_table[:title], 'a%')
- relation = Post.where.not_like(title: 'a%')
- assert_equal([expected], relation.where_values)
- end
-
def test_chaining_multiple
- relation = Post.where.like(title: 'ruby on %').where.not(title: 'ruby on rails').where.not_like(title: '% ales')
+ relation = Post.where.not(author_id: [1, 2]).where.not(title: 'ruby on rails')
- expected = Arel::Nodes::Matches.new(Post.arel_table[:title], 'ruby on %')
+ expected = Arel::Nodes::NotIn.new(Post.arel_table[:author_id], [1, 2])
assert_equal(expected, relation.where_values[0])
expected = Arel::Nodes::NotEqual.new(Post.arel_table[:title], 'ruby on rails')
assert_equal(expected, relation.where_values[1])
-
- expected = Arel::Nodes::DoesNotMatch.new(Post.arel_table[:title], '% ales')
- assert_equal(expected, relation.where_values[2])
end
end
end

68 comments on commit 8d02afe

@kipcole9

Perhaps the value would be that it could be used as a database independent form of LIKE. Since mysql is case insensitive and postgres is case sensitive for LIKE, where.like could be used for a database independent, case insensitive version of LIKE.

@carlosantoniodasilva

@kipcole9 you're right, and actually for postgresql this was generating ilike to work as case insensitive. But unfortunately that was not a strong argument to keep the feature, and another counter argument is that moving from one database to another is something that happens rarely. Thanks!

@olivierlacan

A simple argument for the like/not_like syntax is that it follows the expectations set by the rest of the ActiveRecord query syntax. It's always been odd to me to suddenly manipulate a part of the SQL string for such a trivial (and common) thing as a LIKE operator.

As a beginner I've often tried to do Article.where(title.like: "Rails 3 is%") because it made logical sense that ActiveRecord should cover such behavior.

Both where.like and where.not_like compared to the SQL versions doesn't
seem to give us that much:

Post.where("title LIKE 'ruby on%'")
Post.where.like(title: 'ruby on%'")
Post.where("title NOT LIKE 'ruby on%'")
Post.where.not_like(title: 'ruby on%'")

The most common use I've seen is Post.where("title ILIKE ?", "ruby on%") which is already much messier.

This is a shame, I think the arguments against like/not_like are weak.

@caike

:-1:

I agree that moving from one database to another is something that rarely happens for specific applications, but I think the biggest benefit of having like and not_like is setting a convention for Rails code across applications that might run on different databases.

Rails applications don't move to different databases as much as Rails developers move to different Rails applications - perhaps each one running on a different database ;) This feature would avoid having to always be aware of which database you are working with at the moment.

The alternative to writing database specific SQL, like Post.where("title LIKE 'ruby on%'"), is to dive into Arel and do something like:

posts = Post.arel_table
Post.where(posts[:title].match('ruby on%'))

Which is still better than writing SQL, but IMHO a lot more verbose and error prone.

@fxn
Owner

While Arel status has become a gray area in practice perhaps over time, from Rails point of view it belongs to the internals of Active Record.

If Active Record does not provide this feature, the alternative within the public interface is to write SQL, not to use Arel.

@julian7

@carlosantoniodasilva you're cheating :grinning:! Here's your code snippet:

Post.where("title LIKE 'ruby on%'")
Post.where.like(title: 'ruby on%'")
Post.where("title NOT LIKE 'ruby on%'")
Post.where.not_like(title: 'ruby on%'")

A syntax highlighter comes handy sometimes. First off, #like and #not_like lines could be one char shorter, and secondly, they are not comparable with your string jungle on the odd rows (haha, odd rows...). Here's what we can compare:

Post.where('title LIKE ?', 'ruby on%')
Post.where.like(title: 'ruby on%')
Post.where('title NOT LIKE ?', 'ruby on%')
Post.where.not_like(title: 'ruby on%')

Which one is worse? The one with raw sql string innit, or the one with a hash?

I do feel some weakness in #not_like though (much less than in sql snippets).

@carlosantoniodasilva

@julian7 :smile: bad c&p examples, sorry. Thanks for the examples.

@tomeric

Although changing databases is not a common occurrence for applications, supporting multiple database backends is a common thing in libraries that use ActiveRecord. It would be easier for library developers if they can use ActiveRecord instead of raw SQL.

@bogdan

Postgresql for example supports ilike operator that I am always using instead of like.
Mysql for example can not use indexes for like argument started with wildcard. That makes it slow for 100k+ records tables.

This makes unification though activerecord not so usable.

@roman-zaharenkov

:+1: to like not_like chain methods. I hate SQL inside models. Why do you rollback so clear feature? What is your arguments?

@DouweM

:-1: for this rollback, :+1: for chaining #like. As @tomeric points out, @carlosantoniodasilva's "moving from one database to another is something that happens rarely" argument falls apart when in the context of gems or open source Rails apps that use ActiveRecord. Aren't we using ActiveRecord to abstract away database implementation peculiarities, among other things? And as @julian7 shows, #like actually is cleaner and shorter than the current raw SQL solution.

@DevL

:-1: on rollback. A database agnostic way of handling case-insensitive LIKE (MySQL: LIKE, Postgres: ILIKE) makes a lot of sense and eliminates a reason to dive into SQL.

@eljojo

Actually, @olivierlacan's idea sounds pretty nice:

Article.where(title.like: "Rails 3 is%")
@ReneB

:-1: on rollback. I really liked the fact that the leaky abstraction was going to be patched here. Of course the abstraction provided by ActiveRecord will be leaky in some sense; since it is non-trivial, it will always be. But this change allowed patching of one of the major leaks.

Also, it -like some of the other abstractions- provides a protection against direct injection of variables into SQL. Of course, you should never, etcetera etcetera, but specifically 'LIKE'-queries are notoriously "hard to build"* - and when building SQL-strings, it's easy to slip up somewhere.

edit: (* = No, they're not really hard to build, but they do behave differently and require funky stuff with percent signs, and quotes and all).

@ReneB

@eljojo: looking at your syntax, title needs to be a local variable or method supporting the 'like' method. This is slightly hard to guarantee in all places where the Article / ActiveRecord interface is used without running the risk of shadowing other variables. An alternative could be:

Article.where(:title.like => "Rails 3 is %")

but that requires using rocket syntax as well as monkey- err, freedom-patching Symbol with a 'like' method.

@eljojo

@ReneB sorry, it should have been a symbol all the time, I just copied/pasted, but I agree.

And for the patching of the Symbol class, we could use Ruby 2.0 refinements only on the controllers (or something similar), but yeah, that wouldn't be available everywhere.

@ReneB

I don't think this is the right time to build on Ruby 2.0 refinements in light of the recent discussion on the feature in ruby-core. Also, it would indeed not be available everywhere in our codebases, introducing more confusion ('why does this work in my controller, but not in my helper / extracted module / gem?')

@eljojo

@ReneB yeah, I think you're right, but the original where.like implementation was really good :+1:

@kurko

The arguments for rolling back are so weak that I can't think of anything besides someone above ordering this to be rolled back.

@Hermanverschooten

I agree with most of you that removing this is a bad move.
I recently migrated a Rails 2.3 + MySQL app to 3.2.8 + Postgresql, and this was one of the things that frustrated me.
I ended up using ARel-syntax, because I don't want to write the SQL queries. But I certainly did not like it.

Rails is opinionated, well in my opinion, add it back in.

@dhh
Owner

Here are the design considerations that I had when we decided to roll this back:

1) It's an odd mismatch of SQL and Ruby with the % sign for designating the order.
2) It opens the door for wanting to expand this line of DSL further (including undesireable ventures into greater_or_equal_than).
3) It's being a fairly uncommon query not in need of optimization.
4) not_like is nasty (and "dislike" wont cut it either).

Feel free to extract this into a plugin and mix it into your own flavor of Rails, though.

@RKushnir

@dhh To №4: how about "unlike"?

I have the same argument about the potential for evergrowing DSL, though. There's already things like https://github.com/ernie/squeel, not mentioning the ARel itself.

@rosenfeld

Actually I don't think anyone would write .where("something like 'some value%'"). It is more likely to be something like .where("something like ?", params[:name]) to protect against SQL injection. For this particular case I see how ".like" would be useful. ".ilike" should be useful as well (I don't like the idea of using ilike for .like queries).

But I agree there is no need for not_like. Just write .not.like. But for those cases I'd prefer an approach like Sequel's one:

Book.where(author_id: 3).except(publisher_id: 1)...

@ReneB

Feel free to extract this into a plugin and mix it into your own flavor of Rails, though.

I like the suggestion, and I like the code as it stands, so I might try my hand at it.

@metaskills

counter argument is that moving from one database to another is something that happens rarely

It is not about moving from one database to another, it is about writing code that works on multiple databases. Think of ActiveRecord gem authors, etc.

@dhh
Owner
@dhh
Owner
@roman-zaharenkov

When you announce some new features included to rails release it's something more than yet another gem :) So everyone will know what where.like(:name => '%J%') means.

Squeel is a pretty gem but sometimes you need to do something fast and you no need all powerfull of squeel. ActiveRecord expressions like where('name like %?%', 'J') look realy ugly. I don't see a logic, why did you decide to implement 'not' chain method but not 'like'.

:+1: to unlike method!

@dhh
Owner
@ReneB

As promised, I'm working on it now. I'm trying to find my way around the Rails test suite and how to extract a simple plugin with tests, but I'll get there. For now, I'm simply naming it active_record-like but it's begging for a more clever name. I'm open to suggestions.

@carlosantoniodasilva

@ReneB you could call it where_chain ;)

@ReneB

The where-chain is still being provided by ActiveRecord itself, right? This plugin would only need to supply where.like and where.unlike / where.not_like / where.not.like or whatever, I think.

@ReneB

Aha, looking at the history of the ActiveRecord file wherein this was defined and the branch it was merged from, I finally see what you did there... oh well, let's see if I can still make myself useful.

@ReneB

Actually, one more question for all those concerned. I'm trying to figure out the convoluted history of this patch, and attempting to find my way through the maze of pull requests and fixes to find out who I should credit for this. I think the people involved are @amatsuda and @claudiob, right? Anyone else I should credit?

@avit

Not expecting any reversal of this rollback, but I'd just like to respond to @dhh's objections here since there's an interesting related discussion about the query API in the core mailing list. That thread focuses on Squeel, but I think the demand for merging Squeel points to some common issues.

1) An odd mismatch of SQL and Ruby with the % sign for designating the order.

That mismatch is, to some extent, a necessity. Much of ActiveRecord's query API has to work with it one way or another, but it's actually become more like SQL lately instead of fighting it, for example the chainable .where() instead of :conditions hashes.

The "%" is accepted as the wildcard character in SQL, just like Dir['*.rb'] method knows to do shell globbing.

Ultimately, the smaller and thus more database-agnostic we can write our SQL string fragments, the better. Some operators (!= vs <>), for example, are not totally standardized. IMO those parts (analogous to choosing "LIKE" or "ILIKE") should be for ActiveRecord to decide based on the database connection and maybe a :case_sensitive flag or a separate ilike method to do the right thing.

2) It opens the door for wanting to expand this line of DSL further (including undesireable ventures into greater_or_equal_than).

The fact that "WHERE a = 1" is a first-class citizen while "WHERE a > 1" isn't does make me a little bit sad actually. Same goes for "LIKE". AR does some really nice things like converting where(status: 1..3) ranges & arrays to use "IN()". I wouldn't mind to see the DSL address some of its shortcomings for building comparisons without concatenating strings or digging deep with Arel. (A wild idea appears: .where(:updated_at, :>, Date.today)) Just saying, people are looking for some improvements in this area, case in point, Squeel.

3) It's being a fairly uncommon query not in need of optimization.

That's kind of subjective. "LIKE" is less common than "=" certainly, but does it mean it's "fairly uncommon" enough to dismiss completely? I think the fact that we have to pick and choose which common SQL comparisons can or can't be written for us by ActiveRecord points to some underlying weakness.

4) not_like is nasty (and "dislike" wont cut it either).

Yes, but at least "not_like" mirrors the SQL which makes it predictable. However if there's a more general way of doing negation by chaining .not then that's definitely the better approach.

:+1: for keeping .not to handle the "OR NULL" situation.

@ReneB

(I was going to post this last night, but my ISP decided otherwise.)

However if there's a more general way of doing negation by chaining .not then that's definitely the better approach.

I considered that for activerecord-like but decided against it for now, since the problem when reading code like

Post.where.not.like(title: "%rails 4%").having(...)

might be readability: what is, and what is not, modified by the 'not' specifier?

Also: the plugin is mostly done for now. It can be found at activerecord-like. Let me know what you people think. Bug reports and comments welcome.

@jroes

@ReneB yeah, some sort of context would be necessary for #not, perhaps with a block:

Post.where.not { like(title: "%rails 4%") }.having(...)
@claudiob
Collaborator

@ReneB funnily enough, I gave a talk yesterday at L.A. Ruby's monthly meetup about the "history of this feature", how things got in and out until now. A Youtube video will be updated soon, but meanwhile feel free to check my slides. My presenter notes are in the text transcripts below the slides. http://www.slideshare.net/recent/rails4

@ReneB

@claudiob Awesome! The explanation of the history of this feature is really well done and provides remarkable insight into the process of rejecting and accepting commits for a project that is used as much as Rails. I like the fact how the like and not_like-methods have become 'itch-scratchers' for some people. I have to say I still don't like SQL in my ActiveRecord abstraction layer, but it might not be for Rails core. The suggestion to extract a plugin, then, makes perfect sense.

I hope it might scratch the same itch for other people, and if not, it has been an interesting (and I have to say, humbling, looking at the different patches that make up the history of this one) learning experience.

@mrbrdo

The block syntax is ok (I don't like .not.like). But for me it makes much more sense to include this information inside the where method.
Something like this would be quite acceptable I think

Post.where(title: Arel.like("rails %"))
Post.where(title: Arel.not_like("rails %"))

The Arel is a bit noisy, but at least you don't clutter Object and it's pretty concise I think, reads nicely. And you don't go away from the current syntax. Somehow this would work for me too

Post.where(title_like: "rails %")

Just my 5 cents.

@Fryguy

I'm mixed on this one. I always felt like AR needed methods for building LIKE statements, as well as for comparison operators. I think it's weird that I should have to build SQL at all in situations like these. On the other hand, after the implementation came out, I thought of two problems.

First was @dhh's first point on having that '%' in there. It still felt wrong, and I think part of my problem is in using the term, "like". I would expect methods like "starts_with", "ends_with", and "includes" (or "contains") to be more Ruby-ish ways to express "x%", "%x", and "%x%", respectively. Then there's always the chance that a new DB comes along, and it a different character than % as the wildcard.

Second was that reading the code was weird. The equal operator reads as infix... Post.where(:title => "x") reads as "Posts where the title is x". This operator reads postfix... Post.where.like(:title => "x%") reads as "Posts where like the title is/matches x%" or something. When you mix the infix and postfix together it just gets weird.

I like @mrbrdo's approach above. This looks pretty nice to me:

Post.where(title: Arel.starts_with("rails "))
@kenn

Probably it's only me but it feels like an impedance mismatch either way to have a scope method on a particular column.

Be it where.like or where.not, it acts on a particular column, not on the relation. So we're introducing sub-scope state in the scope chain here. That is the meat of the debate IMO.

In other words,

Post.where.like(title: 'ruby on%')

Post.where.not("author.id", author_to_ignore.id)

could have been

Post.where(:title.like => 'ruby on%')

Post.where("author.id".not => author_to_ignore.id)

and I prefer the predicates directly chained on the subject, not on the relation, the https://github.com/ernie/squeel way.

@ReneB

@kenn: A library in such widespread use as Rails should not just bluntly go around monkeypatching Symbol and String for very context-specific stuff. The way you describe it would require Strings with very specific formatting requirements, as well as Symbols, to respond_to #like - but what on earth is this #like method doing in this class when I'm not actively working with ActiveRecord? Refinements aren't here yet, you know...

@dhh
Owner
@kenn

@ReneB I should have been more specific about my conclusion - I'm not saying Rails should have the latter syntax, I agree with you that it would suck without Ruby 2.x refinements. My point is that both where.like and where.not share the common problem introducing context-sensitive sub-scopes on columns rather than relations. This contradicts our mental model. I think both are premature optimizations - should be a separate gem.

@kenn

@dhh right, that's my point. Make where.not a gem, not official Rails feature. It seems like a premature design decision to me.

@dhh
Owner
@kenn

@dhh If we take where.not as the only exception (not messing with others likegreater_than_or_equal_to), I agree that it has enough benefit worth the inclusion.

One question, though - if where.not must always be called in this order, why not where_not? Is there any practical reason that not is a separate method called upon where? If they are inseparable, making it inseparable seems sensible to me.

@mrbrdo

I don't think #not is good either, because all the other methods are other kind of keywords in SQL. E.g. where, having, group, select... But not is used inside of where (or inside some other keyword), so it doesn't make sense to me to make not equal to where. I know :field_name_not => (or _is_not if you like) looks pretty lame but I'm beginning to think it's actually the best solution (bar refinements). Or where_not. I really don't like where.not, it doesn't make sense.

@amatsuda
Collaborator

For those who are suggesting other new syntaxes, here you can see what I already did. https://github.com/amatsuda/everywhere

I actually suggested these syntaxes in the past, but they were already rejected by the core team, except the where.not chain syntax which was finally merged.

@dhh
Owner
@mrbrdo

@dhh It's not clear to me, are you actually going to merge where.not?

(by the way, is it possible currently with Arel to generate IS NOT NULL without explicitly saying it?)

@dhh
Owner
@mrbrdo

-_- ok well like you said above then. Personally I don't like it, but there is no really good solution that I can see. where.not seems hard to explain and confusing to a newbie.
I know you have to make hard decisions so all props to you on that.

@dhh
Owner
@Hermanverschooten
@reggieb

If one of the issues is the use of %, would one solution be to substitute * which is a more widely used wild card character. So you get:

things.like(name: '*smith')

to generate

name LIKE '%smith'  
@mrbrdo

Imo, the best readable solution is with similar syntax that is used in squeel

things.where(:name.end_with? => 'smith')

But that will not be a nice solution until we have Ruby2 refinements.
When you're doing a DB abstraction layer it's not necessary to actually keep the like keyword, you can make an analogy in ruby (like ends_with, or end_with)

@schuetzm

One thing I haven't seen brought up in the discussion, but which could be important, is that in the string variant you have to explicitly include the table name. This could go wrong a soon as you do this on a complex relation, e.g. one involving a self join, but possibly others too. (And you can never know which kinds of relations you get passed when you're writing a module.)

In this case it's a real advantage if you let Arel decide how it should refer to the column you're referencing.

@schuetzm

My argument of course applies to any situation where you need to use SQL directly, which also include <, <=, >, >= besides LIKE. Thus I concur with point 2 made by @avit above.

@avit

in the string variant you have to explicitly include the table name

Yes, I thought this was obvious, but just to show the actual differences omitted from the original commit message:

Post.where("`#{Post.table_name}`.`title` LIKE 'ruby on%'")
Post.where.like(title: 'ruby on%'")
Post.where("`#{Post.table_name}`.`title` NOT LIKE 'ruby on%'")
Post.where.not_like(title: 'ruby on%'")

Table name is important if you might be merging this relation into a query with any joins/includes. Instead of the above, I prefer: Post.where(arel_table[:title].like 'ruby on%'")

Has anyone considered something like alias_method :t, :arel_table? That gets us pretty close to minimal syntax overhead; then you get all these from Arel. If only some of those were aliased as :>, :<, :>=, :<= etc.

@mrbrdo

Well then you could go even further and have something like

Post.where(t.title.like 'ruby on%'")
@avit

@mrbrdo, I like it! Then t can provide a small wrapper for Arel attributes with predicate method helpers such as >=.

And now for something completely different just for giggles: use backticks to avoid monkey-patching String! (It seems crazy but I wonder if this is what working with refinements will feel like.)

@PikachuEXE

I just tried Post.where.not_like with order and update_all and the values in like/not_like changed to 0 (which cause error)
So I am using plain SQL now...
Example 1:

ScrapedListing.where.not_like(import_error_code: 'abc').latest_created_first.update_all(import_error_code: 'abc')
  SQL (2.9ms)  UPDATE "scraped_listings" SET "import_error_code" = 'abc' WHERE "scraped_listings"."id" IN (SELECT "scraped_listings"."id" FROM "scraped_listings" WHERE ("scraped_listings"."import_error_code" NOT ILIKE 0) ORDER BY "scraped_listings"."created_at" DESC)
@ReneB

Oh wow. @PikachuEXE, could you create a failing test case for activerecord-like ? I don't immediately see what's going on here, but this might be a problem in the like and not_like code. I've attempted to reproduce this error, but the test cases pass for me.

It seems to me, by the way, that you might want to use import_error_code: '%abc%' instead, since this syntax works for exact matches and you might as well not use where.not in this case, since this just tests for inequality.

@PikachuEXE

Can't reproduce on sqlite
Trying to make test run on postgresql

Please sign in to comment.
Something went wrong with that request. Please try again.