Using hash conditions with explicit table name (#9733)

Ahoy!

Have you guys ever needed to do this?:

Post.find(:all, ;include => :comments, :conditions => {'comments.created_at ’ => 14.days.ago…7.days.ago})

Well, I certainly have. And I finally got around to patching sanitize_sql_hash_for_conditions() so rather than doing this:

“SELECT … WHERE posts.comments.created_at BETWEEN …”

it provides us with:

“SELECT … WHERE comments.created_at BETWEEN …”

And you can have the whole package for the low, low price of a +1.

http://dev.rubyonrails.org/ticket/9733

::Jack Danger

+1

I was thinking about this recently, but was toying around with a different, possibly more flexible syntax...

:conditions => {:comments => {:created_at ' => 14.days.ago..7.days.ago}}

Combined with normal conditions...

:conditions => {:field => value, :comments => {:created_at ' => 14.days.ago..7.days.ago}}

You could nest as deep as you like in the same way :include works.

Any issues with this approach instead?

Andrew

This suggestion seems to be a little bit more “ruby way” and tends to organize the code a bit more…

That’s way better! Care to write a patch?

::Jack Danger

Almost done. :slight_smile:

I mentioned nesting to any depth before (just off the top of my head), but I'm not sure if that's useful or not...

Doing this...

Post.find(:all, ;include => {:comments => :users}, :conditions => {:comments => {:users => {:name => 'Joe'}, :created_at => 14.days.ago..7.days.ago}})

would be the same thing as...

Post.find(:all, ;include => {:comments => :users}, :conditions => {:comments => {:created_at => 14.days.ago..7.days.ago}, :users => {:name => 'Joe'}})

resulting in...

comments.created_at BETWEEN ... AND users.name = 'Joe'

My patch does work with conditions to any depth, but is it useful? I suppose it could make things look cleaner as to better match the style of the :include and there might be other styles or uses that I can't think of at the moment.

My patch right now is recursive which allows the extra depth, but if that's not seen as useful, I'll rewrite it with an iterative approach and not allow the extra depth.

Andrew

I mentioned nesting to any depth before (just off the top of my head), but I’m not sure if that’s useful or not…

I don’t think we’ll need the extra depth, just :conditions => {:table => {:column => ‘value’}} should suffice. I can’t think of any complication beyond that that would be useful.

It should be as simple as checking whether the given value is a hash - no recursion should be necessary.

::Jack Danger

I uploaded my patch to http://dev.rubyonrails.org/ticket/9733, but kept the recursive approach. My brain found it to be easiest and cleanest to implement it that way. :slight_smile:

If you want to redo it with an iterative approach, please feel free.

Regards, Andrew

This ticket (http://dev.rubyonrails.org/ticket/9733) has a couple of +1's for the previous string style, but I guess we need some new +1's for my updated hash style.

+1 away if you see no issues.

Andrew