Skip to content

Fix price indexation loop stopping early with non-sequential product ids - #1258

Open
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/indexprices-loop-condition-1198
Open

Fix price indexation loop stopping early with non-sequential product ids#1258
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/indexprices-loop-condition-1198

Conversation

@boo-code

Copy link
Copy Markdown
Contributor
Questions Answers
Description? indexPrices() used $cursor as the do/while continuation check: $cursor < $nbProducts. But $cursor is the last indexed id_product returned by indexPricesUnbreakable() (return (int) $lastIdProduct;), not a progress counter — while $nbProducts is the number of products to index. Product ids are not sequential and are routinely larger than the product count (gaps from deleted / imported / migrated products), so $cursor < $nbProducts flips to false as soon as an indexed id exceeds the count. The batching loop then stops after very few iterations and indexation falls back to a single 100-row batch per indexPrices() self-recursion (CLI/cron) or per AJAX round-trip (BO "rebuild index"). On large catalogues this both slows full indexation dramatically and deepens the indexPrices() recursion (one level per 100 products). The method already maintains $indexedProducts += $length;, which is the real progress counter, so the loop now compares against that instead.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #1198.
How to test? On a catalogue where the highest active id_product is greater than the number of indexable products (e.g. delete/migrate products so ids are sparse, or simply have more than ~100 products with non-contiguous ids), rebuild the faceted-search price index (BO > module configuration, or Configuration::updateValue('PS_LAYERED_INDEXED', 0) then trigger indexing). Before: the layered_price_index table is filled only in small steps (one 100-row batch per call), and on large/sparse catalogues full indexation is slow or doesn't complete within the recursion/limits. After: each call batches until the time/memory limit is reached and the table is fully populated.
Sponsor company

Notes on verification

indexPrices()/indexPricesUnbreakable() are private methods on the main Ps_facetedsearch module class, which currently has no unit-test coverage (the suite targets the src/ classes), and they are tightly coupled to the database. I verified the fix by tracing the code rather than with a mock-heavy reflection test: indexPricesUnbreakable() returns $lastIdProduct (an id, confirmed), and $indexedProducts increments by $length per batch, so $indexedProducts < $nbProducts is the correct continuation condition. It is monotonic (no infinite-loop risk) and the existing memory / max_execution_time / $cursor == 0 guards are untouched. Happy to add a test if you have a preferred pattern for covering the module class.

indexPrices() used $cursor (the last indexed id_product returned by
indexPricesUnbreakable()) as the do/while continuation check against $nbProducts
(the number of products to index). Since product ids are not necessarily
sequential and are commonly larger than the product count (gaps left by deleted,
imported or migrated products), $cursor < $nbProducts becomes false as soon as
an indexed id exceeds the count, so the batching loop stops after very few
iterations and the work falls back to one 100-row batch per recursion/AJAX call.
On large catalogues this both slows full indexation dramatically and deepens the
indexPrices() self-recursion. Compare the real progress counter instead.
@ps-jarvis

Copy link
Copy Markdown

Hello @boo-code!

This is your first pull request on ps_facetedsearch repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

@github-project-automation github-project-automation Bot moved this to Ready for review in PR Dashboard Jun 27, 2026
@ps-jarvis ps-jarvis added the Waiting for QA Status: Action required, Waiting for test feedback label Jul 6, 2026
@ps-jarvis ps-jarvis moved this from Ready for review to To be tested in PR Dashboard Jul 6, 2026
@kpodemski kpodemski added the Waiting for QA by Community Status: Action required, Waiting for test feedback by Community label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for QA by Community Status: Action required, Waiting for test feedback by Community Waiting for QA Status: Action required, Waiting for test feedback

Projects

Status: To be tested

Development

Successfully merging this pull request may close these issues.

[BUG] Wrong loop break condition in indexPrices() prevents full price indexation

3 participants