From 22d82b667f9d474c56013a40d61cf36891ba4bb3 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 18 Mar 2019 14:55:06 -0700 Subject: [PATCH 1/2] Remove roflscaling roflscaling (using frozen string constants instead of literal strings) was added in 2012, before frozen string literals were added in Ruby 2.3. Now that Rails no longer supports Ruby <2.3, and all of these files use frozen string literals, there is no reason to keep the roflscaling. This does not delete or deprecate the related constants. Such a change can be made in a later commit. --- activerecord/lib/arel/visitors/mssql.rb | 2 +- activerecord/lib/arel/visitors/postgresql.rb | 9 +-- activerecord/lib/arel/visitors/to_sql.rb | 85 +++++--------------- 3 files changed, 26 insertions(+), 70 deletions(-) diff --git a/activerecord/lib/arel/visitors/mssql.rb b/activerecord/lib/arel/visitors/mssql.rb index 85815baca2..deb87242b7 100644 --- a/activerecord/lib/arel/visitors/mssql.rb +++ b/activerecord/lib/arel/visitors/mssql.rb @@ -106,7 +106,7 @@ module Arel # :nodoc: all collector = visit o.relation, collector if o.wheres.any? collector << " WHERE " - inject_join o.wheres, collector, AND + inject_join o.wheres, collector, " AND " else collector end diff --git a/activerecord/lib/arel/visitors/postgresql.rb b/activerecord/lib/arel/visitors/postgresql.rb index 920776b4dc..68ec6c02e2 100644 --- a/activerecord/lib/arel/visitors/postgresql.rb +++ b/activerecord/lib/arel/visitors/postgresql.rb @@ -57,23 +57,22 @@ module Arel # :nodoc: all end def visit_Arel_Nodes_Cube(o, collector) - collector << CUBE + collector << "CUBE" grouping_array_or_grouping_element o, collector end def visit_Arel_Nodes_RollUp(o, collector) - collector << ROLLUP + collector << "ROLLUP" grouping_array_or_grouping_element o, collector end def visit_Arel_Nodes_GroupingSet(o, collector) - collector << GROUPING_SETS + collector << "GROUPING SETS" grouping_array_or_grouping_element o, collector end def visit_Arel_Nodes_Lateral(o, collector) - collector << LATERAL - collector << SPACE + collector << "LATERAL " grouping_parentheses o, collector end diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index 583f920290..a93a318e0d 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -9,49 +9,6 @@ module Arel # :nodoc: all end class ToSql < Arel::Visitors::Visitor - ## - # This is some roflscale crazy stuff. I'm roflscaling this because - # building SQL queries is a hotspot. I will explain the roflscale so that - # others will not rm this code. - # - # In YARV, string literals in a method body will get duped when the byte - # code is executed. Let's take a look: - # - # > puts RubyVM::InstructionSequence.new('def foo; "bar"; end').disasm - # - # == disasm: >===== - # 0000 trace 8 - # 0002 trace 1 - # 0004 putstring "bar" - # 0006 trace 16 - # 0008 leave - # - # The `putstring` bytecode will dup the string and push it on the stack. - # In many cases in our SQL visitor, that string is never mutated, so there - # is no need to dup the literal. - # - # If we change to a constant lookup, the string will not be duped, and we - # can reduce the objects in our system: - # - # > puts RubyVM::InstructionSequence.new('BAR = "bar"; def foo; BAR; end').disasm - # - # == disasm: >======== - # 0000 trace 8 - # 0002 trace 1 - # 0004 getinlinecache 11, - # 0007 getconstant :BAR - # 0009 setinlinecache - # 0011 trace 16 - # 0013 leave - # - # `getconstant` should be a hash lookup, and no object is duped when the - # value of the constant is pushed on the stack. Hence the crazy - # constants below. - # - # `matches` and `doesNotMatch` operate case-insensitively via Visitor subclasses - # specialized for specific databases when necessary. - # - WHERE = " WHERE " # :nodoc: SPACE = " " # :nodoc: COMMA = ", " # :nodoc: @@ -161,10 +118,10 @@ module Arel # :nodoc: all else collector << quote(value).to_s end - collector << COMMA unless k == row_len + collector << ", " unless k == row_len end collector << ")" - collector << COMMA unless i == len + collector << ", " unless i == len } collector end @@ -172,7 +129,7 @@ module Arel # :nodoc: all def visit_Arel_Nodes_SelectStatement(o, collector) if o.with collector = visit o.with, collector - collector << SPACE + collector << " " end collector = o.cores.inject(collector) { |c, x| @@ -180,11 +137,11 @@ module Arel # :nodoc: all } unless o.orders.empty? - collector << ORDER_BY + collector << " ORDER BY " len = o.orders.length - 1 o.orders.each_with_index { |x, i| collector = visit(x, collector) - collector << COMMA unless len == i + collector << ", " unless len == i } end @@ -203,17 +160,17 @@ module Arel # :nodoc: all collector = collect_optimizer_hints(o, collector) collector = maybe_visit o.set_quantifier, collector - collect_nodes_for o.projections, collector, SPACE + collect_nodes_for o.projections, collector, " " if o.source && !o.source.empty? collector << " FROM " collector = visit o.source, collector end - collect_nodes_for o.wheres, collector, WHERE, AND - collect_nodes_for o.groups, collector, GROUP_BY - collect_nodes_for o.havings, collector, " HAVING ", AND - collect_nodes_for o.windows, collector, WINDOW + collect_nodes_for o.wheres, collector, " WHERE ", " AND " + collect_nodes_for o.groups, collector, " GROUP BY " + collect_nodes_for o.havings, collector, " HAVING ", " AND " + collect_nodes_for o.windows, collector, " WINDOW " collector end @@ -222,7 +179,7 @@ module Arel # :nodoc: all collector << "/*+ #{sanitize_as_sql_comment(o).join(" ")} */" end - def collect_nodes_for(nodes, collector, spacer, connector = COMMA) + def collect_nodes_for(nodes, collector, spacer, connector = ", ") unless nodes.empty? collector << spacer inject_join nodes, collector, connector @@ -234,7 +191,7 @@ module Arel # :nodoc: all end def visit_Arel_Nodes_Distinct(o, collector) - collector << DISTINCT + collector << "DISTINCT" end def visit_Arel_Nodes_DistinctOn(o, collector) @@ -243,12 +200,12 @@ module Arel # :nodoc: all def visit_Arel_Nodes_With(o, collector) collector << "WITH " - inject_join o.children, collector, COMMA + inject_join o.children, collector, ", " end def visit_Arel_Nodes_WithRecursive(o, collector) collector << "WITH RECURSIVE " - inject_join o.children, collector, COMMA + inject_join o.children, collector, ", " end def visit_Arel_Nodes_Union(o, collector) @@ -281,13 +238,13 @@ module Arel # :nodoc: all collect_nodes_for o.partitions, collector, "PARTITION BY " if o.orders.any? - collector << SPACE if o.partitions.any? + collector << " " if o.partitions.any? collector << "ORDER BY " collector = inject_join o.orders, collector, ", " end if o.framing - collector << SPACE if o.partitions.any? || o.orders.any? + collector << " " if o.partitions.any? || o.orders.any? collector = visit o.framing, collector end @@ -492,8 +449,8 @@ module Arel # :nodoc: all collector = visit o.left, collector end if o.right.any? - collector << SPACE if o.left - collector = inject_join o.right, collector, SPACE + collector << " " if o.left + collector = inject_join o.right, collector, " " end collector end @@ -513,7 +470,7 @@ module Arel # :nodoc: all def visit_Arel_Nodes_FullOuterJoin(o, collector) collector << "FULL OUTER JOIN " collector = visit o.left, collector - collector << SPACE + collector << " " visit o.right, collector end @@ -527,7 +484,7 @@ module Arel # :nodoc: all def visit_Arel_Nodes_RightOuterJoin(o, collector) collector << "RIGHT OUTER JOIN " collector = visit o.left, collector - collector << SPACE + collector << " " visit o.right, collector end @@ -535,7 +492,7 @@ module Arel # :nodoc: all collector << "INNER JOIN " collector = visit o.left, collector if o.right - collector << SPACE + collector << " " visit(o.right, collector) else collector From 6fe624f50d2572c6f7ac3062e9317abead07863e Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 19 Mar 2019 08:48:13 -0700 Subject: [PATCH 2/2] Remove roflscaling constants --- activerecord/lib/arel/visitors/postgresql.rb | 5 ----- activerecord/lib/arel/visitors/to_sql.rb | 10 ---------- 2 files changed, 15 deletions(-) diff --git a/activerecord/lib/arel/visitors/postgresql.rb b/activerecord/lib/arel/visitors/postgresql.rb index 68ec6c02e2..8296f1cdc1 100644 --- a/activerecord/lib/arel/visitors/postgresql.rb +++ b/activerecord/lib/arel/visitors/postgresql.rb @@ -3,11 +3,6 @@ module Arel # :nodoc: all module Visitors class PostgreSQL < Arel::Visitors::ToSql - CUBE = "CUBE" - ROLLUP = "ROLLUP" - GROUPING_SETS = "GROUPING SETS" - LATERAL = "LATERAL" - private def visit_Arel_Nodes_Matches(o, collector) diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index a93a318e0d..73fef94499 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -9,16 +9,6 @@ module Arel # :nodoc: all end class ToSql < Arel::Visitors::Visitor - WHERE = " WHERE " # :nodoc: - SPACE = " " # :nodoc: - COMMA = ", " # :nodoc: - GROUP_BY = " GROUP BY " # :nodoc: - ORDER_BY = " ORDER BY " # :nodoc: - WINDOW = " WINDOW " # :nodoc: - AND = " AND " # :nodoc: - - DISTINCT = "DISTINCT" # :nodoc: - def initialize(connection) super() @connection = connection