Get rid of `Arel::Nodes::Values`

That is completely covered by `Arel::Nodes::ValuesList`.

Related https://github.com/rails/arel/pull/484.

activerecord/lib/arel/insert_manager.rb

@@ -33,13 +33,13 @@ def insert(fields)
           @ast.columns << column
           values << value
         end
-        @ast.values = create_values values, @ast.columns
+        @ast.values = create_values(values)
       end
       self
     end
 
-    def create_values(values, columns)
-      Nodes::Values.new values, columns
+    def create_values(values)
+      Nodes::ValuesList.new([values])
     end
 
     def create_values_list(rows)

activerecord/lib/arel/nodes.rb

@@ -45,7 +45,6 @@
 require "arel/nodes/function"
 require "arel/nodes/count"
 require "arel/nodes/extract"
-require "arel/nodes/values"
 require "arel/nodes/values_list"
 require "arel/nodes/named_function"
 

activerecord/lib/arel/nodes/values.rb

-# frozen_string_literal: true
-
-module Arel # :nodoc: all
-  module Nodes
-    class Values < Arel::Nodes::Binary
-      alias :expressions :left
-      alias :expressions= :left=
-      alias :columns :right
-      alias :columns= :right=
-
-      def initialize(exprs, columns = [])
-        super
-      end
-    end
-  end
-end

activerecord/lib/arel/nodes/values_list.rb

@@ -2,23 +2,8 @@
 
 module Arel # :nodoc: all
   module Nodes
-    class ValuesList < Node
-      attr_reader :rows
-
-      def initialize(rows)
-        @rows = rows
-        super()
-      end
-
-      def hash
-        @rows.hash
-      end
-
-      def eql?(other)
-        self.class == other.class &&
-          self.rows == other.rows
-      end
-      alias :== :eql?
+    class ValuesList < Unary
+      alias :rows :expr
     end
   end
 end

activerecord/lib/arel/visitors/depth_first.rb

@@ -36,6 +36,7 @@ def unary(o)
         alias :visit_Arel_Nodes_Descending        :unary
         alias :visit_Arel_Nodes_UnqualifiedColumn :unary
         alias :visit_Arel_Nodes_OptimizerHints    :unary
+        alias :visit_Arel_Nodes_ValuesList        :unary
 
         def function(o)
           visit o.expressions
@@ -103,7 +104,6 @@ def binary(o)
         alias :visit_Arel_Nodes_Regexp             :binary
         alias :visit_Arel_Nodes_RightOuterJoin     :binary
         alias :visit_Arel_Nodes_TableAlias         :binary
-        alias :visit_Arel_Nodes_Values             :binary
         alias :visit_Arel_Nodes_When               :binary
 
         def visit_Arel_Nodes_StringJoin(o)

activerecord/lib/arel/visitors/dot.rb

@@ -46,8 +46,8 @@ def visit_Arel_Nodes_Count(o)
           visit_edge o, "distinct"
         end
 
-        def visit_Arel_Nodes_Values(o)
-          visit_edge o, "expressions"
+        def visit_Arel_Nodes_ValuesList(o)
+          visit_edge o, "rows"
         end
 
         def visit_Arel_Nodes_StringJoin(o)

activerecord/lib/arel/visitors/to_sql.rb

@@ -159,7 +159,7 @@ def visit_Arel_Nodes_ValuesList(o, collector)
               when Nodes::SqlLiteral, Nodes::BindParam
                 collector = visit(value, collector)
               else
-                collector << quote(value)
+                collector << quote(value).to_s
               end
               collector << COMMA unless k == row_len
             end
@@ -169,25 +169,6 @@ def visit_Arel_Nodes_ValuesList(o, collector)
           collector
         end
 
-        def visit_Arel_Nodes_Values(o, collector)
-          collector << "VALUES ("
-
-          len = o.expressions.length - 1
-          o.expressions.each_with_index { |value, i|
-            case value
-            when Nodes::SqlLiteral, Nodes::BindParam
-              collector = visit value, collector
-            else
-              collector << quote(value).to_s
-            end
-            unless i == len
-              collector << COMMA
-            end
-          }
-
-          collector << ")"
-        end
-
         def visit_Arel_Nodes_SelectStatement(o, collector)
           if o.with
             collector = visit o.with, collector

activerecord/test/cases/arel/insert_manager_test.rb

@@ -11,19 +11,18 @@ class InsertManagerTest < Arel::Spec
     end
 
     describe "insert" do
-      it "can create a Values node" do
+      it "can create a ValuesList node" do
         manager = Arel::InsertManager.new
-        values  = manager.create_values %w{ a b }, %w{ c d }
+        values  = manager.create_values_list([%w{ a b }, %w{ c d }])
 
-        assert_kind_of Arel::Nodes::Values, values
-        assert_equal %w{ a b }, values.left
-        assert_equal %w{ c d }, values.right
+        assert_kind_of Arel::Nodes::ValuesList, values
+        assert_equal [%w{ a b }, %w{ c d }], values.rows
       end
 
       it "allows sql literals" do
         manager = Arel::InsertManager.new
         manager.into Table.new(:users)
-        manager.values = manager.create_values [Arel.sql("*")], %w{ a }
+        manager.values = manager.create_values([Arel.sql("*")])
         manager.to_sql.must_be_like %{
           INSERT INTO \"users\" VALUES (*)
         }
@@ -186,9 +185,9 @@ class InsertManagerTest < Arel::Spec
         manager = Arel::InsertManager.new
         manager.into table
 
-        manager.values = Nodes::Values.new [1]
+        manager.values = Nodes::ValuesList.new([[1], [2]])
         manager.to_sql.must_be_like %{
-          INSERT INTO "users" VALUES (1)
+          INSERT INTO "users" VALUES (1), (2)
         }
       end
 
@@ -210,11 +209,11 @@ class InsertManagerTest < Arel::Spec
         manager = Arel::InsertManager.new
         manager.into table
 
-        manager.values = Nodes::Values.new [1, "aaron"]
+        manager.values = Nodes::ValuesList.new([[1, "aaron"], [2, "david"]])
         manager.columns << table[:id]
         manager.columns << table[:name]
         manager.to_sql.must_be_like %{
-          INSERT INTO "users" ("id", "name") VALUES (1, 'aaron')
+          INSERT INTO "users" ("id", "name") VALUES (1, 'aaron'), (2, 'david')
         }
       end
     end

activerecord/test/cases/arel/visitors/depth_first_test.rb

@@ -33,6 +33,7 @@ def test_raises_with_object
         Arel::Nodes::Ordering,
         Arel::Nodes::StringJoin,
         Arel::Nodes::UnqualifiedColumn,
+        Arel::Nodes::ValuesList,
         Arel::Nodes::Limit,
         Arel::Nodes::Else,
       ].each do |klass|
@@ -116,7 +117,6 @@ def test_right_outer_join
         Arel::Nodes::NotIn,
         Arel::Nodes::Or,
         Arel::Nodes::TableAlias,
-        Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
         Arel::Nodes::JoinSource,

activerecord/test/cases/arel/visitors/dot_test.rb

@@ -37,6 +37,7 @@ def test_named_function
         Arel::Nodes::Offset,
         Arel::Nodes::Ordering,
         Arel::Nodes::UnqualifiedColumn,
+        Arel::Nodes::ValuesList,
         Arel::Nodes::Limit,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do
@@ -61,7 +62,6 @@ def test_named_function
         Arel::Nodes::NotIn,
         Arel::Nodes::Or,
         Arel::Nodes::TableAlias,
-        Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
         Arel::Nodes::JoinSource,

activerecord/test/cases/arel/visitors/to_sql_test.rb

@@ -23,9 +23,9 @@ def compile(node)
         sql.must_be_like "?"
       end
 
-      it "does not quote BindParams used as part of a Values" do
+      it "does not quote BindParams used as part of a ValuesList" do
         bp = Nodes::BindParam.new(1)
-        values = Nodes::Values.new([bp])
+        values = Nodes::ValuesList.new([[bp]])
         sql = compile values
         sql.must_be_like "VALUES (?)"
       end