From 36dc442a96c31bb81d7425d94873b45405e8eefc Mon Sep 17 00:00:00 2001 From: mvgijssel Date: Tue, 30 Jul 2019 22:43:43 +0200 Subject: [PATCH] Add querying and additional context to the enhanced Arel AST (#106) * Add contextual information to transformer nodes * Sort rubocop * Fixed lint issues * Fix comparison for select,update,insert and delete manager * Added .query method to Node * Added AddchemaToTable transformer * Add support for schemas in function calls * Add missing attributes to the SelectCore dot visitor * Added TODO to Arel middleware --- .rubocop.yml | 15 +- lib/arel/extensions/delete_manager.rb | 2 +- lib/arel/extensions/delete_statement.rb | 2 +- lib/arel/extensions/function.rb | 3 + lib/arel/extensions/insert_manager.rb | 2 +- lib/arel/extensions/select_manager.rb | 2 +- lib/arel/extensions/select_statement.rb | 3 + lib/arel/extensions/table.rb | 2 +- lib/arel/extensions/update_manager.rb | 2 +- lib/arel/sql_to_arel/pg_query_visitor.rb | 21 ++- .../pg_query_visitor/frame_options.rb | 2 +- lib/arel/transformer.rb | 3 + lib/arel/transformer/add_schema_to_table.rb | 26 ++++ .../context_enhancer/arel_table.rb | 75 ++++++++++ lib/arel/transformer/node.rb | 17 ++- lib/arel/transformer/path_node.rb | 2 +- lib/arel/transformer/query.rb | 36 +++++ lib/arel/transformer/visitor.rb | 25 +++- spec/arel/extensions/delete_manager_spec.rb | 7 + spec/arel/extensions/insert_manager_spec.rb | 7 + spec/arel/extensions/select_manager_spec.rb | 7 + spec/arel/extensions/update_manager_spec.rb | 7 + .../arel/sql_to_arel/pg_query_visitor_spec.rb | 29 ++++ spec/arel/sql_to_arel_spec.rb | 1 + .../transformer/add_schema_to_table_spec.rb | 23 +++ .../context_enhancer/arel_table_spec.rb | 133 ++++++++++++++++++ spec/arel/transformer/query_spec.rb | 58 ++++++++ .../prints_a_pretty_ast.approved.txt | 68 +++++---- spec/pg_search_spec.rb | 2 +- .../dummy/config/environments/development.rb | 2 +- .../support/dummy/config/environments/test.rb | 2 +- 31 files changed, 532 insertions(+), 54 deletions(-) create mode 100644 lib/arel/transformer/add_schema_to_table.rb create mode 100644 lib/arel/transformer/context_enhancer/arel_table.rb create mode 100644 lib/arel/transformer/query.rb create mode 100644 spec/arel/transformer/add_schema_to_table_spec.rb create mode 100644 spec/arel/transformer/context_enhancer/arel_table_spec.rb create mode 100644 spec/arel/transformer/query_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 7c5e8a4b..af101286 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -3,18 +3,12 @@ AllCops: - 'gemfiles/arel_gems.gemfile' - 'gemfiles/default.gemfile' -Style/FrozenStringLiteralComment: - Enabled: false - Bundler/OrderedGems: Enabled: false Gemspec/OrderedDependencies: Enabled: false -Style/MultilineBlockChain: - Enabled: false - Metrics/LineLength: Enabled: true Max: 100 @@ -31,12 +25,21 @@ Metrics/BlockLength: Style/Documentation: Enabled: false +Style/MultilineBlockChain: + Enabled: false + +Style/TrailingCommaInHashLiteral: + EnforcedStyleForMultiline: comma + Style/TrailingCommaInArrayLiteral: EnforcedStyleForMultiline: comma Style/TrailingCommaInArguments: EnforcedStyleForMultiline: comma +Style/FrozenStringLiteralComment: + Enabled: false + Layout/MultilineMethodCallIndentation: Enabled: true EnforcedStyle: indented diff --git a/lib/arel/extensions/delete_manager.rb b/lib/arel/extensions/delete_manager.rb index fb7a0153..ebefc06c 100644 --- a/lib/arel/extensions/delete_manager.rb +++ b/lib/arel/extensions/delete_manager.rb @@ -4,7 +4,7 @@ module Arel class DeleteManager < Arel::TreeManager def ==(other) - @ast == other.ast && @ctx == other.ctx + other.is_a?(self.class) && @ast == other.ast && @ctx == other.ctx end protected diff --git a/lib/arel/extensions/delete_statement.rb b/lib/arel/extensions/delete_statement.rb index fdb4f4ac..16c3af79 100644 --- a/lib/arel/extensions/delete_statement.rb +++ b/lib/arel/extensions/delete_statement.rb @@ -3,7 +3,7 @@ module Arel module Nodes - # https://www.postgresql.org/docs/9.5/sql-insert.html + # https://www.postgresql.org/docs/10/sql-delete.html class DeleteStatement module DeleteStatementExtension attr_accessor :using diff --git a/lib/arel/extensions/function.rb b/lib/arel/extensions/function.rb index 4b188ee4..be7c0a51 100644 --- a/lib/arel/extensions/function.rb +++ b/lib/arel/extensions/function.rb @@ -10,6 +10,8 @@ module FunctionExtension attr_accessor :filter attr_accessor :within_group attr_accessor :variardic + # postgres only: https://www.postgresql.org/docs/10/ddl-schemas.html + attr_accessor :schema_name def initialize(expr, aliaz = nil) super @@ -31,6 +33,7 @@ class ToSql # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/AbcSize def aggregate(name, o, collector) + collector << "#{o.schema_name}." if o.schema_name collector << "#{name}(" collector << 'DISTINCT ' if o.distinct collector << 'VARIADIC ' if o.variardic diff --git a/lib/arel/extensions/insert_manager.rb b/lib/arel/extensions/insert_manager.rb index f4cc1d2b..7dedb450 100644 --- a/lib/arel/extensions/insert_manager.rb +++ b/lib/arel/extensions/insert_manager.rb @@ -4,7 +4,7 @@ module Arel class InsertManager < Arel::TreeManager def ==(other) - @ast == other.ast + other.is_a?(self.class) && @ast == other.ast end end diff --git a/lib/arel/extensions/select_manager.rb b/lib/arel/extensions/select_manager.rb index b5539d94..90c74ee1 100644 --- a/lib/arel/extensions/select_manager.rb +++ b/lib/arel/extensions/select_manager.rb @@ -4,7 +4,7 @@ module Arel class SelectManager def ==(other) - @ast == other.ast && @ctx == other.ctx + other.is_a?(self.class) && @ast == other.ast && @ctx == other.ctx end protected diff --git a/lib/arel/extensions/select_statement.rb b/lib/arel/extensions/select_statement.rb index 1128bb7b..77dc3a23 100644 --- a/lib/arel/extensions/select_statement.rb +++ b/lib/arel/extensions/select_statement.rb @@ -32,7 +32,10 @@ module SelectStatementExtension def visit_Arel_Nodes_SelectStatement(o) super + visit_edge o, 'lock' + visit_edge o, 'with' visit_edge o, 'union' + visit_edge o, 'values_lists' end end diff --git a/lib/arel/extensions/table.rb b/lib/arel/extensions/table.rb index 28f4949e..1ef2e078 100644 --- a/lib/arel/extensions/table.rb +++ b/lib/arel/extensions/table.rb @@ -7,7 +7,7 @@ class Table module TableExtension # postgres only: https://www.postgresql.org/docs/9.5/sql-select.html attr_accessor :only - # postgres only: https://www.postgresql.org/docs/9.5/ddl-schemas.html + # postgres only: https://www.postgresql.org/docs/10/ddl-schemas.html attr_accessor :schema_name # postgres only: https://www.postgresql.org/docs/9.1/catalog-pg-class.html attr_accessor :relpersistence diff --git a/lib/arel/extensions/update_manager.rb b/lib/arel/extensions/update_manager.rb index 87337356..dc986ddf 100644 --- a/lib/arel/extensions/update_manager.rb +++ b/lib/arel/extensions/update_manager.rb @@ -4,7 +4,7 @@ module Arel class UpdateManager < Arel::TreeManager def ==(other) - @ast == other.ast && @ctx == other.ctx + other.is_a?(self.class) && @ast == other.ast && @ctx == other.ctx end protected diff --git a/lib/arel/sql_to_arel/pg_query_visitor.rb b/lib/arel/sql_to_arel/pg_query_visitor.rb index b2dad442..d04f9c96 100644 --- a/lib/arel/sql_to_arel/pg_query_visitor.rb +++ b/lib/arel/sql_to_arel/pg_query_visitor.rb @@ -427,11 +427,20 @@ def visit_FuncCall( [Arel::Nodes::Overlaps.new(start1, end1, start2, end2)] else - if function_names.length > 1 - boom "Don't know how to handle function names `#{function_names}`" - end + case function_names.length + when 2 + if function_names.first == PG_CATALOG + boom "Missing postgres function `#{function_names.last}`" + end - Arel::Nodes::NamedFunction.new(function_names.first, args) + func = Arel::Nodes::NamedFunction.new(function_names.last, args) + func.schema_name = function_names.first + func + when 1 + Arel::Nodes::NamedFunction.new(function_names.first, args) + else + boom "Don't know how to handle function names length `#{function_names.length}`" + end end func.distinct = (agg_distinct.nil? ? false : true) unless func.is_a?(::Array) @@ -540,12 +549,12 @@ def visit_LockingClause(strength:, wait_policy:) 1 => 'FOR KEY SHARE', 2 => 'FOR SHARE', 3 => 'FOR NO KEY UPDATE', - 4 => 'FOR UPDATE' + 4 => 'FOR UPDATE', }.fetch(strength) wait_policy_clause = { 0 => '', 1 => ' SKIP LOCKED', - 2 => ' NOWAIT' + 2 => ' NOWAIT', }.fetch(wait_policy) Arel::Nodes::Lock.new Arel.sql("#{strength_clause}#{wait_policy_clause}") diff --git a/lib/arel/sql_to_arel/pg_query_visitor/frame_options.rb b/lib/arel/sql_to_arel/pg_query_visitor/frame_options.rb index 29e642c1..3d7d5f43 100644 --- a/lib/arel/sql_to_arel/pg_query_visitor/frame_options.rb +++ b/lib/arel/sql_to_arel/pg_query_visitor/frame_options.rb @@ -56,7 +56,7 @@ def arel(frame_options, start_offset, end_offset) 'FRAMEOPTION_START_VALUE_PRECEDING' => 0x00400, 'FRAMEOPTION_END_VALUE_PRECEDING' => 0x00800, 'FRAMEOPTION_START_VALUE_FOLLOWING' => 0x01000, - 'FRAMEOPTION_END_VALUE_FOLLOWING' => 0x02000 + 'FRAMEOPTION_END_VALUE_FOLLOWING' => 0x02000, }.freeze def biggest_detractable_number(number, candidates) diff --git a/lib/arel/transformer.rb b/lib/arel/transformer.rb index 38fd2b48..7c558e70 100644 --- a/lib/arel/transformer.rb +++ b/lib/arel/transformer.rb @@ -1,8 +1,11 @@ require_relative './transformer/node' require_relative './transformer/path' require_relative './transformer/path_node' +require_relative './transformer/query' require_relative './transformer/visitor' +require_relative './transformer/add_schema_to_table' + module Arel module Transformer end diff --git a/lib/arel/transformer/add_schema_to_table.rb b/lib/arel/transformer/add_schema_to_table.rb new file mode 100644 index 00000000..59ae2f19 --- /dev/null +++ b/lib/arel/transformer/add_schema_to_table.rb @@ -0,0 +1,26 @@ +module Arel + module Transformer + class AddSchemaToTable + attr_reader :schema_name + + def initialize(schema_name) + @schema_name = schema_name + end + + # https://github.com/mvgijssel/arel_toolkit/issues/110 + def call(arel, _context) + tree = Arel.transformer(arel) + + tree.query( + class: Arel::Table, + schema_name: nil, + context: { range_variable: true }, + ).each do |node| + node['schema_name'].replace(schema_name) + end + + tree.object + end + end + end +end diff --git a/lib/arel/transformer/context_enhancer/arel_table.rb b/lib/arel/transformer/context_enhancer/arel_table.rb new file mode 100644 index 00000000..6a3d159a --- /dev/null +++ b/lib/arel/transformer/context_enhancer/arel_table.rb @@ -0,0 +1,75 @@ +module Arel + module Transformer + module ContextEnhancer + class ArelTable + # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/CyclomaticComplexity + # rubocop:disable Metrics/AbcSize + def self.call(node) + context = node.context.merge!(range_variable: false, column_reference: false) + parent_object = node.parent.object + + # Using Arel::Table as SELECT ... FROM + if parent_object.is_a?(Arel::Nodes::JoinSource) + context[:range_variable] = true + + # Using Arel::Table as SELECT ... FROM [
] + elsif parent_object.is_a?(Array) && + node.parent.parent.object.is_a?(Arel::Nodes::JoinSource) + context[:range_variable] = true + + # Using Arel::Table as SELECT ... INNER JOIN
ON TRUE + elsif parent_object.is_a?(Arel::Nodes::Join) + context[:range_variable] = true + + # Using Arel::Table as an attribute SELECT
.id ... + elsif parent_object.is_a?(Arel::Attributes::Attribute) + context[:column_reference] = true + + # Using Arel::Table in an INSERT INTO
+ elsif parent_object.is_a?(Arel::Nodes::InsertStatement) + context[:range_variable] = true + + # Using Arel::Table in an UPDATE
... + elsif parent_object.is_a?(Arel::Nodes::UpdateStatement) + context[:range_variable] = true + + # Arel::Table in UPDATE ... FROM [
] + elsif parent_object.is_a?(Array) && + node.parent.parent.object.is_a?(Arel::Nodes::UpdateStatement) + context[:range_variable] = true + + # Using Arel::Table in an DELETE FROM
+ elsif parent_object.is_a?(Arel::Nodes::DeleteStatement) + context[:range_variable] = true + + # Arel::Table in DELETE ... USING [
] + elsif parent_object.is_a?(Array) && + node.parent.parent.object.is_a?(Arel::Nodes::DeleteStatement) + context[:range_variable] = true + + # Using Arel::Table as an "alias" for WITH
AS (SELECT 1) SELECT 1 + elsif parent_object.is_a?(Arel::Nodes::As) && + node.parent.parent.parent.object.is_a?(Arel::Nodes::With) + context[:alias] = true + + # Using Arel::Table as an "alias" for WITH RECURSIVE
AS (SELECT 1) SELECT 1 + elsif parent_object.is_a?(Arel::Nodes::As) && + node.parent.parent.parent.object.is_a?(Arel::Nodes::WithRecursive) + context[:alias] = true + + # Using Arel::Table as an "alias" for SELECT INTO
... + elsif parent_object.is_a?(Arel::Nodes::Into) + context[:alias] = true + + else + raise "Unknown AST location for table #{node.inspect}, #{node.root_node.to_sql}" + end + end + # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/AbcSize + end + end + end +end diff --git a/lib/arel/transformer/node.rb b/lib/arel/transformer/node.rb index 92c36ae2..33e9fb59 100644 --- a/lib/arel/transformer/node.rb +++ b/lib/arel/transformer/node.rb @@ -7,6 +7,7 @@ class Node attr_reader :fields attr_reader :children attr_reader :root_node + attr_reader :context def initialize(object) @object = object @@ -15,6 +16,7 @@ def initialize(object) @fields = [] @children = {} @dirty = false + @context = {} end def inspect @@ -63,7 +65,7 @@ def add(path_node, node) def to_sql(engine = Table.engine) return nil if children.empty? - target_object = object.is_a?(Arel::SelectManager) ? object.ast : object + target_object = object.is_a?(Arel::TreeManager) ? object.ast : object collector = Arel::Collectors::SQLString.new collector = engine.connection.visitor.accept target_object, collector collector.value @@ -73,6 +75,19 @@ def [](key) @children.fetch(key) end + def child_at_path(path_items) + selected_node = self + path_items.each do |path_item| + selected_node = selected_node[path_item] + return nil if selected_node.nil? + end + selected_node + end + + def query(**kwargs) + Arel::Transformer::Query.call(self, kwargs) + end + protected attr_writer :path diff --git a/lib/arel/transformer/path_node.rb b/lib/arel/transformer/path_node.rb index cff9e85c..c9e209d0 100644 --- a/lib/arel/transformer/path_node.rb +++ b/lib/arel/transformer/path_node.rb @@ -16,7 +16,7 @@ def arguments? def inspect case value when String - "\"#{value}\"" + "'#{value}'" else value.inspect end diff --git a/lib/arel/transformer/query.rb b/lib/arel/transformer/query.rb new file mode 100644 index 00000000..470cbf24 --- /dev/null +++ b/lib/arel/transformer/query.rb @@ -0,0 +1,36 @@ +module Arel + module Transformer + class Query + def self.call(node, kwargs) + node_attributes = %i[context parent] + node_args = kwargs.slice(*node_attributes) + object_args = kwargs.except(*node_attributes) + + node.each.select do |child_node| + next unless matches?(child_node, node_args) + + matches?(child_node.object, object_args) + end + end + + def self.matches?(object, test) + case test + when Hash + case object + when Hash + test <= object + else + test.all? do |test_key, test_value| + next false unless object.respond_to?(test_key) + + object_attribute_value = object.public_send(test_key) + matches? object_attribute_value, test_value + end + end + else + object == test + end + end + end + end +end diff --git a/lib/arel/transformer/visitor.rb b/lib/arel/transformer/visitor.rb index bd92681c..742fd2da 100644 --- a/lib/arel/transformer/visitor.rb +++ b/lib/arel/transformer/visitor.rb @@ -1,13 +1,25 @@ +require_relative './context_enhancer/arel_table' + module Arel module Transformer # rubocop:disable Naming/MethodName class Visitor < Arel::Visitors::Dot - def accept(object) + DEFAULT_CONTEXT_ENHANCERS = { + Arel::Table => Arel::Transformer::ContextEnhancer::ArelTable, + }.freeze + + attr_reader :context_enhancers + + def accept(object, context_enhancers = DEFAULT_CONTEXT_ENHANCERS) + @context_enhancers = context_enhancers + root_node = Arel::Transformer::Node.new(object) accept_with_root(object, root_node) end - def accept_with_root(object, root_node) + def accept_with_root(object, root_node, context_enhancers = DEFAULT_CONTEXT_ENHANCERS) + @context_enhancers = context_enhancers + with_node(root_node) do visit object end @@ -42,6 +54,8 @@ def process_node(arel_node, path_node) node = Arel::Transformer::Node.new(arel_node) current_node.add(path_node, node) + update_context(node) + with_node node do visit arel_node end @@ -54,6 +68,13 @@ def visit(object) def current_node @node_stack.last end + + def update_context(node) + enhancer = context_enhancers[node.object.class] + return if enhancer.nil? + + enhancer.call(node) + end end # rubocop:enable Naming/MethodName end diff --git a/spec/arel/extensions/delete_manager_spec.rb b/spec/arel/extensions/delete_manager_spec.rb index f97eb37c..98ecb284 100644 --- a/spec/arel/extensions/delete_manager_spec.rb +++ b/spec/arel/extensions/delete_manager_spec.rb @@ -13,5 +13,12 @@ expect(tree1).to_not eq(tree2) end + + it 'works for comparing other objects' do + tree = Arel::DeleteManager.new.tap { |d| d.from(Arel::Table.new('posts')) } + other_object = 'foo' + + expect(tree).to_not eq other_object + end end end diff --git a/spec/arel/extensions/insert_manager_spec.rb b/spec/arel/extensions/insert_manager_spec.rb index 40f818d9..ff606c2b 100644 --- a/spec/arel/extensions/insert_manager_spec.rb +++ b/spec/arel/extensions/insert_manager_spec.rb @@ -13,5 +13,12 @@ expect(tree1).to_not eq(tree2) end + + it 'works for comparing other objects' do + tree = Arel::InsertManager.new.tap { |i| i.into(Arel::Table.new('posts')) } + other_object = 'foo' + + expect(tree).to_not eq other_object + end end end diff --git a/spec/arel/extensions/select_manager_spec.rb b/spec/arel/extensions/select_manager_spec.rb index fe6bbfe1..3953411f 100644 --- a/spec/arel/extensions/select_manager_spec.rb +++ b/spec/arel/extensions/select_manager_spec.rb @@ -13,5 +13,12 @@ expect(tree1).to_not eq(tree2) end + + it 'works for comparing other objects' do + tree = Arel::SelectManager.new(Arel::Table.new('posts')) + other_object = 'foo' + + expect(tree).to_not eq other_object + end end end diff --git a/spec/arel/extensions/update_manager_spec.rb b/spec/arel/extensions/update_manager_spec.rb index e4e5905d..4010ac8b 100644 --- a/spec/arel/extensions/update_manager_spec.rb +++ b/spec/arel/extensions/update_manager_spec.rb @@ -13,5 +13,12 @@ expect(tree1).to_not eq(tree2) end + + it 'works for comparing other objects' do + tree = Arel::UpdateManager.new.tap { |u| u.table(Arel::Table.new('posts')) } + other_object = 'foo' + + expect(tree).to_not eq other_object + end end end diff --git a/spec/arel/sql_to_arel/pg_query_visitor_spec.rb b/spec/arel/sql_to_arel/pg_query_visitor_spec.rb index 136853df..53f9504f 100644 --- a/spec/arel/sql_to_arel/pg_query_visitor_spec.rb +++ b/spec/arel/sql_to_arel/pg_query_visitor_spec.rb @@ -81,6 +81,35 @@ end end + describe 'visit_FuncCall' do + it 'raises an exception when given an unknown pg_catalog function' do + funcname = [ + { 'String' => { 'str' => 'pg_catalog' } }, + { 'String' => { 'str' => 'some_new_function' } }, + ] + + expect do + described_class.new.send(:visit_FuncCall, funcname: funcname) + end.to raise_error do |error| + expect(error.message).to include('Missing postgres function `some_new_function`') + end + end + + it 'raises an exception when there are more than 2 function names' do + funcname = [ + { 'String' => { 'str' => 'some_schema' } }, + { 'String' => { 'str' => 'some_function' } }, + { 'String' => { 'str' => 'something_unknown' } }, + ] + + expect do + described_class.new.send(:visit_FuncCall, funcname: funcname) + end.to raise_error do |error| + expect(error.message).to include("Don't know how to handle function names length `3`") + end + end + end + describe 'visit_TypeName' do it 'raises an exception when typemod is not -1' do expect do diff --git a/spec/arel/sql_to_arel_spec.rb b/spec/arel/sql_to_arel_spec.rb index 8c22e111..dbd240d5 100644 --- a/spec/arel/sql_to_arel_spec.rb +++ b/spec/arel/sql_to_arel_spec.rb @@ -109,6 +109,7 @@ visit 'select', '1.9', pg_node: 'PgQuery::FLOAT' visit 'select', 'SUM("a") AS some_a_sum', pg_node: 'PgQuery::FUNC_CALL' visit 'select', 'RANK("b")', pg_node: 'PgQuery::FUNC_CALL' + visit 'select', 'public.rank("c")', pg_node: 'PgQuery::FUNC_CALL' visit 'select', 'COUNT("c")', pg_node: 'PgQuery::FUNC_CALL' visit 'select', 'GENERATE_SERIES(1, 5)', pg_node: 'PgQuery::FUNC_CALL' visit 'select', 'MAX("d")', pg_node: 'PgQuery::FUNC_CALL' diff --git a/spec/arel/transformer/add_schema_to_table_spec.rb b/spec/arel/transformer/add_schema_to_table_spec.rb new file mode 100644 index 00000000..28a845fb --- /dev/null +++ b/spec/arel/transformer/add_schema_to_table_spec.rb @@ -0,0 +1,23 @@ +describe Arel::Transformer::AddSchemaToTable do + it 'adds the given schema name to all range variable tables' do + transformer = Arel::Transformer::AddSchemaToTable.new('secret') + sql = 'SELECT posts.id FROM posts INNER JOIN comments ON comments.post_id = posts.id' + result = Arel.sql_to_arel(sql) + new_sql = transformer.call(result.first, nil).to_sql + + expect(new_sql) + .to eq 'SELECT "posts"."id" FROM "secret"."posts" INNER JOIN "secret"."comments" ' \ + 'ON "comments"."post_id" = "posts"."id"' + end + + it 'does not override existing schema name' do + transformer = Arel::Transformer::AddSchemaToTable.new('secret') + sql = 'SELECT posts.id FROM posts INNER JOIN public.comments ON comments.post_id = posts.id' + result = Arel.sql_to_arel(sql) + new_sql = transformer.call(result.first, nil).to_sql + + expect(new_sql) + .to eq 'SELECT "posts"."id" FROM "secret"."posts" INNER JOIN "public"."comments" ' \ + 'ON "comments"."post_id" = "posts"."id"' + end +end diff --git a/spec/arel/transformer/context_enhancer/arel_table_spec.rb b/spec/arel/transformer/context_enhancer/arel_table_spec.rb new file mode 100644 index 00000000..f213b761 --- /dev/null +++ b/spec/arel/transformer/context_enhancer/arel_table_spec.rb @@ -0,0 +1,133 @@ +describe Arel::Transformer::ContextEnhancer::ArelTable do + it 'adds additional context to Arel::Table nodes' do + result = Arel.sql_to_arel('SELECT posts.id FROM posts') + original_arel = result.first + + tree = Arel.transformer(original_arel) + from_table_node = tree.child_at_path(['ast', 'cores', 0, 'source', 'left']) + projection_table_node = tree.child_at_path(['ast', 'cores', 0, 'projections', 0, 'relation']) + + expect(from_table_node.context).to eq(range_variable: true, column_reference: false) + expect(projection_table_node.context).to eq(range_variable: false, column_reference: true) + end + + it 'works for a single table inside FROM' do + sql = 'SELECT * FROM posts' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + table_node = tree.child_at_path(['ast', 'cores', 0, 'source', 'left']) + + expect(table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for multiple tables inside FROM' do + sql = 'SELECT * FROM posts, comments' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + + posts_table_node = tree.child_at_path ['ast', 'cores', 0, 'source', 'left', 0] + comments_table_node = tree.child_at_path ['ast', 'cores', 0, 'source', 'left', 1] + + expect(posts_table_node.context).to eq(range_variable: true, column_reference: false) + expect(comments_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for joining tables' do + sql = 'SELECT * FROM posts INNER JOIN comments ON true LEFT JOIN users ON true' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + + users_table_node = tree.child_at_path ['ast', 'cores', 0, 'source', 'right', 1, 'left'] + comments_table_node = tree.child_at_path ['ast', 'cores', 0, 'source', 'right', 0, 'left'] + + expect(users_table_node.context).to eq(range_variable: true, column_reference: false) + expect(comments_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for SELECT projections' do + sql = 'SELECT posts.id' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + attribute_table_node = tree.child_at_path(['ast', 'cores', 0, 'projections', 0, 'relation']) + + expect(attribute_table_node.context).to eq(range_variable: false, column_reference: true) + end + + it 'works for INSERT INTO table' do + sql = 'INSERT INTO posts (public) VALUES (true)' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + insert_table_node = tree.child_at_path(%w[ast relation]) + + expect(insert_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for UPDATE table' do + sql = 'UPDATE posts SET public = TRUE' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + update_table_node = tree.child_at_path(%w[ast relation]) + + expect(update_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for the UPDATE FROM tables' do + sql = 'UPDATE posts SET public = TRUE FROM users, comments' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + users_table_node = tree.child_at_path(['ast', 'froms', 0]) + comments_table_node = tree.child_at_path(['ast', 'froms', 1]) + + expect(users_table_node.context).to eq(range_variable: true, column_reference: false) + expect(comments_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for DELETE table' do + sql = 'DELETE FROM posts' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + delete_table_node = tree.child_at_path(%w[ast relation]) + + expect(delete_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for DELETE USING tables' do + sql = 'DELETE FROM posts USING users, comments' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + users_table_node = tree.child_at_path(['ast', 'using', 0]) + comments_table_node = tree.child_at_path(['ast', 'using', 1]) + + expect(users_table_node.context).to eq(range_variable: true, column_reference: false) + expect(comments_table_node.context).to eq(range_variable: true, column_reference: false) + end + + it 'works for a CTE table' do + sql = 'WITH posts AS (SELECT 1) SELECT * FROM posts' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + cte_node = tree.child_at_path(['ast', 'with', 'expr', 0, 'left']) + + expect(cte_node.context).to eq(range_variable: false, column_reference: false, alias: true) + end + + it 'works for a recursive CTE table' do + sql = 'WITH RECURSIVE posts AS (SELECT 1) SELECT * FROM posts' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + cte_node = tree.child_at_path(['ast', 'with', 'expr', 0, 'left']) + + expect(cte_node.context).to eq(range_variable: false, column_reference: false, alias: true) + end + + it 'works for a SELECT INTO table' do + sql = 'SELECT INTO public_posts FROM posts WHERE posts.public = TRUE' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + into_table_node = tree.child_at_path(['ast', 'cores', 0, 'into', 'expr']) + + expect(into_table_node.context) + .to eq(range_variable: false, column_reference: false, alias: true) + end + + it 'raises an error for an arel table in an unknown location' do + sql = 'SELECT 1' + tree = Arel.transformer(Arel.sql_to_arel(sql).first) + limit_node = tree.child_at_path(%w[ast limit]) + + expect do + limit_node.replace(Arel::Table.new('strange_table')) + end.to raise_error do |error| + expect(error.message) + .to include("Unknown AST location for table only = - schema_name = - relpersistence = - > right = - > projections = - 1 = - > wheres = - name = - > > right = - > @@ -99,29 +99,41 @@ > > windows = - into = - > > limit = - orders = - offset = - + lock = + + with = + union = - + values_lists = + > diff --git a/spec/pg_search_spec.rb b/spec/pg_search_spec.rb index bc18f5cb..76ed2c7c 100644 --- a/spec/pg_search_spec.rb +++ b/spec/pg_search_spec.rb @@ -57,7 +57,7 @@ class Salami < ActiveRecord::Base pg_search_scope :tasty_search, associated_against: { cheeses: %i[kind brand], - cracker: :kind + cracker: :kind, } end diff --git a/spec/support/dummy/config/environments/development.rb b/spec/support/dummy/config/environments/development.rb index 7b5ddb21..e4653ee6 100644 --- a/spec/support/dummy/config/environments/development.rb +++ b/spec/support/dummy/config/environments/development.rb @@ -19,7 +19,7 @@ config.cache_store = :memory_store config.public_file_server.headers = { - 'Cache-Control' => "public, max-age=#{2.days.to_i}" + 'Cache-Control' => "public, max-age=#{2.days.to_i}", } else config.action_controller.perform_caching = false diff --git a/spec/support/dummy/config/environments/test.rb b/spec/support/dummy/config/environments/test.rb index bf3aeb3f..b2bff210 100644 --- a/spec/support/dummy/config/environments/test.rb +++ b/spec/support/dummy/config/environments/test.rb @@ -15,7 +15,7 @@ # Configure public file server for tests with Cache-Control for performance. config.public_file_server.enabled = true config.public_file_server.headers = { - 'Cache-Control' => "public, max-age=#{1.hour.to_i}" + 'Cache-Control' => "public, max-age=#{1.hour.to_i}", } # Show full error reports and disable caching.