[mssql] there are 3 hard problems in it: naming, caching and SQLServer AREL visitors !

Author: kares

lib/arel/visitors/sql_server.rb

@@ -1,21 +1,20 @@
 require 'arel/visitors/compat'
 
-loaded = nil
-if ArJdbc::AR42
-  require 'arel/visitors/sql_server/ng42.rb'
-  loaded = true
-end
-
 module Arel
   module Visitors
+    ToSql.class_eval do
+      alias_method :_visit_Arel_Nodes_SelectStatement, :visit_Arel_Nodes_SelectStatement
+    end
     # @note AREL set's up `Arel::Visitors::MSSQL` but its not usable as is ...
     # @private
     class SQLServer < const_defined?(:MSSQL) ? MSSQL : ToSql
 
+      private
+
       def visit_Arel_Nodes_SelectStatement(*args) # [o] AR <= 4.0 [o, a] on 4.1
         o, a = args.first, args.last
 
-        return super if ! o.limit && ! o.offset
+        return _visit_Arel_Nodes_SelectStatement(*args) if ! o.limit && ! o.offset
 
         unless o.orders.empty?
           select_order_by = do_visit_columns o.orders, a, 'ORDER BY '
@@ -27,14 +26,14 @@ def visit_Arel_Nodes_SelectStatement(*args) # [o] AR <= 4.0 [o, a] on 4.1
           core_order_by = select_order_by || determine_order_by(x, a)
           if select_count? x
             x.projections = [
-              core_order_by ? _row_num_literal(core_order_by) : Arel::Nodes::SqlLiteral.new("*")
+              Arel::Nodes::SqlLiteral.new(core_order_by ? over_row_num(core_order_by) : '*')
             ]
             select_count = true
           else
             # NOTE: this should really be added here and we should built the
             # wrapping SQL but than #replace_limit_offset! assumes it does that
             # ... MS-SQL adapter code seems to be 'hacked' by a lot of people
-            #x.projections << _row_num_literal(order_by)
+            #x.projections << Arel::Nodes::SqlLiteral.new over_row_num(order_by)
           end
           sql << do_visit_select_core(x, a)
         end
@@ -48,7 +47,7 @@ def visit_Arel_Nodes_SelectStatement(*args) # [o] AR <= 4.0 [o, a] on 4.1
         add_lock!(sql, :lock => o.lock && true)
 
         sql
-      end
+      end unless ArJdbc::AR42
 
       # @private
       MAX_LIMIT_VALUE = 9_223_372_036_854_775_807
@@ -67,19 +66,19 @@ def visit_Arel_Nodes_Lock o, a = nil
         #
         # we return nothing here and add the appropriate stuff with #add_lock!
         #do_visit o.expr, a
-      end
+      end unless ArJdbc::AR42
 
       def visit_Arel_Nodes_Top o, a = nil
         # `top` wouldn't really work here:
         #   User.select("distinct first_name").limit(10)
         # would generate "select top 10 distinct first_name from users",
         # which is invalid should be "select distinct top 10 first_name ..."
-        ''
+        a || ''
       end
 
       def visit_Arel_Nodes_Limit o, a = nil
         "TOP (#{do_visit o.expr, a})"
-      end
+      end unless ArJdbc::AR42
 
       def visit_Arel_Nodes_Ordering o, a = nil
         expr = do_visit o.expr, a
@@ -88,7 +87,7 @@ def visit_Arel_Nodes_Ordering o, a = nil
         else
           expr
         end
-      end
+      end unless ArJdbc::AR42
 
       def visit_Arel_Nodes_Bin o, a = nil
         expr = o.expr; sql = do_visit expr, a
@@ -98,7 +97,7 @@ def visit_Arel_Nodes_Bin o, a = nil
           sql << " #{::ArJdbc::MSSQL.cs_equality_operator} "
           sql
         end
-      end
+      end unless ArJdbc::AR42
 
       private
 
@@ -110,19 +109,17 @@ def select_count? x
         x.projections.length == 1 && Arel::Nodes::Count === x.projections.first
       end unless possibly_private_method_defined? :select_count?
 
-      unless possibly_private_method_defined? :determine_order_by
-        def determine_order_by x, a
-          unless x.groups.empty?
-            do_visit_columns x.groups, a, 'ORDER BY '
-          else
-            table_pk = find_left_table_pk(x, a)
-            table_pk == 'NULL' ? nil : "ORDER BY #{table_pk}"
-          end
+      def determine_order_by x, a
+        unless x.groups.empty?
+          do_visit_columns x.groups, a, 'ORDER BY '
+        else
+          table_pk = find_left_table_pk(x)
+          table_pk && "ORDER BY #{table_pk}"
         end
+      end
 
-        def find_left_table_pk o, a
-          primary_key_from_table table_from_select_core(o)
-        end
+      def find_left_table_pk o
+        primary_key_from_table table_from_select_core(o)
       end
 
       def do_visit_columns(colls, a, sql)
@@ -133,6 +130,18 @@ def do_visit_columns(colls, a, sql)
         sql
       end
 
+      def do_visit_columns(colls, a, sql)
+        prefix = sql
+        sql = Arel::Collectors::PlainString.new
+        sql << prefix if prefix
+
+        last = colls.size - 1
+        colls.each_with_index do |x, i|
+          visit(x, sql); sql << ', ' unless i == last
+        end
+        sql.value
+      end if ArJdbc::AR42
+
       def do_visit_columns(colls, a, sql)
         non_simple_order = /\sASC|\sDESC|\sCASE|\sCOLLATE|[\.,\[\(]/i # MIN(width)
 
@@ -151,9 +160,9 @@ def do_visit_columns(colls, a, sql)
         sql
       end if Arel::VERSION < '4.0.0'
 
-      def _row_num_literal order_by
-        Arel::Nodes::SqlLiteral.new("ROW_NUMBER() OVER (#{order_by}) as _row_num")
-      end
+      def over_row_num order_by
+        "ROW_NUMBER() OVER (#{order_by}) as _row_num"
+      end # unless possibly_private_method_defined? :row_num_literal
 
       def table_from_select_core core
         if Arel::Table === core.from
@@ -189,14 +198,14 @@ def primary_key_from_table t
           return pk if pk
         end
 
-        pk = @primary_keys[table_name = engine.table_name] ||= begin
+        pk = (@primary_keys ||= {}).fetch(table_name = engine.table_name) do
           pk_name = @connection.primary_key(table_name)
           # some tables might be without primary key
-          pk_name && t[pk_name]
+          @primary_keys[table_name] = pk_name && t[pk_name]
         end
         return pk if pk
 
-        column_name = engine.columns.first
+        column_name = engine.columns.first.try(:name)
         column_name && t[column_name]
       end
 
@@ -209,5 +218,8 @@ def primary_key_from_table t
     class SQLServer2000 < SQLServer
       include ArJdbc::MSSQL::LimitHelpers::SqlServer2000ReplaceLimitOffset
     end
+
+    load 'arel/visitors/sql_server/ng42.rb' if ArJdbc::AR42
+
   end
-end unless loaded
+end

lib/arel/visitors/sql_server/ng42.rb

@@ -1,6 +1,6 @@
 module Arel
   module Visitors
-    class SQLServer < Arel::Visitors::ToSql
+    class SQLServerNG < SQLServer # Arel::Visitors::ToSql
 
       OFFSET = " OFFSET "
       ROWS = " ROWS"
@@ -56,18 +56,55 @@ def visit_Arel_Nodes_Limit o, collector
       end
 
       def visit_Arel_Nodes_SelectStatement o, collector
-        @select_statement = o
         distinct_One_As_One_Is_So_Not_Fetch o
+
+        @select_statement = o
+
         if o.with
           collector = visit o.with, collector
           collector << SPACE
         end
-        collector = o.cores.inject(collector) { |c,x|
+
+        return _visit_Arel_Nodes_SelectStatement(o, collector) if ! o.limit && ! o.offset
+
+        # collector = o.cores.inject(collector) { |c,x|
+        #   visit_Arel_Nodes_SelectCore(x, c)
+        # }
+
+        unless o.orders.empty?
+          select_order_by = do_visit_columns o.orders, collector, 'ORDER BY '
+        end
+
+        select_count = false
+        collector = o.cores.inject(collector) do |c, x|
+          unless core_order_by = select_order_by
+            core_order_by = generate_order_by determine_order_by(o, x)
+          end
+
+          if select_count? x
+            x.projections = [ Arel::Nodes::SqlLiteral.new(over_row_num(core_order_by)) ]
+            select_count = true
+          else
+            # NOTE: this should really be added here and we should built the
+            # wrapping SQL but than #replace_limit_offset! assumes it does that
+            # ... MS-SQL adapter code seems to be 'hacked' by a lot of people
+            #x.projections << Arel::Nodes::SqlLiteral.new(over_row_num(select_order_by))
+          end if core_order_by
           visit_Arel_Nodes_SelectCore(x, c)
-        }
-        collector = visit_Orders_And_Let_Fetch_Happen o, collector
-        collector = visit_Make_Fetch_Happen o, collector
-        collector
+        end
+        # END collector = o.cores.inject(collector) { |c,x|
+
+        # collector = visit_Orders_And_Let_Fetch_Happen o, collector
+        # collector = visit_Make_Fetch_Happen o, collector
+        # collector # __method__ END
+
+        self.class.collector_proxy(collector) do |sql|
+          select_order_by ||= "ORDER BY #{@connection.determine_order_clause(sql)}"
+          replace_limit_offset!(sql, limit_for(o.limit), o.offset && o.offset.value.to_i, select_order_by)
+          sql = "SELECT COUNT(*) AS count_id FROM (#{sql}) AS subquery" if select_count
+          sql
+        end
+
       ensure
         @select_statement = nil
       end
@@ -125,22 +162,13 @@ def visit_Make_Fetch_Happen o, collector
 
       # SQLServer Helpers
 
-      def node_value(node)
-        return nil unless node
-        case node.expr
-        when NilClass then nil
-        when Numeric then node.expr
-        when Arel::Nodes::Unary then node.expr.expr
-        end
-      end
-
       def select_statement_lock?
-        @select_statement && @select_statement.lock
+        @select_statement && @select_statement.lock # AVOID INSTANCE var
       end
 
       def make_Fetch_Possible_And_Deterministic o
         return if o.limit.nil? && o.offset.nil?
-        if o.orders.empty?
+        if o.orders.empty? # ORDER BY mandatory with OFFSET FETCH clause
           t = table_From_Statement o
           pk = primary_Key_From_Table t
           return unless pk
@@ -184,8 +212,77 @@ def primary_Key_From_Table t
         column_name ? t[column_name] : nil
       end
 
+      def determine_order_by o, x
+        if o.orders.any?
+          o.orders
+        elsif x.groups.any?
+          x.groups
+        else
+          pk = find_left_table_pk(x)
+          pk ? [ pk ] : nil # []
+        end
+      end
+
+      def generate_order_by orders
+        do_visit_columns orders, nil, 'ORDER BY '
+      end
+
+      SQLString = ActiveRecord::ConnectionAdapters::AbstractAdapter::SQLString
+      # BindCollector = ActiveRecord::ConnectionAdapters::AbstractAdapter::BindCollector
+
+      def self.collector_proxy(collector, &block)
+        if collector.is_a?(SQLString)
+          return SQLStringProxy.new(collector, block)
+        end
+        BindCollectorProxy.new(collector, block)
+      end
+
+      class BindCollectorProxy < ActiveRecord::ConnectionAdapters::AbstractAdapter::BindCollector
+
+        def initialize(collector, block); @delegate = collector; @block = block end
+
+        def << str; @delegate << str; self end
+
+        def add_bind bind; @delegate.add_bind bind; self end
+
+        def value; @delegate.value; end
+
+        #def substitute_binds bvs; @delegate.substitute_binds(bvs); self end
+
+        def compile(bvs, conn)
+          _yield_str @delegate.compile(bvs, conn)
+        end
+
+        private
+
+        def method_missing(name, *args, &block); @delegate.send(name, args, &block) end
+
+        def _yield_str(str); @block ? @block.call(str) : str end
+
+      end
+
+      class SQLStringProxy < ActiveRecord::ConnectionAdapters::AbstractAdapter::SQLString
+
+        def initialize(collector, block); @delegate = collector; @block = block end
+
+        def << str; @delegate << str; self end
+
+        def add_bind bind; @delegate.add_bind bind; self end
+
+        def compile(bvs, conn)
+          _yield_str @delegate.compile(bvs, conn)
+        end
+
+        private
+
+        def method_missing(name, *args, &block); @delegate.send(name, args, &block) end
+
+        def _yield_str(str); @block ? @block.call(str) : str end
+
+      end
+
     end
   end
 end
 
-Arel::Visitors::VISITORS['mssql'] = Arel::Visitors::VISITORS['sqlserver'] = Arel::Visitors::SQLServer
+Arel::Visitors::VISITORS['mssql'] = Arel::Visitors::VISITORS['sqlserver'] = Arel::Visitors::SQLServerNG

lib/arjdbc/mssql/adapter.rb

@@ -108,6 +108,10 @@ def self.arel_visitor_type(config)
         ::Arel::Visitors::SQLServer2000 : ::Arel::Visitors::SQLServer
     end
 
+    def self.arel_visitor_type(config)
+      require 'arel/visitors/sql_server'; ::Arel::Visitors::SQLServerNG
+    end if AR42
+
     # @deprecated no longer used
     # @see ActiveRecord::ConnectionAdapters::JdbcAdapter#arel2_visitors
     def self.arel2_visitors(config)