From 8d0b8819b22ab5fcfeb3978ae23ae7984b0c807d Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Sat, 24 Oct 2026 09:34:10 -0700 Subject: [PATCH 1/2] feat: Order results by a field of their nested children with `order_by_child` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: nested documents could filter parents by their children (`with_child`, `without_child`) but not sort by them, so "projects by when their design milestone started" wasn't expressible. Sunspot has no precedent for sorting by children, so this follows Solr's: sort by a `{!parent}` block join query whose score is the children's value. `order_by_child :milestones, :started_at, :desc do … end` takes the same block as `with_child`, and adds a `Sort::ChildSort`. The sort is `exists(query($child_sort_N)) desc, query($child_sort_N) `, where `child_sort_N` is `{!parent … score=}` over `{!bool}` of the block's restrictions, a filter that the field has a value, and `{!func}` of the field (`ms()` for a time). A parent with no matching child that has a value comes last in either direction. Several children combine by the smallest value ascending and the largest descending, unless `:score` says otherwise. `SortComposite` names each child sort's parameters by its position. `DSL::Scope#build_block_join` is shared with `with_child`. Only single-value numeric and time fields can be sorted by; a date range field raises `ArgumentError`. Solr compares the scores as floats, so values too close together can tie, and reads a negative score as zero, so times before 1970 tie with each other and negative numbers with zero. The README and the doc comment say so. It needs Solr 8 or later, which the README and doc comment also say. Solr 5.3.1, which `sunspot_solr` bundles, and 6.6.6 reject the sort, and 7.7.3 doesn't tie times before 1970 and gets a descending `:score => :min` sort wrong. `IntegrationHelper#skip_before_solr` skips its integration specs before Solr 8. Co-Authored-By: Claude Opus 5.5 --- README.md | 17 +++++ sunspot/lib/sunspot/dsl/field_query.rb | 44 +++++++++++ sunspot/lib/sunspot/dsl/scope.rb | 8 +- sunspot/lib/sunspot/query/sort.rb | 57 ++++++++++++++ sunspot/lib/sunspot/query/sort_composite.rb | 18 ++++- sunspot/spec/api/nested_documents_spec.rb | 76 +++++++++++++++++++ sunspot/spec/helpers/integration_helper.rb | 6 ++ .../spec/integration/nested_documents_spec.rb | 74 ++++++++++++++++++ sunspot/spec/mocks/project.rb | 1 + 9 files changed, 297 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 883c5908f..245cb6305 100644 --- a/README.md +++ b/README.md @@ -1073,6 +1073,23 @@ The block inside `with_child` / `without_child` takes the same restrictions as a Searching several classes covers each class that declares the association, and a child field resolves as parent fields do: when every class declaring it agrees on its type. +#### Sorting by a child field + +**Solr 8 and above.** Solr 6 and earlier, including the Solr that `sunspot_solr` bundles, reject the sort, and Solr 7 orders some parents differently. + +`order_by_child` sorts parents by a single-value numeric or time field of their children, among the children meeting the block's restrictions. A parent with no such child, or whose such children have no value for the field, comes last in either direction. + +```ruby +# Projects by when their design milestone started, latest first +Project.search do + order_by_child :milestones, :started_at, :desc do + with :name, 'design' + end +end +``` + +A parent with several such children sorts by the smallest value ascending and the largest descending. `:score => :min`, `:max`, `:avg` or `:total` combines them another way. Solr compares the values as floats, so times less than about two minutes apart, and integers above 2**24, can tie. Solr also reads a negative value as zero, so times before 1970 all sort as 1970, and negative numbers as zero. A later `order_by` breaks the tie. + #### Schema The schema needs a `_root_` field with the same type as `id`. The bundled configset has one. diff --git a/sunspot/lib/sunspot/dsl/field_query.rb b/sunspot/lib/sunspot/dsl/field_query.rb index 46ecbb613..52933253e 100644 --- a/sunspot/lib/sunspot/dsl/field_query.rb +++ b/sunspot/lib/sunspot/dsl/field_query.rb @@ -52,6 +52,50 @@ def order_by_geodist(field_name, lat, lon, direction = nil) ) end + # + # Specify that the results should be ordered by a field of their children + # in a nested association, among the children that satisfy every + # restriction in the block. The block takes the same restrictions as + # DSL::Scope#with_child. Results with no such child, or whose such + # children have no value for the field, come last, whatever the + # direction. Requires Solr 8 or later. + # + # A result with several such children sorts by the earliest or smallest + # value ascending and by the latest or largest descending, unless + # +:score+ chooses how Solr combines them. Solr reads a negative value + # as zero, so times before 1970 tie with each other, and negative + # numbers with zero. + # + # ==== Parameters + # + # name:: + # the association, declared with DSL::Fields#nested + # field_name:: + # a single-value numeric or time field of the child + # direction:: + # :asc or :desc (default :asc) + # + # ==== Options + # + # :score:: + # :min, :max, :avg or :total, Solr's names for combining the values of + # several children + # + # ==== Example + # + # Sunspot.search(Project) do + # order_by_child :milestones, :started_at, :desc do + # with :name, 'design' + # end + # end + # + def order_by_child(name, field_name, direction = nil, options = {}, &block) + block_join, child_setup = build_block_join(false, name, &block) + @query.add_sort( + Sunspot::Query::Sort::ChildSort.new(block_join, child_setup.field(field_name), direction, options) + ) + end + # # DEPRECATED Use order_by(:random) # diff --git a/sunspot/lib/sunspot/dsl/scope.rb b/sunspot/lib/sunspot/dsl/scope.rb index 14af387a5..8f3d9917e 100644 --- a/sunspot/lib/sunspot/dsl/scope.rb +++ b/sunspot/lib/sunspot/dsl/scope.rb @@ -242,11 +242,17 @@ def text_fields(&block) private def add_block_join(negated, name, &block) + @scope.add_component(build_block_join(negated, name, &block).first) + end + + # Returns a block join on the association, restricted by the block, and + # the setup that resolves the association's fields. + def build_block_join(negated, name, &block) nested_setups = @setup.nested_setups_named(name) child_setup = nested_setups.one? ? nested_setups.first : CompositeNestedSetup.new(nested_setups) block_join = Sunspot::Query::BlockJoin.new(nested_setups.first, negated, nil, nested_setups.map(&:path).uniq) Util.instance_eval_or_call(Scope.new(block_join.scope, child_setup), &block) if block - @scope.add_component(block_join) + [block_join, child_setup] end def add_restriction(negated, *args) diff --git a/sunspot/lib/sunspot/query/sort.rb b/sunspot/lib/sunspot/query/sort.rb index dcc1bb7aa..8db878fa5 100644 --- a/sunspot/lib/sunspot/query/sort.rb +++ b/sunspot/lib/sunspot/query/sort.rb @@ -115,6 +115,63 @@ def to_param end end + # + # A ChildSort sorts parents by a field of their children in a nested + # association, among the children matching its block join's scope that + # have a value for the field. The sort is by a block join query whose + # score is those children's field values, combined by +:score+. A parent + # with no such child doesn't match that query, and sorts after those that + # do. + # + # The query takes its parts from parameters of its own, named by + # SortComposite, which keeps each part's escaping to one level. Solr + # compares scores as floats, so two times less than about two minutes + # apart, or integers above 2**24, may tie. + # + class ChildSort < Abstract + SCORES = [:min, :max, :avg, :total] + + def initialize(block_join, field, direction = nil, options = {}) + super(direction) + if field.multiple? + raise(ArgumentError, "#{field.name} cannot be used for ordering because it is a multiple-value field") + end + if field.type.is_a?(Type::DateRangeType) || !(field.type.is_a?(Type::TimeType) || field.type.is_a?(Type::IntegerType) || field.type.is_a?(Type::FloatType)) + raise(ArgumentError, "#{field.name} cannot be used for ordering children because it is not a numeric or time field") + end + @block_join, @field = block_join, field + @score = (options[:score] || (direction_for_solr == 'asc' ? :min : :max)).to_sym + unless SCORES.include?(@score) + raise(ArgumentError, "Unknown score #{@score.inspect}. Acceptable input is: #{SCORES.map(&:inspect) * ', '}") + end + end + + def to_param(name = 'child_sort') + "exists(query($#{name})) desc, query($#{name}) #{direction_for_solr}" + end + + # The parameters #to_param refers to, by the same +name+ + def params(name) + { + :"#{name}" => %Q({!parent which="#{BlockJoin.escape(BlockJoin::ROOTS)}" score=#{@score} v=$#{name}_child}), + :"#{name}_child" => "{!bool filter=$#{name}_filter filter=$#{name}_present must=$#{name}_value}", + :"#{name}_filter" => @block_join.scope.to_boolean_phrase, + :"#{name}_present" => "#{@field.indexed_name}:[* TO *]", + :"#{name}_value" => "{!func}#{value_function}" + } + end + + private + + def value_function + if @field.type.is_a?(Type::TimeType) + "ms(#{@field.indexed_name})" + else + "field(#{@field.indexed_name})" + end + end + end + class SolrIdSort < Abstract def to_param "id #{direction_for_solr}" diff --git a/sunspot/lib/sunspot/query/sort_composite.rb b/sunspot/lib/sunspot/query/sort_composite.rb index b75ef764c..633f8fd85 100644 --- a/sunspot/lib/sunspot/query/sort_composite.rb +++ b/sunspot/lib/sunspot/query/sort_composite.rb @@ -25,13 +25,25 @@ def include?(sort) @sorts.any? { |s| s.to_param.include?(sort) } end - # - # Combine the sorts into a single param by joining them + # + # Combine the sorts into a single param by joining them. A sort with + # parameters of its own, such as a Sort::ChildSort, has them named by + # its position, so two of them can't collide. # def to_params(prefix = "") unless @sorts.empty? key = "#{prefix}sort".to_sym - { key => @sorts.map { |sort| sort.to_param } * ', ' } + params = {} + clauses = @sorts.each_with_index.map do |sort, index| + if sort.respond_to?(:params) + name = "child_sort_#{index}" + params.merge!(sort.params(name)) + sort.to_param(name) + else + sort.to_param + end + end + params.merge(key => clauses * ', ') else {} end diff --git a/sunspot/spec/api/nested_documents_spec.rb b/sunspot/spec/api/nested_documents_spec.rb index aa1f6e6f6..15f457154 100644 --- a/sunspot/spec/api/nested_documents_spec.rb +++ b/sunspot/spec/api/nested_documents_spec.rb @@ -258,6 +258,82 @@ def children(document) end end + describe 'ordering by a child field' do + def last_search + connection.searches.last + end + + it 'sorts by a block join over the matching children, with parents that have none last' do + session.search(Project) { order_by_child(:milestones, :started_at) { with :name, 'design' } } + expect(last_search[:sort]).to eq('exists(query($child_sort_0)) desc, query($child_sort_0) asc') + expect(last_search[:child_sort_0]).to eq(%q({!parent which="*:* -_sunspot_nested_path_s:[* TO *]" score=min v=$child_sort_0_child})) + expect(last_search[:child_sort_0_child]).to eq('{!bool filter=$child_sort_0_filter filter=$child_sort_0_present must=$child_sort_0_value}') + expect(last_search[:child_sort_0_filter]).to eq('(_sunspot_nested_path_s:"Project.milestones" AND name_s:design)') + expect(last_search[:child_sort_0_present]).to eq('started_at_d:[* TO *]') + expect(last_search[:child_sort_0_value]).to eq('{!func}ms(started_at_d)') + end + + it 'leaves the filter queries alone' do + session.search(Project) { order_by_child(:milestones, :started_at) { with :name, 'design' } } + expect(last_search[:fq]).to eq(['type:Project']) + end + + it 'takes the earliest child ascending and the latest descending' do + session.search(Project) { order_by_child(:milestones, :started_at, :desc) } + expect(last_search[:sort]).to eq('exists(query($child_sort_0)) desc, query($child_sort_0) desc') + expect(last_search[:child_sort_0]).to include('score=max') + end + + it 'combines the children by the given score' do + session.search(Project) { order_by_child(:milestones, :started_at, :asc, :score => :max) } + expect(last_search[:child_sort_0]).to include('score=max') + end + + it 'raises for a score Solr has no name for' do + expect { session.search(Project) { order_by_child(:milestones, :started_at, :asc, :score => :median) } }.to raise_error(ArgumentError) + end + + it 'sorts by the field itself for a number' do + session.search(Project) { order_by_child(:milestones, :days_late) } + expect(last_search[:child_sort_0_value]).to eq('{!func}field(days_late_i)') + end + + it 'scopes a sort with no block to the association alone' do + session.search(Project) { order_by_child(:milestones, :started_at) } + expect(last_search[:child_sort_0_filter]).to eq('_sunspot_nested_path_s:"Project.milestones"') + end + + it 'names each sort by its position among the others' do + session.search(Project) do + order_by :name + order_by_child(:milestones, :started_at) { with :name, 'design' } + order_by_child(:milestones, :finished_at) { with :name, 'build' } + end + expect(last_search[:sort]).to eq('name_s asc, exists(query($child_sort_1)) desc, query($child_sort_1) asc, exists(query($child_sort_2)) desc, query($child_sort_2) asc') + expect(last_search[:child_sort_1_value]).to eq('{!func}ms(started_at_d)') + expect(last_search[:child_sort_2_value]).to eq('{!func}ms(finished_at_d)') + end + + it 'raises for a text, string, date range or multiple-value field' do + expect { session.search(Project) { order_by_child(:milestones, :name) } }.to raise_error(ArgumentError) + expect { session.search(Project) { order_by_child(:milestones, :active) } }.to raise_error(ArgumentError) + expect { session.search(Project) { order_by_child(:milestones, :owner_names) } }.to raise_error(ArgumentError) + end + + it 'raises for a field the association does not declare' do + expect { session.search(Project) { order_by_child(:milestones, :verdict) } }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it 'raises for an association that was never declared' do + expect { session.search(Project) { order_by_child(:tasks, :started_at) } }.to raise_error(Sunspot::UnrecognizedFieldError) + end + + it 'resolves the association across a multi-class search' do + session.search(Project, Program) { order_by_child(:milestones, :started_at) } + expect(last_search[:child_sort_0_filter]).to include('Project.milestones') + end + end + describe 'removal' do it 'removes the whole block when removing a parent' do project = Project.new diff --git a/sunspot/spec/helpers/integration_helper.rb b/sunspot/spec/helpers/integration_helper.rb index bcf5db52b..598ceaa81 100644 --- a/sunspot/spec/helpers/integration_helper.rb +++ b/sunspot/spec/helpers/integration_helper.rb @@ -19,6 +19,12 @@ def skip_from_solr(major, reason) skip "#{reason} (Solr #{version})" if version.to_i >= major end + # Skips the example before Solr `major`, for behaviour that only that version and later have + def skip_before_solr(major, reason) + version = IntegrationHelper.solr_version + skip "#{reason} (Solr #{version})" if version.to_i < major + end + def featured_for_posts(method, param, negated = false) with_method = negated ? :without : :with param = date_ranges[param] if param.is_a? String diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index f463f2683..6758ff016 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -111,6 +111,80 @@ def skip_before_solr_8 end end + describe 'order_by_child' do + before { skip_before_solr 8, 'Solr 6 and earlier reject the sort, and Solr 7 orders some parents differently' } + + it 'sorts by the matching child, with parents that have none last' do + expect(search { order_by_child(:milestones, :started_at) { with :name, 'design' } }).to eq([@designed_earlier, @designed_in_q1, @without_milestones]) + end + + it 'keeps parents that have no matching child last when descending' do + expect(search { order_by_child(:milestones, :started_at, :desc) { with :name, 'design' } }).to eq([@designed_in_q1, @designed_earlier, @without_milestones]) + end + + it 'sorts by the earliest of several children ascending' do + expect(search { order_by_child(:milestones, :started_at) }.first).to eq(@designed_earlier) + end + + it 'combines several children by the given score' do + @designed_earlier.milestones << milestone('launch', Time.utc(2026, 3, 1)) + Sunspot.index!(@designed_earlier) + expect(search { order_by_child(:milestones, :started_at, :desc) }.first).to eq(@designed_earlier) + expect(search { order_by_child(:milestones, :started_at, :desc, :score => :min) }.first).to eq(@designed_in_q1) + end + + it 'sorts a child whose value is zero before parents with no child' do + expect(search { order_by_child(:milestones, :days_late) }).to eq([@designed_earlier, @designed_in_q1, @without_milestones]) + expect(search { order_by_child(:milestones, :days_late, :desc) }).to eq([@designed_in_q1, @designed_earlier, @without_milestones]) + end + + it 'sorts a parent whose matching children lack the field with the parents that have none' do + undated = Project.new(:name => 'undated', :milestones => [milestone('design', nil)]) + Sunspot.index!(undated) + expect(search { order_by_child(:milestones, :started_at) { with :name, 'design' } }.first(2)).to eq([@designed_earlier, @designed_in_q1]) + expect(search { order_by_child(:milestones, :started_at, :desc) { with :name, 'design' } }.first(2)).to eq([@designed_in_q1, @designed_earlier]) + end + + it 'sorts times before 1970 as 1970, so they tie with each other' do + historic = Project.new(:name => 'historic', :milestones => [milestone('design', Time.utc(1960, 1, 1))]) + older = Project.new(:name => 'older', :milestones => [milestone('design', Time.utc(1950, 1, 1))]) + Sunspot.index!(historic, older) + by_name = lambda do |direction| + search do + order_by_child(:milestones, :started_at) { with :name, 'design' } + order_by(:name, direction) + end + end + # Their names order them, in either direction, though 1950 is before 1960 + expect(by_name.call(:asc)).to eq([historic, older, @designed_earlier, @designed_in_q1, @without_milestones]) + expect(by_name.call(:desc)).to eq([older, historic, @designed_earlier, @designed_in_q1, @without_milestones]) + descending = search { order_by_child(:milestones, :started_at, :desc) { with :name, 'design' } } + expect(descending.first(2)).to eq([@designed_in_q1, @designed_earlier]) + expect(descending[2, 2]).to contain_exactly(historic, older) + end + + it 'sorts the matching parents alongside a filter on another child' do + results = search do + with_child(:milestones) { with :name, 'build' } + order_by_child(:milestones, :started_at) { with :name, 'design' } + end + expect(results).to eq([@designed_earlier]) + end + + it 'breaks ties with the next sort' do + results = search do + order_by_child(:milestones, :started_at) { with :name, 'build' } + order_by :name, :desc + end + expect(results).to eq([@designed_earlier, @without_milestones, @designed_in_q1]) + end + + it 'sorts across pages' do + pages = (1..3).map { |page| search { order_by_child(:milestones, :started_at) { with :name, 'design' }; paginate :page => page, :per_page => 1 } } + expect(pages.flatten).to eq([@designed_earlier, @designed_in_q1, @without_milestones]) + end + end + describe 'searching the parents alone' do it 'returns parents, never children' do expect(search {}).to match_array([@designed_in_q1, @designed_earlier, @without_milestones]) diff --git a/sunspot/spec/mocks/project.rb b/sunspot/spec/mocks/project.rb index f5d709308..b04df3d5e 100644 --- a/sunspot/spec/mocks/project.rb +++ b/sunspot/spec/mocks/project.rb @@ -27,6 +27,7 @@ def reviews time :due_at string :owner_names, :multiple => true integer(:days_late) { finished_at && due_at && finished_at > due_at ? ((finished_at - due_at) / 86_400).to_i : 0 } + date_range(:active) { started_at && finished_at && (started_at..finished_at) } end nested :reviews do From 1f1caa072fcff253bc922607834fa43b2d578852 Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Sat, 24 Oct 2026 09:34:10 -0700 Subject: [PATCH 2/2] test: Use `skip_before_solr` for the nested documents reindex specs The nested documents spec defined its own `skip_before_solr_8`, which asked Solr for its version on every call. It now calls `IntegrationHelper#skip_before_solr`, which reads the memoized `IntegrationHelper.solr_version`. Co-Authored-By: Claude Opus 5.5 --- sunspot/spec/integration/nested_documents_spec.rb | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sunspot/spec/integration/nested_documents_spec.rb b/sunspot/spec/integration/nested_documents_spec.rb index 6758ff016..595217e0c 100644 --- a/sunspot/spec/integration/nested_documents_spec.rb +++ b/sunspot/spec/integration/nested_documents_spec.rb @@ -17,12 +17,6 @@ def solr_count(query) let(:q1) { Time.utc(2026, 1, 1)...Time.utc(2026, 4, 1) } - # Solr before 8 replaces a document with children by _root_ and one without by id - def skip_before_solr_8 - version = Sunspot.session.send(:connection).get('admin/system')['lucene']['solr-spec-version'] - skip "Solr #{version} doesn't replace a block whose children went from some to none or none to some" if version.to_i < 8 - end - before :each do Sunspot.remove_all! @@ -205,8 +199,9 @@ def skip_before_solr_8 expect(search { with_child(:milestones) { with :owner_names, 'ben' } }).to eq([]) end + # Solr before 8 replaces a document with children by _root_ and one without by id it 'removes every child when a parent is reindexed with none' do - skip_before_solr_8 + skip_before_solr 8, "doesn't replace a block whose children went from some to none or none to some" @designed_earlier.milestones = [] Sunspot.index!(@designed_earlier) @@ -215,7 +210,7 @@ def skip_before_solr_8 end it 'replaces a parent that had no children when it is reindexed with some' do - skip_before_solr_8 + skip_before_solr 8, "doesn't replace a block whose children went from some to none or none to some" @without_milestones.milestones = [milestone('launch', Time.utc(2026, 3, 1))] Sunspot.index!(@without_milestones)