#40704: Prevent full table scan in sales order child collections for orders without an id - #41106
Open
lbajsarowicz wants to merge 1 commit into
Conversation
…rders without id setOrderFilter() marked the collection as loaded with zero records instead of adding a condition to the select. Anything that resets the loaded state afterwards - clear(), _reset() - left the select unfiltered, so the next load() or getSize() scanned the whole sales_invoice, sales_shipment or sales_creditmemo table. Add a never-matching condition on the parent id field as well, so a reset cannot leak into a full table scan.
|
Hi @lbajsarowicz. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
Contributor
Author
|
@magento run all tests |
|
@magento run all tests |
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.
Description
Magento\Sales\Model\ResourceModel\Order\Collection\AbstractCollection::setOrderFilter()handles an order without an ID by faking the loaded state instead of constraining the select:No condition is added to
_select, so the select still matches every row ofsales_invoice/sales_shipment/sales_creditmemo. As long as nothing touches the loaded state, the collection behaves correctly. But any reset of that state re-opens the unfiltered select:Magento\Framework\Data\Collection::clear()sets_isCollectionLoaded = falseand_totalRecords = nullMagento\Framework\Data\Collection\AbstractDb::_reset()sets_isCollectionLoaded = falseAfter either of those, the next
load()orgetSize()runs against the whole table — a full table scan returning unrelated invoices, shipments or credit memos for an order that has none.Fixed Issues
Reported by @ln8711 in #40704 (comment) while reviewing #40731.
Related to #40704.
Manual testing scenarios
Before:
getSize()reports the total number of invoices in the table andgetItems()returns them.After: both report
0, and the emitted SQL carriesparent_id = 0.Questions or comments
The
_setIsLoaded(true)shortcut is kept on purpose — it keeps the common "new order, no children yet" path free of any query during order placement. The added filter only guarantees that a reset cannot degrade into a table scan.Covered by a new integration test in
Magento\Sales\Model\ResourceModel\Order\Collection\AbstractCollectionTest.