fix one more time order_hacks method for Oracle

correctly split order string by commas ignoring commas which separate function arguments

2 files changed, 29 insertions, 8 deletions

diff --git a/lib/arel/visitors/oracle.rb b/lib/arel/visitors/oracle.rb
index bb6ebd4ab8..00572213e6 100644
--- a/lib/arel/visitors/oracle.rb
+++ b/lib/arel/visitors/oracle.rb
@@ -70,19 +70,14 @@ module Arel
             /DISTINCT.*FIRST_VALUE/ === projection
           end
         end
-        # FIXME: previous version with join and split broke ORDER BY clause
+        # Previous version with join and split broke ORDER BY clause
         # if it contained functions with several arguments (separated by ',').
-        # Currently splitting is done only if there is no function calls
         #
         # orders   = o.orders.map { |x| visit x }.join(', ').split(',')
         orders   = o.orders.map do |x|
           string = visit x
-          # if there is function call
-          if string.include?('(')
-            string
-          # if no function call then comma splits several ORDER BY columns
-          elsif string.include?(',')
-            string.split(',')
+          if string.include?(',')
+            split_order_string(string)
           else
             string
           end
@@ -94,6 +89,20 @@ module Arel
         end
         o
       end
+
+      # Split string by commas but count opening and closing brackets
+      # and ignore commas inside brackets.
+      def split_order_string(string)
+        array = []
+        i = 0
+        string.split(',').each do |part|
+          array[i] ||= ""
+          array[i] << part
+          i += 1 if array[i].count('(') == array[i].count(')')
+        end
+        array
+      end
+
     end
   end
 end
diff --git a/test/visitors/test_oracle.rb b/test/visitors/test_oracle.rb
index d2a73ea8c5..2c9bba15d8 100644
--- a/test/visitors/test_oracle.rb
+++ b/test/visitors/test_oracle.rb
@@ -43,6 +43,18 @@ module Arel
         }
       end
 
+      it 'splits orders with commas and function calls' do
+        # *sigh*
+        select = "DISTINCT foo.id, FIRST_VALUE(projects.name) OVER (foo) AS alias_0__"
+        stmt = Nodes::SelectStatement.new
+        stmt.cores.first.projections << Nodes::SqlLiteral.new(select)
+        stmt.orders << Nodes::SqlLiteral.new('NVL(LOWER(bar, foo), foo) DESC, UPPER(baz)')
+        sql = @visitor.accept(stmt)
+        sql.must_be_like %{
+          SELECT #{select} ORDER BY alias_0__ DESC, alias_1__
+        }
+      end
+
       describe 'Nodes::SelectStatement' do
         describe 'limit' do
           it 'adds a rownum clause' do