SQL Server: CPK 8.1.0 and activerecord-sqlserver-adapter 4.2.4 generate wrong SQL in some queries on tables without a DB primary key

[bunyan]

If I have a table with no primary key in DB defined like this:

create_table :no_id_models, id: false do |t|
      t.integer :pk1
      t.integer :pk2
    end

and a corresponding model with CPK

class NoIdModel < ActiveRecord::Base
  self.primary_keys = [:pk1, :pk2]
end

Calling

NoIdModel.limit(1).to_a

will result in the following error:

TinyTds::Error: Invalid column name 'pk1,pk2'.: EXEC sp_executesql N'SELECT  [no_id_models].* FROM [no_id_models]  ORDER BY [no_id_models].[pk1,pk2] ASC OFFSET 0 ROWS FETCH NEXT 1 ROWS ONLY'

I actually faced this with a belongs_to association but this is a simpler case.

In make_Fetch_Possible_And_Deterministic (/activerecord-sqlserver-adapter-4.2.4/lib/arel/visitors/sqlserver.rb:142), the adapter gets the composite key for a table and applies the sorting order to the whole "array" key which then results in wrong SQL. It's hardly a bug of the sqlserver-adapter so I'm posting it here.

To make things work for me I overrode the method in a Rails initializer to expand the query:

module SQLServerCPKOrderFix
  def make_Fetch_Possible_And_Deterministic o
    super
    o.orders.each_with_index do |node, i|
      rel = node.expr.relation
      expr_name = node.expr.name
      if expr_name.is_a?(CompositePrimaryKeys::CompositeKeys)
        o.orders[i] = expr_name.collect { |a| rel[a].send(node.direction) }
      end
    end
  end
end

Arel::Visitors::SQLServer.prepend(SQLServerCPKOrderFix)

Not sure how to incorporate this into the gem though.

A gist that reproduces the bug: https://gist.github.com/bunyan/1ea31f8b6ce8290c50b6

[cfis]

Look at the connection_adapters folder - that's where we put overrides to ActiveRecord connection adapters. So could the adapter be fixed instead?

[ckoch-ncsa]

In a project with composite_primary_keys (cpk), we experienced a similar error. It was noticeable when calling MODEL.reload. It would incorrectly form primary keys in order by, like [MODEL][pk1, pk2]. In order to make it work, we used code suggested by @bunyan from above, but we patched the gem method make_Fetch_Possible_And_Deterministic directly. (the difference is that it gets run only when o.orders are empty and this worked for us)

def make_Fetch_Possible_And_Deterministic o
  return if o.limit.nil? && o.offset.nil?
  t = table_From_Statement o
  pk = primary_Key_From_Table t
  return unless pk
  if o.orders.empty?
    # Prefer deterministic vs a simple `(SELECT NULL)` expr.
    o.orders = [pk.asc]
    o.orders.each_with_index do |node, i|
      rel = node.expr.relation
      expr_name = node.expr.name
      if expr_name.is_a?(CompositePrimaryKeys::CompositeKeys)
        o.orders[i] = expr_name.collect { |a| rel[a].send(node.direction) }
      end
    end
  end
end

We found that the first time the query was loaded the cpks were rendered in a way that worked for the query:
[MODEL][pk1], [MODEL][pk2]
but on the reload, the primary_keys were empty.

[vladhadzhiyski]

Has anyone been able to fix or come up with a more straight forward solution than monkey patching things?

I'm also running into this same issue:

ActiveRecord::StatementInvalid: TinyTds::Error: Invalid column name '"ShopID", "UserID"'.: EXEC sp_executesql N'SELECT  [NHIF_Patients].* FROM [NHIF_Patients] WHERE [NHIF_Patients].[PatEGN] = @0  ORDER BY [NHIF_Patients].["ShopID", "UserID"] ASC OFFSET 0 ROWS FETCH NEXT @1 ROWS ONLY', N'@0 varchar(10), @1 int', @0 = '5812236053', @1 = 1

[cfis]

For what its worth, the latest release has the best sql servers support CPK has ever had. But it does require Active Record 6.

For old CPK releases that are no longer supported, yes, just make a monkey patch. Feel free to submit a patch and it can be integrated in if tests pass.

[codeodor]

Can you show the full stack trace?