From dbc102e0da4d1d2af0a4bae22d2ab78d494e1032 Mon Sep 17 00:00:00 2001 From: Carlos Silva <carlinhus.fsilva@gmail.com> Date: Thu, 6 Sep 2018 00:06:53 -0300 Subject: [PATCH 1/4] Move from USING to ON because of Join complexity --- lib/torque/postgresql/arel.rb | 1 - lib/torque/postgresql/arel/select_manager.rb | 6 ------ lib/torque/postgresql/arel/using.rb | 10 ---------- lib/torque/postgresql/arel/visitors.rb | 7 ------- lib/torque/postgresql/relation/inheritance.rb | 4 ++-- spec/tests/auxiliary_statement_spec.rb | 1 - spec/tests/table_inheritance_spec.rb | 10 +++++----- 7 files changed, 7 insertions(+), 32 deletions(-) delete mode 100644 lib/torque/postgresql/arel/using.rb diff --git a/lib/torque/postgresql/arel.rb b/lib/torque/postgresql/arel.rb index 0a90719..8ba2352 100644 --- a/lib/torque/postgresql/arel.rb +++ b/lib/torque/postgresql/arel.rb @@ -1,4 +1,3 @@ require_relative 'arel/join_source' require_relative 'arel/select_manager' require_relative 'arel/visitors' -require_relative 'arel/using' diff --git a/lib/torque/postgresql/arel/select_manager.rb b/lib/torque/postgresql/arel/select_manager.rb index 0ee50c8..895d36f 100644 --- a/lib/torque/postgresql/arel/select_manager.rb +++ b/lib/torque/postgresql/arel/select_manager.rb @@ -3,12 +3,6 @@ module Torque module Arel module SelectManager - def using column - column = ::Arel::Nodes::SqlLiteral.new(column.to_s) - @ctx.source.right.last.right = Using.new(column) - self - end - def only @ctx.source.only = true end diff --git a/lib/torque/postgresql/arel/using.rb b/lib/torque/postgresql/arel/using.rb deleted file mode 100644 index 800e194..0000000 --- a/lib/torque/postgresql/arel/using.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Torque - module PostgreSQL - module Arel - class Using < ::Arel::Nodes::Unary - end - - ::Arel::Nodes::Using = Using - end - end -end diff --git a/lib/torque/postgresql/arel/visitors.rb b/lib/torque/postgresql/arel/visitors.rb index 69cc661..9a52aba 100644 --- a/lib/torque/postgresql/arel/visitors.rb +++ b/lib/torque/postgresql/arel/visitors.rb @@ -16,13 +16,6 @@ module Torque super end - # Add USING modifier to query - def visit_Torque_PostgreSQL_Arel_Using(o, collector) - collector << 'USING ( ' - collector << o.expr - collector << ' )' - end - end ::Arel::Visitors::PostgreSQL.include Visitors diff --git a/lib/torque/postgresql/relation/inheritance.rb b/lib/torque/postgresql/relation/inheritance.rb index 12f5ddc..ae32346 100644 --- a/lib/torque/postgresql/relation/inheritance.rb +++ b/lib/torque/postgresql/relation/inheritance.rb @@ -79,13 +79,13 @@ module Torque # Build as many left outer join as necessary for each dependent table def build_inheritances_joins(arel, types) columns = Hash.new{ |h, k| h[k] = [] } - primary_key = quoted_primary_key + base_on_key = model.arel_table[primary_key] base_attributes = model.attribute_names # Iterate over each casted dependent calculating the columns types.each.with_index do |model, idx| join_table = model.arel_table.alias("\"i_#{idx}\"") - arel.outer_join(join_table).using(primary_key) + arel.outer_join(join_table).on(base_on_key.eq(join_table[primary_key])) (model.attribute_names - base_attributes).each do |column| columns[column] << join_table end diff --git a/spec/tests/auxiliary_statement_spec.rb b/spec/tests/auxiliary_statement_spec.rb index 03176b7..57bf984 100644 --- a/spec/tests/auxiliary_statement_spec.rb +++ b/spec/tests/auxiliary_statement_spec.rb @@ -469,5 +469,4 @@ RSpec.describe 'AuxiliaryStatement' do expect{ subject.with(:comments).arel.to_sql }.to raise_error(StandardError) end end - end diff --git a/spec/tests/table_inheritance_spec.rb b/spec/tests/table_inheritance_spec.rb index 8bda318..fc0d3e0 100644 --- a/spec/tests/table_inheritance_spec.rb +++ b/spec/tests/table_inheritance_spec.rb @@ -284,9 +284,9 @@ RSpec.describe 'TableInheritance' do result << ", \"record_class\".\"_record_class\" IN ('activity_books', 'activity_posts', 'activity_post_samples') AS _auto_cast" result << ' FROM "activities"' result << ' INNER JOIN "record_class" ON "activities"."tableoid" = "record_class"."oid"' - result << ' LEFT OUTER JOIN "activity_books" "i_0" USING ( "id" )' - result << ' LEFT OUTER JOIN "activity_posts" "i_1" USING ( "id" )' - result << ' LEFT OUTER JOIN "activity_post_samples" "i_2" USING ( "id" )' + result << ' LEFT OUTER JOIN "activity_books" "i_0" ON "activities"."id" = "i_0"."id"' + result << ' LEFT OUTER JOIN "activity_posts" "i_1" ON "activities"."id" = "i_1"."id"' + result << ' LEFT OUTER JOIN "activity_post_samples" "i_2" ON "activities"."id" = "i_2"."id"' expect(base.cast_records.all.to_sql).to eql(result) end @@ -297,7 +297,7 @@ RSpec.describe 'TableInheritance' do result << ", \"record_class\".\"_record_class\" IN ('activity_books') AS _auto_cast" result << ' FROM "activities"' result << ' INNER JOIN "record_class" ON "activities"."tableoid" = "record_class"."oid"' - result << ' LEFT OUTER JOIN "activity_books" "i_0" USING ( "id" )' + result << ' LEFT OUTER JOIN "activity_books" "i_0" ON "activities"."id" = "i_0"."id"' expect(base.cast_records(child).all.to_sql).to eql(result) end @@ -308,7 +308,7 @@ RSpec.describe 'TableInheritance' do result << ", \"record_class\".\"_record_class\" IN ('activity_books') AS _auto_cast" result << ' FROM "activities"' result << ' INNER JOIN "record_class" ON "activities"."tableoid" = "record_class"."oid"' - result << ' LEFT OUTER JOIN "activity_books" "i_0" USING ( "id" )' + result << ' LEFT OUTER JOIN "activity_books" "i_0" ON "activities"."id" = "i_0"."id"' result << " WHERE \"activities\".\"_record_class\" = 'activity_books'" expect(base.cast_records(child, filter: true).all.to_sql).to eql(result) end -- GitLab From a1f15ad740335fcaee8fa178b2618cfa05658067 Mon Sep 17 00:00:00 2001 From: Carlos Silva <carlinhus.fsilva@gmail.com> Date: Fri, 7 Sep 2018 11:28:59 -0300 Subject: [PATCH 2/4] Add Enumerable to enums class Closes #16 --- lib/torque/postgresql/attributes/enum.rb | 9 +++++- spec/tests/enum_spec.rb | 38 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/torque/postgresql/attributes/enum.rb b/lib/torque/postgresql/attributes/enum.rb index f309ad3..0ae0d79 100644 --- a/lib/torque/postgresql/attributes/enum.rb +++ b/lib/torque/postgresql/attributes/enum.rb @@ -9,7 +9,9 @@ module Torque LAZY_VALUE = 0.chr class << self - delegate :each, :sample, to: :values + include Enumerable + + delegate :each, :sample, to: :members # Find or create the class that will handle the value def lookup(name) @@ -40,6 +42,11 @@ module Torque end end + # Different from values, it returns the list of items already casted + def members + values.dup.map(&method(:new)) + end + # Fetch a value from the list # see https://github.com/rails/rails/blob/v5.0.0/activerecord/lib/active_record/fixtures.rb#L656 # see https://github.com/rails/rails/blob/v5.0.0/activerecord/lib/active_record/validations/uniqueness.rb#L101 diff --git a/spec/tests/enum_spec.rb b/spec/tests/enum_spec.rb index fd93595..7c35c3d 100644 --- a/spec/tests/enum_spec.rb +++ b/spec/tests/enum_spec.rb @@ -276,6 +276,44 @@ RSpec.describe 'Enum' do expect(mock_value.to_i).to_not be_eql(15) expect(mock_value.to_i).to be_eql(4) end + + context 'on members' do + it 'has enumerable operations' do + expect(subject).to respond_to(:all?) + expect(subject).to respond_to(:any?) + expect(subject).to respond_to(:collect) + expect(subject).to respond_to(:count) + expect(subject).to respond_to(:cycle) + expect(subject).to respond_to(:detect) + expect(subject).to respond_to(:drop) + expect(subject).to respond_to(:drop_while) + expect(subject).to respond_to(:each) + expect(subject).to respond_to(:each_with_index) + expect(subject).to respond_to(:entries) + expect(subject).to respond_to(:find) + expect(subject).to respond_to(:find_all) + expect(subject).to respond_to(:find_index) + expect(subject).to respond_to(:first) + expect(subject).to respond_to(:flat_map) + expect(subject).to respond_to(:include?) + expect(subject).to respond_to(:inject) + expect(subject).to respond_to(:lazy) + expect(subject).to respond_to(:map) + expect(subject).to respond_to(:member?) + expect(subject).to respond_to(:one?) + expect(subject).to respond_to(:reduce) + expect(subject).to respond_to(:reject) + expect(subject).to respond_to(:reverse_each) + expect(subject).to respond_to(:select) + expect(subject).to respond_to(:sort) + expect(subject).to respond_to(:zip) + end + + it 'works with map' do + result = subject.map(&:to_i) + expect(result).to be_eql([0, 1, 2, 3]) + end + end end context 'on OID' do -- GitLab From 4c9b7023a2cc436dd1c46515b64ee35a6957ec5c Mon Sep 17 00:00:00 2001 From: Carlos Silva <carlinhus.fsilva@gmail.com> Date: Fri, 7 Sep 2018 15:16:53 -0300 Subject: [PATCH 3/4] Fix issues associated with calculated operations on relations Closes #17 --- lib/torque/postgresql/relation.rb | 27 +++++++++++++++-- .../relation/auxiliary_statement.rb | 2 +- lib/torque/postgresql/relation/inheritance.rb | 2 +- lib/torque/postgresql/relation/merger.rb | 6 ++++ spec/mocks/cache_query.rb | 16 ++++++++++ spec/spec_helper.rb | 1 + spec/tests/auxiliary_statement_spec.rb | 30 +++++++++++++++++++ spec/tests/table_inheritance_spec.rb | 24 +++++++++++++++ 8 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 spec/mocks/cache_query.rb diff --git a/lib/torque/postgresql/relation.rb b/lib/torque/postgresql/relation.rb index c0e3a0a..5dc6ecb 100644 --- a/lib/torque/postgresql/relation.rb +++ b/lib/torque/postgresql/relation.rb @@ -14,9 +14,22 @@ module Torque include Inheritance SINGLE_VALUE_METHODS = [:itself_only] - MULTI_VALUE_METHODS = [:distinct_on, :auxiliary_statements, :cast_records] + MULTI_VALUE_METHODS = [:distinct_on, :auxiliary_statements, :cast_records, + :dynamic_selection] VALUE_METHODS = SINGLE_VALUE_METHODS + MULTI_VALUE_METHODS + # :nodoc: + def dynamic_selection_values; get_value(:dynamic_selection); end + # :nodoc: + def dynamic_selection_values=(value); set_value(:dynamic_selection, value); end + + # Resolve column name when calculating models, allowing the column name to + # be more complex while keeping the query selection quality + def calculate(operation, column_name) + column_name = resolve_column(column_name).first if column_name.is_a?(Hash) + super(operation, column_name) + end + # Resolve column definition up to second value. # For example, based on Post model: # @@ -65,6 +78,16 @@ module Torque private + def dynamic_selection + @dynamic_selection ||= [] + end + + def build_arel + arel = super + arel.project(*dynamic_selection) if select_values.blank? && dynamic_selection.any? + arel + end + # Compatibility method with 5.0 unless ActiveRecord::Relation.method_defined?(:get_value) def get_value(name) @@ -108,7 +131,7 @@ module Torque ActiveRecord::Relation::MULTI_VALUE_METHODS += Relation::MULTI_VALUE_METHODS ActiveRecord::Relation::VALUE_METHODS += Relation::VALUE_METHODS ActiveRecord::QueryMethods::VALID_UNSCOPING_VALUES += [:cast_records, :itself_only, - :distinct_on, :auxiliary_statements] + :distinct_on, :auxiliary_statements, :dynamic_selection] if ActiveRecord::QueryMethods.const_defined?('DEFAULT_VALUES') Relation::SINGLE_VALUE_METHODS.each do |value| diff --git a/lib/torque/postgresql/relation/auxiliary_statement.rb b/lib/torque/postgresql/relation/auxiliary_statement.rb index 1c4b8be..23bb795 100644 --- a/lib/torque/postgresql/relation/auxiliary_statement.rb +++ b/lib/torque/postgresql/relation/auxiliary_statement.rb @@ -59,7 +59,7 @@ module Torque columns.flatten! arel.with(subqueries.flatten) - arel.project(*columns) if columns.any? + dynamic_selection.concat(columns) if columns.any? end # Throw an error showing that an auxiliary statement of the given diff --git a/lib/torque/postgresql/relation/inheritance.rb b/lib/torque/postgresql/relation/inheritance.rb index ae32346..4bb4dd6 100644 --- a/lib/torque/postgresql/relation/inheritance.rb +++ b/lib/torque/postgresql/relation/inheritance.rb @@ -73,7 +73,7 @@ module Torque end columns.push(build_auto_caster_marker(arel, self.cast_records_value)) - arel.project(*columns) if columns.any? + dynamic_selection.concat(columns) if columns.any? end # Build as many left outer join as necessary for each dependent table diff --git a/lib/torque/postgresql/relation/merger.rb b/lib/torque/postgresql/relation/merger.rb index d749661..24a8605 100644 --- a/lib/torque/postgresql/relation/merger.rb +++ b/lib/torque/postgresql/relation/merger.rb @@ -6,6 +6,7 @@ module Torque def merge # :nodoc: super + merge_dynamic_selection merge_distinct_on merge_auxiliary_statements merge_inheritance @@ -15,6 +16,11 @@ module Torque private + # Merge dynamic selection columns + def merge_dynamic_selection + relation.dynamic_selection_values.concat(other.dynamic_selection_values) + end + # Merge distinct on columns def merge_distinct_on return if other.distinct_on_values.blank? diff --git a/spec/mocks/cache_query.rb b/spec/mocks/cache_query.rb new file mode 100644 index 0000000..9cc0d90 --- /dev/null +++ b/spec/mocks/cache_query.rb @@ -0,0 +1,16 @@ +module Mocks + module CacheQuery + def get_last_executed_query(&block) + conn = ActiveRecord::Base.connection + conn.instance_variable_set(:@query_cache_enabled, true) + + block.call + result = conn.query_cache.keys.first + + conn.instance_variable_set(:@query_cache_enabled, false) + conn.instance_variable_get(:@query_cache).delete(result) + + result + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a2f6ae3..a3dae27 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,7 @@ I18n.load_path << Pathname.pwd.join('spec', 'en.yml') load File.join('schema.rb') RSpec.configure do |config| config.extend Mocks::CreateTable + config.include Mocks::CacheQuery config.formatter = :documentation config.color = true diff --git a/spec/tests/auxiliary_statement_spec.rb b/spec/tests/auxiliary_statement_spec.rb index 57bf984..96d1c90 100644 --- a/spec/tests/auxiliary_statement_spec.rb +++ b/spec/tests/auxiliary_statement_spec.rb @@ -392,6 +392,36 @@ RSpec.describe 'AuxiliaryStatement' do Comment.columns_hash.delete('source_id') end + it 'works with count and does not add extra columns' do + klass.send(:auxiliary_statement, :comments) do |cte| + cte.query Comment.all + cte.attributes content: :comment_content + end + + result = 'WITH "comments" AS' + result << ' (SELECT "comments"."content" AS comment_content, "comments"."user_id" FROM "comments")' + result << ' SELECT COUNT(*) FROM "users"' + result << ' INNER JOIN "comments" ON "users"."id" = "comments"."user_id"' + + query = get_last_executed_query{ subject.with(:comments).count } + expect(query).to eql(result) + end + + it 'works with sum and does not add extra columns' do + klass.send(:auxiliary_statement, :comments) do |cte| + cte.query Comment.all + cte.attributes id: :value + end + + result = 'WITH "comments" AS' + result << ' (SELECT "comments"."id" AS value, "comments"."user_id" FROM "comments")' + result << ' SELECT SUM("comments"."value") FROM "users"' + result << ' INNER JOIN "comments" ON "users"."id" = "comments"."user_id"' + + query = get_last_executed_query{ subject.with(:comments).sum(comments: :value) } + expect(query).to eql(result) + end + it 'raises an error when using an invalid type of object as query' do klass.send(:auxiliary_statement, :comments) do |cte| cte.query :string, String diff --git a/spec/tests/table_inheritance_spec.rb b/spec/tests/table_inheritance_spec.rb index fc0d3e0..0478c2a 100644 --- a/spec/tests/table_inheritance_spec.rb +++ b/spec/tests/table_inheritance_spec.rb @@ -313,6 +313,30 @@ RSpec.describe 'TableInheritance' do expect(base.cast_records(child, filter: true).all.to_sql).to eql(result) end + it 'works with count and does not add extra columns' do + result = 'WITH "record_class" AS (SELECT "pg_class"."oid", "pg_class"."relname" AS _record_class FROM "pg_class")' + result << ' SELECT COUNT(*)' + result << ' FROM "activities"' + result << ' INNER JOIN "record_class" ON "activities"."tableoid" = "record_class"."oid"' + result << ' LEFT OUTER JOIN "activity_books" "i_0" ON "activities"."id" = "i_0"."id"' + result << ' LEFT OUTER JOIN "activity_posts" "i_1" ON "activities"."id" = "i_1"."id"' + result << ' LEFT OUTER JOIN "activity_post_samples" "i_2" ON "activities"."id" = "i_2"."id"' + query = get_last_executed_query{ base.cast_records.all.count } + expect(query).to eql(result) + end + + it 'works with sum and does not add extra columns' do + result = 'WITH "record_class" AS (SELECT "pg_class"."oid", "pg_class"."relname" AS _record_class FROM "pg_class")' + result << ' SELECT SUM("activities"."id")' + result << ' FROM "activities"' + result << ' INNER JOIN "record_class" ON "activities"."tableoid" = "record_class"."oid"' + result << ' LEFT OUTER JOIN "activity_books" "i_0" ON "activities"."id" = "i_0"."id"' + result << ' LEFT OUTER JOIN "activity_posts" "i_1" ON "activities"."id" = "i_1"."id"' + result << ' LEFT OUTER JOIN "activity_post_samples" "i_2" ON "activities"."id" = "i_2"."id"' + query = get_last_executed_query{ base.cast_records.all.sum(:id) } + expect(query).to eql(result) + end + it 'returns the correct model object' do ActivityPost.create(title: 'Activity post') ActivityPost::Sample.create(title: 'Activity post') -- GitLab From d7f567c49c8681c83b3699397fbb2c7781b97b38 Mon Sep 17 00:00:00 2001 From: Carlos Silva <carlinhus.fsilva@gmail.com> Date: Fri, 7 Sep 2018 15:26:01 -0300 Subject: [PATCH 4/4] v0.2.6 --- lib/torque/postgresql/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/torque/postgresql/version.rb b/lib/torque/postgresql/version.rb index 84ca902..fa5fe05 100644 --- a/lib/torque/postgresql/version.rb +++ b/lib/torque/postgresql/version.rb @@ -1,5 +1,5 @@ module Torque module PostgreSQL - VERSION = '0.2.5' + VERSION = '0.2.6' end end -- GitLab