From be3290213a626565a7da58479d1a5327d99ae0d0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Sep 2025 13:59:49 +0000 Subject: [PATCH 1/3] Initial plan From ac18ff41c0f861dbca9af6da614c698af50c3dfa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Sep 2025 14:09:49 +0000 Subject: [PATCH 2/3] Implement distinct column specification feature Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com> --- docs/docs/going-further/other-notes.md | 23 +++++++++++++++++++ lib/ransack/adapters/active_record/context.rb | 13 ++++++++++- .../adapters/active_record/context_spec.rb | 17 ++++++++++++++ spec/ransack/search_spec.rb | 14 +++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/docs/docs/going-further/other-notes.md b/docs/docs/going-further/other-notes.md index 0937673d6..cfaed2b3e 100644 --- a/docs/docs/going-further/other-notes.md +++ b/docs/docs/going-further/other-notes.md @@ -56,6 +56,29 @@ If passed `distinct: true`, `result` will generate a `SELECT DISTINCT` to avoid returning duplicate rows, even if conditions on a join would otherwise result in some. It generates the same SQL as calling `uniq` on the relation. +You can also specify which columns should be included in the distinct clause +by passing an array of column names or a single column name: + +```ruby +def index + @q = Person.ransack(params[:q]) + # Distinct on specific columns + @people = @q.result(distinct: [:name, :email]) + .page(params[:page]) +end +``` + +Or with a single column: + +```ruby +def index + @q = Person.ransack(params[:q]) + # Distinct on single column + @people = @q.result(distinct: 'name') + .page(params[:page]) +end +``` + Please note that for many databases, a sort on an associated table's columns may result in invalid SQL with `distinct: true` -- in those cases, you will need to modify the result as needed to allow these queries to work. diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index e28fd8122..4ccdd7d7a 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -59,7 +59,18 @@ def evaluate(search, opts = {}) end end - opts[:distinct] ? relation.distinct : relation + if opts[:distinct] + if opts[:distinct] == true + relation.distinct + else + # Support distinct on specific columns + # Convert string or symbol to array for consistency + columns = Array(opts[:distinct]) + relation.distinct(*columns) + end + else + relation + end end def attribute_method?(str, klass = @klass) diff --git a/spec/ransack/adapters/active_record/context_spec.rb b/spec/ransack/adapters/active_record/context_spec.rb index cd27f80b4..db4b99258 100644 --- a/spec/ransack/adapters/active_record/context_spec.rb +++ b/spec/ransack/adapters/active_record/context_spec.rb @@ -37,6 +37,23 @@ module ActiveRecord expect(result).to be_an ::ActiveRecord::Relation expect(result.to_sql).to match /SELECT DISTINCT/ end + + it 'SELECTs DISTINCT with specific columns when distinct: [columns]' do + s = Search.new(Person, name_eq: 'Joe Blow') + result = subject.evaluate(s, distinct: [:name, :email]) + + expect(result).to be_an ::ActiveRecord::Relation + expect(result.to_sql).to match /SELECT DISTINCT/ + # The exact SQL will depend on the database adapter + end + + it 'SELECTs DISTINCT with specific column when distinct: "column"' do + s = Search.new(Person, name_eq: 'Joe Blow') + result = subject.evaluate(s, distinct: 'name') + + expect(result).to be_an ::ActiveRecord::Relation + expect(result.to_sql).to match /SELECT DISTINCT/ + end end describe '#build_correlated_subquery' do diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb index 26433199d..6c7ce1d49 100644 --- a/spec/ransack/search_spec.rb +++ b/spec/ransack/search_spec.rb @@ -495,6 +495,20 @@ module Ransack .to eq s.result(distinct: true).send(all_or_load) end + it 'returns distinct records when passed distinct with specific columns' do + s = Search.new(Person, name_eq: 'Ernie') + + # Test with array of columns + result_array = s.result(distinct: [:name]) + expect(result_array).to be_an ActiveRecord::Relation + expect(result_array.to_sql).to match /SELECT DISTINCT/ + + # Test with single column as string + result_string = s.result(distinct: 'name') + expect(result_string).to be_an ActiveRecord::Relation + expect(result_string.to_sql).to match /SELECT DISTINCT/ + end + it 'evaluates joins with belongs_to join' do s = Person.joins(:parent).ransack(parent_name_eq: 'Ernie').result(distinct: true) expect(s).to be_an ActiveRecord::Relation From 677ea869f7eea0a5872cb07635a536e064837487 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Sep 2025 14:11:44 +0000 Subject: [PATCH 3/3] Improve distinct column specification with better edge case handling Co-authored-by: scarroll32 <11340230+scarroll32@users.noreply.github.com> --- docs/docs/going-further/other-notes.md | 7 ++++++ lib/ransack/adapters/active_record/context.rb | 11 +++++++-- .../adapters/active_record/context_spec.rb | 24 +++++++++++++++++++ spec/ransack/search_spec.rb | 19 +++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/docs/docs/going-further/other-notes.md b/docs/docs/going-further/other-notes.md index cfaed2b3e..4d1945dd7 100644 --- a/docs/docs/going-further/other-notes.md +++ b/docs/docs/going-further/other-notes.md @@ -79,6 +79,13 @@ def index end ``` +The distinct option accepts: +- `true` - SELECT DISTINCT on all columns (default behavior) +- `false` or `nil` - no DISTINCT clause +- `'column_name'` - SELECT DISTINCT on a single column +- `[:column1, :column2]` - SELECT DISTINCT on specific columns +- `[]` (empty array) - falls back to SELECT DISTINCT on all columns + Please note that for many databases, a sort on an associated table's columns may result in invalid SQL with `distinct: true` -- in those cases, you will need to modify the result as needed to allow these queries to work. diff --git a/lib/ransack/adapters/active_record/context.rb b/lib/ransack/adapters/active_record/context.rb index 4ccdd7d7a..8713567da 100644 --- a/lib/ransack/adapters/active_record/context.rb +++ b/lib/ransack/adapters/active_record/context.rb @@ -60,13 +60,20 @@ def evaluate(search, opts = {}) end if opts[:distinct] - if opts[:distinct] == true + case opts[:distinct] + when true relation.distinct + when false, nil + relation else # Support distinct on specific columns # Convert string or symbol to array for consistency columns = Array(opts[:distinct]) - relation.distinct(*columns) + if columns.empty? + relation.distinct + else + relation.distinct(*columns) + end end else relation diff --git a/spec/ransack/adapters/active_record/context_spec.rb b/spec/ransack/adapters/active_record/context_spec.rb index db4b99258..07304c4cd 100644 --- a/spec/ransack/adapters/active_record/context_spec.rb +++ b/spec/ransack/adapters/active_record/context_spec.rb @@ -54,6 +54,30 @@ module ActiveRecord expect(result).to be_an ::ActiveRecord::Relation expect(result.to_sql).to match /SELECT DISTINCT/ end + + it 'does not SELECT DISTINCT when distinct: false' do + s = Search.new(Person, name_eq: 'Joe Blow') + result = subject.evaluate(s, distinct: false) + + expect(result).to be_an ::ActiveRecord::Relation + expect(result.to_sql).not_to match /SELECT DISTINCT/ + end + + it 'does not SELECT DISTINCT when distinct: nil' do + s = Search.new(Person, name_eq: 'Joe Blow') + result = subject.evaluate(s, distinct: nil) + + expect(result).to be_an ::ActiveRecord::Relation + expect(result.to_sql).not_to match /SELECT DISTINCT/ + end + + it 'SELECTs DISTINCT when distinct: [] (empty array falls back to DISTINCT)' do + s = Search.new(Person, name_eq: 'Joe Blow') + result = subject.evaluate(s, distinct: []) + + expect(result).to be_an ::ActiveRecord::Relation + expect(result.to_sql).to match /SELECT DISTINCT/ + end end describe '#build_correlated_subquery' do diff --git a/spec/ransack/search_spec.rb b/spec/ransack/search_spec.rb index 6c7ce1d49..b10821057 100644 --- a/spec/ransack/search_spec.rb +++ b/spec/ransack/search_spec.rb @@ -509,6 +509,25 @@ module Ransack expect(result_string.to_sql).to match /SELECT DISTINCT/ end + it 'handles distinct edge cases correctly' do + s = Search.new(Person, name_eq: 'Ernie') + + # Test with false - should not use distinct + result_false = s.result(distinct: false) + expect(result_false).to be_an ActiveRecord::Relation + expect(result_false.to_sql).not_to match /SELECT DISTINCT/ + + # Test with nil - should not use distinct + result_nil = s.result(distinct: nil) + expect(result_nil).to be_an ActiveRecord::Relation + expect(result_nil.to_sql).not_to match /SELECT DISTINCT/ + + # Test with empty array - should fall back to regular distinct + result_empty = s.result(distinct: []) + expect(result_empty).to be_an ActiveRecord::Relation + expect(result_empty.to_sql).to match /SELECT DISTINCT/ + end + it 'evaluates joins with belongs_to join' do s = Person.joins(:parent).ransack(parent_name_eq: 'Ernie').result(distinct: true) expect(s).to be_an ActiveRecord::Relation