Repository navigation
Conversation
kurko
changed the title
When a resource is included through a has_many, its inverse linkage (for example the book's book_comments when comments include their book) is built from a SELECT DISTINCT ordered only by the related key. Every row shares that key, so the linkage order depends on the query plan. On Postgres the plan switches from Sort/Unique to HashAggregate once autovacuum analyzes the freshly created test tables, and the cache warmup request (which adds updated_at to the DISTINCT) then returns a different order than the normal request. This made test_pagination_related_resources_data_includes fail at random in Postgres CI jobs, and gave API clients an unstable linkage order. This change adds the source primary key as a final ORDER BY in find_related_monomorphic_fragments. The column is already in the select list as source_id, so it is valid with DISTINCT. It also adds a test that analyzes the tables first, which makes the failure reproduce on every run. find_related_polymorphic_fragments builds a similar query and may need the same tiebreaker. test_sort_included_attribute has a separate flake: it sorts on a nullable column and asserts which of several NULL rows comes first.
The cacheable GET assertions compared the cached and uncached responses as whole strings, so they also required the same order inside to-many linkage arrays. JSON:API gives that order no meaning, and the library does not guarantee it: on Postgres the plan for the included linkage query changes once autovacuum analyzes the test tables, and the two requests then return the linkage in different orders. The previous commit made the order fixed with an extra ORDER BY, which puts a production cost on every included has_many only to satisfy a test expectation stricter than the spec. This change reverts that ORDER BY. It adds normalize_linkage_order to the functional helpers, which sorts relationship data arrays in data and included, and both cacheable assertions compare normalized responses. Primary data order is untouched, since the sort parameter controls it. The regression test now checks which comment ids are in the linkage, not their order. The included array has no defined order either, and could vary the same way if a future plan change reorders it. It is left strict until a test shows that happening.
kurko
force-pushed
the
fix-included-linkage-order
branch
from
October 25, 2026 01:01
961a85b to
d709508
Compare
8 of 9 tasks
kurko
marked this pull request as ready for review
October 25, 2026 01:11
Collaborator
Author
|
Failures are unrelated to this PR. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_pagination_related_resources_data_includes fails at random in Postgres CI jobs (seen twice on #1496). The cache assertions compare the cached and uncached responses as whole strings, so they expect the same order inside to-many linkage arrays. The JSON:API spec gives that order no meaning, so the test expects more than the spec does. On Postgres the plan for the included linkage query changes once autovacuum analyzes the test tables, and the two requests then return the linkage in different orders.
This branch sorts relationship linkage arrays in both responses before comparing them (normalize_linkage_order in test/helpers/functional_helpers.rb), with tests for the helper and a regression test that runs ANALYZE first so the failure reproduces on every run. Library code is unchanged.
CI jobs for Ruby 3.1 and older currently fail before running tests, because ubuntu-latest moved to 26.04 and setup-ruby has no builds of those versions for it. That affects every PR and is not fixed here.