Rails Ruby on Rails

Comments and changes to this ticket

• 

  Neeraj Singh July 13th, 2010 @ 12:57 AM

  • Importance changed from “” to “Low”

  As far as I know order by should never be quoted.

  SELECT cars.* FROM "cars" ORDER BY id 
  SELECT cars.* FROM "cars" ORDER BY 'id'
  

  Above two sql statements are not same. The second one is meaningless. It will sort on literal string 'id'.

• You flagged this item as spam.
  

  Matt Moriarity July 13th, 2010 @ 01:25 AM

  I mean double-quoted, which will treat it as a column name and not a keyword:

  SELECT cars.* FROM "cars" ORDER BY "id"

  would be the same as the original, but if "id" was "order", this one would work while the original wouldn't.

• 

  Neeraj Singh July 13th, 2010 @ 02:09 AM

  I did not know that in the case you mentioned double quote would work but not single quote.

  Anyway here is snippet from mysql adapter.

        def quote(value, column = nil)
          if value.kind_of?(String) && column && column.type == :binary && column.class.respond_to?(:string_to_binary)
            s = column.class.string_to_binary(value).unpack("H*")[0]
            "x'#{s}'"
          elsif value.kind_of?(BigDecimal)
            value.to_s("F")
          else
            super
          end
        end
  

  As you can see even if code is put into quote the data then in mysql it would be quoted using single quote unless code is changed to handle this special case.

• You flagged this item as spam.
  

  Miles Egan July 13th, 2010 @ 03:18 AM

  I think this is actually an Arel thing. If you look at the code for the ordering clause in arel. Specifically, if you look at
  lib/arel/engines/sql/formatters.rb on line 48:

  class OrderClause < PassThrough
    def ordering(ordering)
      "#{quote_table_name(name_for(ordering.attribute.original_relation))}.#{quote_column_name(ordering.attribute.name)} #{ordering.\
  direction_sql}"
    end
  end
  

  You can see that the OrderClause class doesn't override the value() method of its parent and just passes the order value through unchanged. If you add a value clause like this:

  class OrderClause < PassThrough
    def value(value)
      quote_column_name(value)
    end
  
    def ordering(ordering)
    "#{quote_table_name(name_for(ordering.attribute.original_relation))}.#{quote_column_name(ordering.attribute.name)} #{ordering.\
  direction_sql}"
    end
  end
  

  Then queries like Lists.order(:order) start to work but this breaks queries like Lists.order("order asc"). The only way I can see to make this work is to make the OrderClause class smart enough to parse the various expressions you can put in an order clause, but maybe this is easy in Arel. I don't understand the internals of Arel well enough to say.

• 

  Neeraj Singh July 13th, 2010 @ 03:28 AM

  In query_methods.rb in ActiveRecord you can do following

  def order(*args)
    clone.tap { |r| r.order_values += args if args.present? }
    #clone.tap { |r| r.order_values += args.collect {|e| connection.quote(e)} if args.present? }
  end
  

  commented out code is the change I did.

  I ran this test with mysql and lots of tests failed. That is because mysql put single quotes around values. I had no idea that single quote vs double quotes matter. I guess quote can be patched to accept parameter then this might work.

  Question:

  Do all databases(sqlite3, mysql, postgresql, oracle) support having double quotes around order by values?

• You flagged this item as spam.
  

  Miles Egan July 13th, 2010 @ 10:18 PM

  I just noticed that this page:
  http://wiki.rubyonrails.org/rails/pages/reservedwords

  Which advises against using any sql reserved words for column names. So, I wouldn't say that people should expect this to work. I guess it's worth an easy fix if there is one but maybe not worth tearing up much of the ActiveRelation code if that's what it takes.

• 

  Neeraj Singh August 5th, 2010 @ 04:07 AM

  • State changed from “new” to “wontfix”

  It will take a lot of work to fix it and even then it would be flaky. It is best to not to use reserved words.

• You flagged this item as spam.
  

  Matt Moriarity August 5th, 2010 @ 04:32 AM

  I guess I'm just surprised this is such a difficult fix, since the double quotes are already used earlier in the SQL statement in the SELECT block. There's no reason not to apply them, even if the word is not reserved.

  But then again, I'm not intimately familiar with how ActiveRelation and ActiveRecord work internally, so I'll have to take you at your word that this is not as easy as it seems to me. I changed my column name anyway, so I'm no longer hindered by this.

  Thanks for the response.

• You flagged this item as spam.
  

  Miles Egan August 5th, 2010 @ 04:45 AM

  The thing that makes it difficult is that the order by parameter can be any sql expression, so you'd have to try to parse it, not just throw quotes around it.

• You flagged this item as spam.
  

  Matt Moriarity August 5th, 2010 @ 08:47 PM

  Perhaps it could be quoted automatically only if the user provided a symbol as the parameter.

  For instance, List.order(:order) would generate '... ORDER BY "order"' but List.order('order') would generate '... ORDER BY order'

• You flagged this item as spam.
  

  Miles Egan August 5th, 2010 @ 08:53 PM

  I'm inclined to agree with Neeraj here. For the sake of simplicity and maintainability it's better to ask people to avoid used reserved SQL words as field names.

• You flagged this item as spam.
  

  Matt Moriarity August 5th, 2010 @ 08:54 PM

  Alright, it was worth a shot. Still loving Rails 3 anyway!

• 

  Neeraj Singh August 5th, 2010 @ 08:56 PM

  If rails start supporting usage of reserved words then hundreds of test cases need to be added to ensure that rails through and through works with reserved words. Just not worth the effort in my opinion.