Skip to content

Batch translations during model hydration - #679

Merged
daftspunk merged 16 commits into
octobercms:developfrom
samuelpatro:perf/translation-batching
Sep 26, 2026
Merged

daftspunk merged 16 commits into
octobercms:developfrom
samuelpatro:perf/translation-batching

Conversation

@samuelpatro

@samuelpatro samuelpatro commented Sep 5, 2026 •

Copy link
Copy Markdown
Member

Fetching translatable models in a non-default locale runs one translation query per row during afterFetch. This preloads the active locale's translations for the rows being hydrated, in chunks of 500 ids, and gives each model its rows before fetched fires, so fetched listeners still see translated values. Fetching 100 records goes from 101 queries to 2.

Missing translations, the default locale, eager-loaded translations, dirty tracking and saves behave as before. Models that override loadTranslatableData() or getTranslateAttributeTable(), or whose morph class depends on the row, keep loading per row, since those can read other storage. cursor() is unchanged. Rows selected without their key also skip the translation query they used to run.

@samuelpatro
samuelpatro marked this pull request as ready for review September 18, 2026 07:42
@samuelpatro

Copy link
Copy Markdown
Member Author

Recheck of head 3c60da88 found an uncovered model override regression.

[P2] Allow replacement translation loaders to bypass the default store. hydrateWithTranslatableBatch() queries getTranslateAttributeTable() before a custom loadTranslatableData($locale) can run. A model whose loader supplies translations entirely from JSON/an external service, without calling parent, need not have the standard translation table.

Reproduced on PHP 8.4 with such a loader and no translate_attributes table: merge-base cursor() and get() both return the translated value; this PR's cursor() works but get() throws SQLSTATE[HY000]: General error: 1 no such table: translate_attributes from the preload query. The new override test decorates the parent loader and misses replacement storage.

Preserve this extension behavior with an override-aware fallback or overridable batch eligibility/storage, and add a regression without the default translation table. This contradicts the promised preservation of normal model overrides. The existing full suite passes (251 tests, 1529 assertions, one pre-existing risky test), so this needs additional coverage.

Separately, current develop has an add/add conflict in tests/Database/Traits/TranslatableTest.php; retain both sets of tests when updating.

@daftspunk daftspunk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to resolve conflicts

Models that override the loader or table getter already skip batching,
so the hydration snapshot no longer needs to route through the loader.
Keep only tests that fail when a batching guarantee breaks.
Check the locale before reading row keys, and return before reading
the key when no batch is active, so default-locale fetches cost the
same as before.
@samuelpatro

samuelpatro commented Sep 26, 2026 •

Copy link
Copy Markdown
Member Author

@daftspunk rechecked and simplified this. Models that override loadTranslatableData() or getTranslateAttributeTable() already skip batching, so the preloaded rows no longer need to go through the loader. Model::newFromBuilder() now makes one applyTranslatableBatch() call that gives each model its rows before fetched. That removes the per-instance snapshot, the connection and table checks, and the newFromBuilderInstance() split. A model whose getMorphClass() depends on the row still loads per row. The test file went from 23 tests to 15, and each one fails when the behavior it covers breaks, including querying the wrong morph class or connection.

get() in a non-default locale (SQLite in memory, PHP 8.4, median of 15 runs):

Rows develop this PR
25 26 queries, 1.04 ms 2 queries, 0.31 ms
100 101 queries, 4.11 ms 2 queries, 1.09 ms
1000 1001 queries, 40.1 ms 3 queries, 12.4 ms

The default locale still runs one query and takes the same time (6.3 to 7.1 ms for 1000 rows on both). On MySQL or Postgres each skipped query also saves a network round trip, so the gap is larger than shown here.

Ready for another pass.

The batch must query the model's morph class on the default
connection, the same storage the per-row loader reads.
@daftspunk
daftspunk merged commit ade81a4 into octobercms:develop Sep 26, 2026
4 checks passed
@daftspunk

Copy link
Copy Markdown
Member

Bravo! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants