Fix the configurable stock index child status join and drop the isSalable N+1 on listings - #37
Merged
jeanmarcos-dev merged 2 commits intoJul 30, 2026
Conversation
…join of the parent stock index
… from the stock index
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.
Two related changes in the configurable-product salability path: a correctness fix on the stock index, and the removal of an N+1 on product listings.
1. Stock index: harden the child status join
InventoryConfigurableProductIndexer/Indexer/SelectBuilder.phpjoinscatalog_product_entity_intso that disabled children stop counting towards the parent's salable aggregate (introduced by magento#3241). Two problems with that join:It is not scoped to a store.
statushas one row per store scope, so a child enabled in the default scope and in a store view produced two rows in the group, doublingSUM(stock.quantity). Measured on a parent with 15 children of 100 qty each, on a non-default stock:quantitystore_id = 0is_salablewas unaffected; the inflated value is what the "only X left" displays consume.It drops the parent row entirely when no child is enabled, because it is an
INNER JOIN. A row that is absent behaves like a row set to0foraddStockDataToCollection(which filtersis_salable = 1), but not forGetStockItemsData: a single-sku lookup that finds nothing falls back tocataloginventory_stock_item, which for a configurable reports in-stock and therefore salable.Both are fixed by pinning the join to
Store::DEFAULT_STORE_IDand turning it into aLEFT JOIN, moving the status test into the aggregate:MAX(IF(product_status.value = <enabled>, stock.is_salable, 0)). The parent row now stays in the index withis_salable = 0. Pinning to the default scope is deliberate — the index has no store dimension, so a store-level status override is not reflected, which is the same limitation the legacycataloginventory_stockindexer has.Verified end to end on a non-default stock, parent with 15 children:
is_salableeffectively unknown)is_salable = 011quantityof the parent2. Listings: drop the per-product salable children count
Magento\ConfigurableProduct\Model\Product\Type\Configurable::isSalable()memoises per sku, so a listing of N configurables pays NCOUNT(DISTINCT e.entity_id)queries joiningcatalog_product_super_link, the status attribute and the stock index. The question that COUNT answers — "does this parent have at least one enabled, in-stock child?" — is the aggregate the stock index already stores on the parent's own row, which the collection has already joined in asis_salable.The new
IsSalablePlugin(frontend only, next to the existingIsSalableOptionPlugin) answers from that loaded value and delegates to the core otherwise:is_salableon the product (PDP, repository-loaded product, quote item) →$proceed(). The bulk stock API is deliberately not used here: it answers fromcataloginventory_stock_statuswith a fallback tocataloginventory_stock_item, which is not equivalent to the core's COUNT when the legacy index is incomplete.$proceed(). Note this compares store ids rather than checkinggetStoreFilter()for null: in frontendUsedProductsWebsiteFilterstamps the current store on the product on everygetUsedProducts()call, and listing swatches run before the template asksisSaleable(), so a null check would disable the fast path on exactly the pages this targets.Throwable→$proceed(). An optimisation must not take a page down.Otherwise it returns what
AbstractType::isSalable()already computes fromstatus+is_salable, which is the cheap half the core runs before the COUNT.Measured
Category listing of 12 configurables, warm caches, FPC and block cache off:
The page total drops by exactly N — the fast path adds no query of its own. SPX over the same scenario:
Configurable::isSalablegoes from 12 calls / 17.5 ms (8.2 ms of it ingetSize()) to ~250 us; total function calls 101.2K → 70.5K.Validation