Skip to content

fix: preserve input order in bulk manage_relationship#2639

Merged
zachdaniel merged 4 commits intoash-project:mainfrom
nallwhy:fix/bulk-create-relationship-ordering
Mar 19, 2026
Merged

fix: preserve input order in bulk manage_relationship#2639
zachdaniel merged 4 commits intoash-project:mainfrom
nallwhy:fix/bulk-create-relationship-ordering

Conversation

@nallwhy
Copy link
Contributor

@nallwhy nallwhy commented Mar 18, 2026

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

Summary

  • Add sorted?: true to Ash.bulk_create calls in do_batch_create to preserve order within create results (the do_batch_create_m2m path already had this)
  • Fix input ordering when bulk_manage_relationship processes a mix of creates and updates — they are batched separately (creates first, updates second), which breaks the original input order regardless of sorted?
  • Tag each managed record with its original input index via metadata, then sort by index instead of reversing

Problem

There are two ordering issues in the bulk manage_relationship path:

1. Missing sorted?: true in do_batch_create

After the sequential → bulk_create refactoring, the has_many path was missing sorted?: true. Without it, bulk_create may return records in a different order than the input.

2. Creates and updates processed in separate batches

bulk_manage_relationship classifies inputs into creates and updates, then processes creates first, updates second. Each result is prepended via [record | acc], and the caller does Enum.reverse at the end. This means the final order is always creates before updates, regardless of original input order.

For example, input [existing, new1, new2] produces [new1, new2, existing].

The sequential path processes inputs one-by-one in order, so it doesn't have this problem.

Both issues are reproducible with the ETS data layer — the added regression test demonstrates this.

Solution

  1. sorted?: true — Added to Ash.bulk_create calls so create results match input order.
  2. Input index tagging — Every code path that adds a record to new_value (struct inputs, already_created, bulk_create results, m2m join, sequential fallback, updates, lookups) tags it with __manage_relationship_index__ metadata via tag_input_index/2.
  3. Index-based sorting — At the final step, if metadata is present, sort by index and strip the metadata instead of reversing. If absent (sequential path), fall back to the existing Enum.reverse.

Copilot AI review requested due to automatic review settings March 18, 2026 05:09
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes ordering of records returned from manage_relationship when creating multiple related records in bulk, ensuring the returned list preserves the input order across data layers where bulk insert return order is not guaranteed.

Changes:

  • Add sorted?: true to the Ash.bulk_create/4 call in the do_batch_create (has_many) path to preserve input ordering.
  • Align has_many bulk-create behavior with existing many_to_many bulk-create logic that already uses sorted?: true.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nallwhy nallwhy marked this pull request as draft March 18, 2026 06:55
@nallwhy nallwhy changed the title fix: preserve input order in manage_relationship bulk_create fix: preserve input order in bulk manage_relationship Mar 19, 2026
@nallwhy nallwhy requested a review from Copilot March 19, 2026 00:11
@nallwhy nallwhy marked this pull request as ready for review March 19, 2026 00:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes ordering issues in bulk_manage_relationship so that results preserve the original input order even when creates and updates are interleaved.

Changes:

  • Ensure Ash.bulk_create/3 returns records in input order by adding sorted?: true to the has_many batch create path.
  • Preserve original input order across mixed create/update batches by tagging managed records with their input index and sorting by that index instead of relying on Enum.reverse/1.
  • Add a regression test that fails when interleaved creates/updates don’t preserve order.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/ash/actions/managed_relationships.ex Tags managed records with input indices, sorts results by index (vs reverse), and adds sorted?: true to bulk creates to preserve order.
test/manage_relationship_test.exs Adds a regression test asserting interleaved creates/updates preserve the user-provided order.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

zachdaniel
zachdaniel previously approved these changes Mar 19, 2026
@zachdaniel
Copy link
Contributor

Let's check format here. I also think that sorting might be something that should be opt-in in the manage relationship options, as this does add a fair bit of expense. Can you add a comment that contains 4.0 or open an issue for us making this opt-in in 4.0?

@nallwhy
Copy link
Contributor Author

nallwhy commented Mar 19, 2026

@zachdaniel

sorted?: true actually does in-memory sorting only — it tags each changeset with an index via metadata during creation, then sorts by that index after the bulk operation completes. The real cost is two O(n log n) in-memory sorts where n is the number of relationship records (typically small).

The motivation here is that without this, the result returned from an update can have a different order than the input, which causes issues in forms — e.g., a user submits relationship items in order [A, B, C, D], but the returned result comes back as [B, D, A, C], causing the UI to reorder items unexpectedly on re-render.

That said, I'm happy to open a 4.0 issue for making this opt-in if you still think it's worth it. Let me know!

@zachdaniel
Copy link
Contributor

Its probably fine to have as is but in general we have sorted outputs as an opt-in feature which allows us to freely change the internals without having to preserve ordering, so changing it in 4.0 to not guarantee order would make sense IMO. But you're also right the cost is probably not significant enough.

@zachdaniel
Copy link
Contributor

Looks like mix format needs to run then its good 😄

@zachdaniel zachdaniel merged commit 60fce17 into ash-project:main Mar 19, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants