Skip to content

Commit

Permalink
wip: remove nil queries from union_of
Browse files Browse the repository at this point in the history
  • Loading branch information
ezekg committed Jan 13, 2024
1 parent 0bb327c commit f21756c
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 18 deletions.
14 changes: 8 additions & 6 deletions lib/union_of.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,16 @@ def last_chain_scope(scope, reflection, owner)
reflection = association.reflection
primary_key = reflection.active_record_primary_key

# FIXME(ezekg) Should we use Arel here instead of this private API?
association.send(:association_scope)
.select(primary_key)
.unscope(:order)
.arel
# FIXME(ezekg) Find an alternative to this private API.
next nil if
reflection.belongs_to? && !association.send(:foreign_key_present?)

association.scope.select(primary_key)
.unscope(:order)
.arel
end

unions = sources.reduce(nil) do |left, right|
unions = sources.compact.reduce(nil) do |left, right|
if left
Arel::Nodes::Union.new(left, right)
else
Expand Down
47 changes: 37 additions & 10 deletions spec/lib/union_of_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@
end

it 'should be a union' do
# FIXME(ezekg) The gsubs are to match the #to_sql format. Maybe create a SQL matcher?
expect(record.licenses.to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(record.licenses.to_sql).to match_sql <<~SQL.squish
SELECT
"licenses".*
FROM
Expand Down Expand Up @@ -74,7 +73,7 @@
end

it 'should produce a union join' do
expect(model.joins(:machines).to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(model.joins(:machines).to_sql).to match_sql <<~SQL.squish
SELECT
"users".*
FROM
Expand Down Expand Up @@ -111,7 +110,7 @@

it 'should produce a union query' do
# TODO(ezekg) Add DISTINCT?
expect(record.machines.to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(record.machines.to_sql).to match_sql <<~SQL.squish
SELECT
"machines".*
FROM
Expand Down Expand Up @@ -149,7 +148,7 @@
end

it 'should produce a deep union join' do
expect(model.joins(:components).to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(model.joins(:components).to_sql).to match_sql <<~SQL.squish
SELECT
"users".*
FROM
Expand Down Expand Up @@ -187,7 +186,7 @@

it 'should produce a deep union query' do
# TODO(ezekg) Add DISTINCT?
expect(record.components.to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(record.components.to_sql).to match_sql <<~SQL.squish
SELECT
"machine_components".*
FROM
Expand Down Expand Up @@ -226,7 +225,7 @@
end

it 'should produce a deeper union join' do
expect(Product.joins(:users).to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2')
expect(Product.joins(:users).to_sql).to match_sql <<~SQL.squish
SELECT
"products".*
FROM
Expand Down Expand Up @@ -265,9 +264,7 @@
it 'should produce a deeper union query' do
product = create(:product, account:)

# FIXME(ezekg) The additional gsubs are because #to_sql will format the SQL
# "( ( ( SELECT" to "(( (SELECT"
expect(product.users.to_sql).to eq <<~SQL.squish.gsub(/(\()\s*([\w'"])/, '\1\2').gsub(/([\w"'])\s*(\))/, '\1\2').gsub(/\s+(\()\s+(\()\s+/, ' \1\2 ').gsub(/\s+(\))\s+(\))\s+/, ' \1\2 ')
expect(product.users.to_sql).to match_sql <<~SQL.squish
SELECT
DISTINCT "users".*
FROM
Expand Down Expand Up @@ -301,6 +298,36 @@
SQL
end

context 'with nil belongs_to association' do
let(:record) { create(:license, account:) }

it 'should not produce a union query' do
expect(record.users.to_sql).to match_sql <<~SQL.squish
SELECT
"users".*
FROM
"users"
WHERE
"users"."id" IN (
SELECT
"users"."id"
FROM
(
SELECT
"users"."id"
FROM
"users"
INNER JOIN "license_users" ON "users"."id" = "license_users"."user_id"
WHERE
"license_users"."license_id" = '#{record.id}'
) "users"
)
ORDER BY
"users"."created_at" ASC
SQL
end
end

# TODO(ezekg) Add exhaustive tests for all association macros, e.g.
# belongs_to, has_many, etc.
end
2 changes: 1 addition & 1 deletion spec/support/matchers/deep_include_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def deep_include?(actual, expected, path = [])
Actual array did not include value at #{path}:
expected:
#{@failing_expected_array_item.inspect}
but matching value not found in array:
got:
#{@failing_array.inspect}
MSG
else
Expand Down
2 changes: 1 addition & 1 deletion spec/support/matchers/log_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
Expected block to output matching log messages to the following log levels: #{@levels.inspect}.
expected:
#{@expected.inspect}
actual:
got:
#{@messages.join.inspect}
MSG
end
Expand Down
22 changes: 22 additions & 0 deletions spec/support/matchers/sql_matcher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

RSpec::Matchers.define :match_sql do |expected|
match do |actual|
# Make our SQL string match #to_sql. This mostly deals with formatting parentheses.
actual == expected.squish
.gsub(/(\()\s*([\w'"])/, '\1\2') # `( "table"."column"` => `("table"."column"`
.gsub(/([\w"'])\s*(\))/, '\1\2') # `"table"."column" = 'value')` => `"table"."column" = 'value')`
.gsub(/\s+(\()\s+(\()\s+/, ' \1\2 ') # ` ( ( ` => ` (( `
.gsub(/\s+(\))\s+(\))\s+/, ' \1\2 ') # ` ) ) ` => ` )) `
end

failure_message do |actual|
<<~MSG
Expected SQL to match:
expected:
#{expected}
got:
#{actual}
MSG
end
end

0 comments on commit f21756c

Please sign in to comment.