This project is archived and is in readonly mode.

#1765 ✓wontfix
Student

AR::Base#all(:order => '') => invalid SQL

Reported by Student | January 15th, 2009 @ 07:49 PM | in 2.x


class Venue < ActiveRecord::Base
end

Venue.find(:all, :order => '')
ActiveRecord::StatementInvalid: Mysql::Error: You have an error in your SQL syntax; check the manual that corresponds to
your MySQL server version for the right syntax to use near '' at line 1: SELECT * FROM `venues`      ORDER BY 
        from /usr/local/lib/ruby/gems/1.8/gems/activerecord-2.1.2/lib/active_record/connection_adapters/
abstract_adapter.rb:147:in `log'

:conditions and :joins appear to work, at least in isolation.

Comments and changes to this ticket

  • Yaroslav Markin

    Yaroslav Markin January 17th, 2009 @ 08:40 AM

    • Tag changed from 2.1.2, activerecord, mysql to 2.1.2, activerecord, mysql, patch, tiny
    • Assigned user set to “Pratik”

    Patch attached

  • Pratik

    Pratik January 18th, 2009 @ 03:46 AM

    • State changed from “new” to “wontfix”

    You shouldn't supply empty string :) It's not really designed to work with conditions and :joins, but it just does. It could easily break.

  • Student

    Student January 18th, 2009 @ 03:33 PM

    Is this my payment for minimizing my example? :/

    I found this using generated code. With this functionality, I can do the following:

    @@@ruby cond_string = big computation order_string = bigger computation join_string = biggest computation

    Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)

    
    
    Without, I'm stuck with:
    
    @@@ruby
    cond_string = big computation
    order_string = bigger computation
    join_string = biggest computation
    
    options = {}
    options[:conditions] = cond_string unless cond_string.empty?
    options[:order] = order_string unless order_string.empty?
    options[:joins] = join_string unless join_string.empty?
    
    Venue.all(options)
    

    Unless I "really shouldn't" pass in an empty options string, either.

  • Student

    Student January 18th, 2009 @ 03:36 PM

    That didn't format as advertised, and I don't seem to be able to edit my comment.

    Is this my payment for minimizing my example? :/

    I found this using generated code. With this functionality, I can do the following:

    cond_string = big computation order_string = bigger computation join_string = biggest computation

    Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)

    Without, I'm stuck with:

    cond_string = big computation order_string = bigger computation join_string = biggest computation

    options = {} options[:conditions] = cond_string unless cond_string.empty? options[:order] = order_string unless order_string.empty? options[:joins] = join_string unless join_string.empty?

    Venue.all(options)

    Unless I "really shouldn't" pass in an empty options string, either.

  • Student

    Student January 18th, 2009 @ 03:41 PM

    Ick. Spaces matter.

    Is this my payment for minimizing my example? :/

    
    cond_string = big computation 
    order_string = bigger computation 
    join_string = biggest computation
    
    Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)
    

    Without, I'm stuck with:

    
    cond_string = big computation 
    order_string = bigger computation 
    join_string = biggest computation
    
    options = {} options[:conditions] = cond_string unless cond_string.empty? 
    options[:order] = order_string unless order_string.empty? 
    options[:joins] = join_string unless join_string.empty?
    
    Venue.all(options)
    

    Unless I "really shouldn't" pass in an empty options string, either.

  • Pratik

    Pratik January 18th, 2009 @ 06:06 PM

    
    order_string = bigger_computation
    order_string = nil if order_string.blank? 
    cond_string = big computation 
    join_string = biggest computation
    
    Venue.all(:joins => join_string, :conditions => cond_string, :order => order_string)
    
  • Student

    Student January 18th, 2009 @ 08:47 PM

    There is currently no documentation on what happens if an option is passed which is nil or empty. I discovered this behavior (:joins, and :conditions work with blank parameters, :order does not) when using generated fragments. In my case, these generated fragments were themselves compounds, built with a final Array#join, thus always strings.

    I would like a clean, smooth, supported interface. Your solution would appear to be none of these.

  • Yaroslav Markin

    Yaroslav Markin January 18th, 2009 @ 08:53 PM

    @lifo while this ticket is far from important and I fully agree that @nil@-ifying parameter is just what Student needs, I think there is still sense in applying: right now there is a code snippet that checks for @nil@, we can just change it to @.present?@, that solves the problem just right.

  • Xavier Noria

    Xavier Noria January 18th, 2009 @ 09:12 PM

    I am -1 here. The API shouldn't be designed to eat everything. In particular I don't really see the documentation stating that you can pass blank?s to :order (if it was really supported it should be documented).

    In my opinion :order has either a valid value or nothing. An empty string is an invalid value.

    Another possibility would be to clean the hash with a custom #compact method for hashes that removes entries with blank? values.

  • Student

    Student January 19th, 2009 @ 03:33 AM

    I agree that this is not an error as in "the documentation says X will happen and it does not". But I believe that the observed behavior is not optimal, in that the caller is responsible (in some cases) to normalize data when there is only one reasonable way to interpret the data.

    Certainly, the caller can, as I currently must, ensure that no option is set with blank parameters. But if the purpose of a framework is to reduce the work of the using programmer, this would seem to be a strange way to proceed.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages