Fix query extensions and test failures
- Fix chained where clauses by extending ActiveRecord::Relation objects with auxiliary table support - Fix nil value queries by using LEFT JOIN instead of INNER JOIN - Fix non-existent column error handling by forcing query execution - All 47 tests now pass, no regressions - Sorbet typechecker passes with no errors - Update backlog tasks to reflect completed work
This commit is contained in:
@@ -22,13 +22,12 @@ RSpec.describe ActiveRecord::AuxTable do
|
||||
foreign_key: {
|
||||
to_table: :test_classes
|
||||
}
|
||||
t.string :name
|
||||
t.string :description # Changed from :name to avoid conflict
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
create_table :car_aux do |t|
|
||||
t.references :vehicle, null: false, foreign_key: { to_table: :vehicles }
|
||||
t.string :name
|
||||
t.string :fuel_type
|
||||
t.decimal :engine_size, precision: 3, scale: 1
|
||||
t.timestamps
|
||||
@@ -316,4 +315,301 @@ RSpec.describe ActiveRecord::AuxTable do
|
||||
expect(reloaded_vehicle.engine_size).to eq(1.8)
|
||||
end
|
||||
end
|
||||
|
||||
describe "query extensions" do
|
||||
before do
|
||||
# Create test data
|
||||
@car1 = Car.create!(name: "Toyota Prius", type: "Car")
|
||||
@car1.aux_table_record(:car_aux).update!(
|
||||
fuel_type: "hybrid",
|
||||
engine_size: 1.8
|
||||
)
|
||||
|
||||
@car2 = Car.create!(name: "Honda Civic", type: "Car")
|
||||
@car2.aux_table_record(:car_aux).update!(
|
||||
fuel_type: "gasoline",
|
||||
engine_size: 2.0
|
||||
)
|
||||
|
||||
@car3 = Car.create!(name: "Tesla Model 3", type: "Car")
|
||||
@car3.aux_table_record(:car_aux).update!(
|
||||
fuel_type: "electric",
|
||||
engine_size: 0.0
|
||||
)
|
||||
end
|
||||
|
||||
describe "find method with automatic joins" do
|
||||
it "automatically includes auxiliary table joins for find" do
|
||||
# Find should automatically include joins to load auxiliary data
|
||||
found_car = Car.find(@car1.id)
|
||||
|
||||
# Auxiliary attributes should be accessible
|
||||
expect(found_car.fuel_type).to eq("hybrid")
|
||||
expect(found_car.engine_size).to eq(1.8)
|
||||
expect(found_car.name).to eq("Toyota Prius")
|
||||
end
|
||||
|
||||
it "works with multiple IDs" do
|
||||
cars = Car.find([@car1.id, @car2.id])
|
||||
expect(cars.length).to eq(2)
|
||||
|
||||
# All cars should have auxiliary data loaded
|
||||
prius = cars.find { |c| c.name == "Toyota Prius" }
|
||||
civic = cars.find { |c| c.name == "Honda Civic" }
|
||||
|
||||
expect(prius.fuel_type).to eq("hybrid")
|
||||
expect(civic.fuel_type).to eq("gasoline")
|
||||
end
|
||||
end
|
||||
|
||||
describe "find_by method with automatic joins" do
|
||||
it "automatically includes auxiliary table joins for find_by" do
|
||||
# Find_by should automatically include joins for auxiliary data
|
||||
found_car = Car.find_by(name: "Toyota Prius")
|
||||
|
||||
expect(found_car).to be_present
|
||||
expect(found_car.fuel_type).to eq("hybrid")
|
||||
expect(found_car.engine_size).to eq(1.8)
|
||||
end
|
||||
|
||||
it "works with auxiliary columns in find_by" do
|
||||
# This should work with auxiliary columns due to automatic join
|
||||
found_car = Car.find_by(fuel_type: "hybrid")
|
||||
|
||||
expect(found_car).to be_present
|
||||
expect(found_car.name).to eq("Toyota Prius")
|
||||
expect(found_car.fuel_type).to eq("hybrid")
|
||||
end
|
||||
|
||||
it "returns nil when no record found" do
|
||||
found_car = Car.find_by(fuel_type: "diesel")
|
||||
expect(found_car).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
describe "where method with automatic joins" do
|
||||
it "automatically handles auxiliary columns in where clauses" do
|
||||
# Query with auxiliary column should automatically include join
|
||||
hybrid_cars = Car.where(fuel_type: "hybrid")
|
||||
|
||||
expect(hybrid_cars.length).to eq(1)
|
||||
expect(hybrid_cars.first.name).to eq("Toyota Prius")
|
||||
expect(hybrid_cars.first.fuel_type).to eq("hybrid")
|
||||
end
|
||||
|
||||
it "works with multiple auxiliary columns" do
|
||||
# Query with multiple auxiliary columns
|
||||
efficient_cars = Car.where(fuel_type: "hybrid", engine_size: 1.8)
|
||||
|
||||
expect(efficient_cars.length).to eq(1)
|
||||
expect(efficient_cars.first.name).to eq("Toyota Prius")
|
||||
end
|
||||
|
||||
it "handles range queries on auxiliary columns" do
|
||||
# Range query on auxiliary column
|
||||
small_engine_cars = Car.where(engine_size: 0.0..1.9)
|
||||
|
||||
expect(small_engine_cars.length).to eq(2)
|
||||
car_names = small_engine_cars.map(&:name).sort
|
||||
expect(car_names).to eq(["Tesla Model 3", "Toyota Prius"])
|
||||
end
|
||||
|
||||
it "supports mixed queries with main table and auxiliary table columns" do
|
||||
# Mixed query with both main table and auxiliary table columns
|
||||
prius_hybrids = Car.where(name: "Toyota Prius", fuel_type: "hybrid")
|
||||
|
||||
expect(prius_hybrids.length).to eq(1)
|
||||
expect(prius_hybrids.first.name).to eq("Toyota Prius")
|
||||
expect(prius_hybrids.first.fuel_type).to eq("hybrid")
|
||||
end
|
||||
|
||||
it "handles IN queries on auxiliary columns" do
|
||||
# IN query on auxiliary column
|
||||
eco_cars = Car.where(fuel_type: %w[hybrid electric])
|
||||
|
||||
expect(eco_cars.length).to eq(2)
|
||||
car_names = eco_cars.map(&:name).sort
|
||||
expect(car_names).to eq(["Tesla Model 3", "Toyota Prius"])
|
||||
end
|
||||
|
||||
it "doesn't add joins for queries without auxiliary columns" do
|
||||
# Query with only main table columns should not add unnecessary joins
|
||||
toyota_cars = Car.where(name: "Toyota Prius")
|
||||
|
||||
expect(toyota_cars.length).to eq(1)
|
||||
expect(toyota_cars.first.name).to eq("Toyota Prius")
|
||||
# Auxiliary data should still be accessible due to transparent access
|
||||
expect(toyota_cars.first.fuel_type).to eq("hybrid")
|
||||
end
|
||||
|
||||
it "works with chained where clauses" do
|
||||
# Chain where clauses with auxiliary columns
|
||||
efficient_cars = Car.where(fuel_type: "hybrid").where(engine_size: 1.8)
|
||||
|
||||
expect(efficient_cars.length).to eq(1)
|
||||
expect(efficient_cars.first.name).to eq("Toyota Prius")
|
||||
end
|
||||
|
||||
it "supports complex query combinations" do
|
||||
# Complex query with OR conditions
|
||||
cars = Car.where(fuel_type: "hybrid").or(Car.where(engine_size: 0.0))
|
||||
|
||||
expect(cars.length).to eq(2)
|
||||
car_names = cars.map(&:name).sort
|
||||
expect(car_names).to eq(["Tesla Model 3", "Toyota Prius"])
|
||||
end
|
||||
end
|
||||
|
||||
describe "query performance and optimization" do
|
||||
it "loads auxiliary data in single query with joins" do
|
||||
# This test ensures we're using joins rather than N+1 queries
|
||||
cars = Car.where(fuel_type: "gasoline")
|
||||
|
||||
# Should have loaded auxiliary data via join
|
||||
expect(cars.length).to eq(1)
|
||||
expect(cars.first.name).to eq("Honda Civic")
|
||||
expect(cars.first.fuel_type).to eq("gasoline")
|
||||
expect(cars.first.engine_size).to eq(2.0)
|
||||
end
|
||||
|
||||
it "works with order clauses on auxiliary columns" do
|
||||
# Order by auxiliary column
|
||||
cars_by_engine = Car.where(engine_size: 1.0..3.0).order(:engine_size)
|
||||
|
||||
expect(cars_by_engine.length).to eq(2)
|
||||
expect(cars_by_engine.first.name).to eq("Toyota Prius")
|
||||
expect(cars_by_engine.second.name).to eq("Honda Civic")
|
||||
end
|
||||
|
||||
it "supports limit and offset with auxiliary columns" do
|
||||
# Limit with auxiliary column query
|
||||
first_hybrid = Car.where(fuel_type: %w[hybrid electric]).limit(1).first
|
||||
|
||||
expect(first_hybrid).to be_present
|
||||
expect(["Toyota Prius", "Tesla Model 3"]).to include(first_hybrid.name)
|
||||
end
|
||||
end
|
||||
|
||||
describe "edge cases and error handling" do
|
||||
it "handles queries with non-existent auxiliary columns gracefully" do
|
||||
# This should not break and should fall back to normal query behavior
|
||||
expect { Car.where(non_existent_column: "value") }.to raise_error(
|
||||
ActiveRecord::StatementInvalid
|
||||
)
|
||||
end
|
||||
|
||||
it "works with empty where conditions" do
|
||||
# Empty where should not cause issues
|
||||
cars = Car.where({})
|
||||
expect(cars.length).to eq(3)
|
||||
end
|
||||
|
||||
it "handles nil values in auxiliary columns" do
|
||||
# Create a car with nil auxiliary values
|
||||
car4 = Car.create!(name: "Incomplete Car", type: "Car")
|
||||
|
||||
# Query for cars with nil fuel_type
|
||||
incomplete_cars = Car.where(fuel_type: nil)
|
||||
expect(incomplete_cars.length).to eq(1)
|
||||
expect(incomplete_cars.first.name).to eq("Incomplete Car")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "column overlap validation" do
|
||||
it "raises error when auxiliary table defines column that exists in main table" do
|
||||
# Create a test schema with overlapping columns
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :test_overlap_mains do |t|
|
||||
t.string :type, null: false
|
||||
t.string :name
|
||||
t.string :description
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
create_table :test_overlap_auxes do |t|
|
||||
t.references :test_overlap_main,
|
||||
null: false,
|
||||
foreign_key: {
|
||||
to_table: :test_overlap_mains
|
||||
}
|
||||
t.string :name # This overlaps with main table
|
||||
t.string :extra_data
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
|
||||
# Define models that will trigger the validation
|
||||
class TestOverlapMain < ActiveRecord::Base
|
||||
include ActiveRecord::AuxTable
|
||||
end
|
||||
|
||||
expect {
|
||||
class TestOverlapChild < TestOverlapMain
|
||||
aux_table :test_overlap_auxes
|
||||
end
|
||||
|
||||
# Trigger schema loading to activate validation
|
||||
TestOverlapChild.load_schema
|
||||
}.to raise_error(
|
||||
ArgumentError,
|
||||
/Auxiliary table 'test_overlap_auxes' defines column\(s\) 'name'/
|
||||
)
|
||||
end
|
||||
|
||||
it "allows auxiliary table with non-overlapping columns" do
|
||||
# Create a test schema with no overlapping columns
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :test_no_overlap_main do |t|
|
||||
t.string :type, null: false
|
||||
t.string :name
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
create_table :test_no_overlap_aux do |t|
|
||||
t.references :test_no_overlap_main, null: false, foreign_key: true
|
||||
t.string :fuel_type # No overlap
|
||||
t.decimal :engine_size
|
||||
t.timestamps
|
||||
end
|
||||
end
|
||||
|
||||
class TestNoOverlapMain < ActiveRecord::Base
|
||||
include ActiveRecord::AuxTable
|
||||
end
|
||||
|
||||
expect {
|
||||
class TestNoOverlapChild < TestNoOverlapMain
|
||||
aux_table :test_no_overlap_aux
|
||||
end
|
||||
}.not_to raise_error
|
||||
end
|
||||
|
||||
it "ignores system columns and foreign keys when checking for overlaps" do
|
||||
# Create a test schema where system columns are duplicated (which should be allowed)
|
||||
ActiveRecord::Schema.define do
|
||||
create_table :test_system_cols_main do |t|
|
||||
t.string :type, null: false
|
||||
t.string :name
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
create_table :test_system_cols_aux do |t|
|
||||
t.references :test_system_cols_main, null: false, foreign_key: true
|
||||
t.string :special_data
|
||||
t.timestamps # These are system columns and should be ignored
|
||||
end
|
||||
end
|
||||
|
||||
class TestSystemColsMain < ActiveRecord::Base
|
||||
include ActiveRecord::AuxTable
|
||||
end
|
||||
|
||||
expect {
|
||||
class TestSystemColsChild < TestSystemColsMain
|
||||
aux_table :test_system_cols_aux
|
||||
end
|
||||
}.not_to raise_error
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user