A column has been specified more than once in order by list

[themel]

Hi,

I have a Redmine installation failing on simple-looking find statements with the dread "duplicate ORDER BY" error.

ActiveRecord::StatementInvalid (TinyTds::Error: A column has been specified more than once in the order by list. Columns in the order by list must be unique.: SELECT * FROM (SELECT TOP 1 * FROM (SELECT TOP 1 * FROM [messages] WHERE ([messages].parent_id = 4)  ORDER BY messages.created_on ASC) AS tmp1 ORDER BY [created_on] DESC, [created_on] DESC) AS tmp2 ORDER BY [created_on] ASC, [created_on] ASC):
  D:/redmine/ruby187/lib/ruby/gems/1.8/gems/activerecord-sqlserver-adapter-2.3.24/lib/active_record/connection_adapters/sqlserver_adapter.rb:1005:in `raw_select'
  D:/redmine/ruby187/lib/ruby/gems/1.8/gems/activerecord-sqlserver-adapter-2.3.24/lib/active_record/connection_adapters/sqlserver_adapter.rb:966:in `select'
  app/controllers/messages_controller.rb:42:in `show'
  config/initializers/mongrel.rb:62:in `dispatch_cgi'

The call in question is

    @replies =  @topic.children.find(:all, :include => [:author, :attachments, {:board => :project}],
                                           :order => "#{Message.table_name}.created_on ASC",
                                           :limit => @reply_pages.items_per_page,
                                           :offset => @reply_pages.current.offset)

This is similar to closed issue #307, but you can see that it gets properly de-duplicated for the innermost select with the qualified name, but then breaks on the outer select statements. Is there anything that can be done about that?

ciao, Thomas

[GasCreature]

Exact same problem I am having initializing the seed data into the database when completing the installation of BrowserCMS (the latest, 3.5.5). I'm using TinyTDS 0.5.1 and ActiveSupport 3.2.13. The error message "A column has been specified more than once in the order by list. Columns in the order by list must be unique." is straight from SQL Server.

Assuming the problem is in lib/arel/visitors/sqlserver.rb, I will try to identify which location in the file is implicated.

[GasCreature]

Note - This analysis and patch has been cross posted to the BrowserCMS github page: https://github.com/browsermedia/browsercms/issues/589

As far as my situation with BrowserCMS goes, I believe neither ActiveRecord SQL Server Adapter nor Arel (A Relational Algebra) needs to be fixed. Duplicate order-by specifiers are probably all originating from incorrect application code. In the BrowserCMS case, I did a trace backwards and (monkey) patched it at the application level. To summarize this, backward towards the originator, ...

TDS statement

SELECT [cms_connectors].*
  FROM [cms_connectors] WHERE [cms_connectors].[page_id] = 2
   AND [cms_connectors].[page_version] = 2
 ORDER BY cms_connectors.container ASC, cms_connectors.position ASC, position ASC

Arel::SelectManager

@orders=
[#<Arel::Nodes::Ascending:0x74c2bd8 @expr="cms_connectors.container">,
 #<Arel::Nodes::Ascending:0x74c2b78 @expr="cms_connectors.position">,
 #<Arel::Nodes::Ascending:0x74c2ad0 @expr="position">]

Note that cms_connectors.position and position are referring to the same column but Arel doesn't know that.

BrowserCMS/app/models/cms/page.rb

def after_publish
  self.reload
  self.connectors.for_page_version(self.version).all(:order => "position").each do |c|
    if c.connectable_type.constantize.publishable? && con = c.connectable
      con.publish
    end
  end
end

The above can by monkey patched by using this line instead, to provide an explicit table name. Now Arel will be able to remove the duplicate order specifier because it is clearly a duplicate.

  self.connectors.for_page_version(self.version).all(:order => "#{Cms::Connector.table_name}.position").each do |c|

[metaskills]

So yes, SQL Server does not like duplicate order by clauses and the adapter does what it can to remove them. However, there is only so much we can do. All my current works allow us to pass the ActiveRecord test suite. If you find that you can make this better, please open up a tested pull request. Thanks!