#26708 Fix: ORDER BY has two similar conditions in the SQL query

[vasilii-b]

Description (*)

The PR fixes two similar order by condition in the SQL query of the app/code/Magento/Catalog/Block/Product/ListProduct.php

Related Pull Requests

#26754 - Original PR (the author gave up on the PR #26754 (comment))
#11473 - PR that introduced the issue

Fixed Issues (if relevant)

1. ORDER BY has two similar conditions #26708: ORDER BY has two similar conditions

Manual testing scenarios (*)

Please see #26708

Questions or comments

@lenaorobei Could you please follow this PR, as you already did the review of the closed one #11473.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @vasilii-b. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[ihor-sviziev]

It seems to me related to #26934

[eduard13]

@ihor-sviziev in my opinion isn't exactly the same, but I'd additionally check if this affects somehow the product listing with Elastic Search enabled. Moreover, I see that there are couple of failing tests related to ElasticSearch.

[vasilii-b]

Hi @eduard13,
Could you please advise on the failed tests? The failed functional tests seem not related to the changes in this PR. Also, the static failed test points to a part of the code that was not changed in the scope of this PR.

Thank you!

[eduard13]

@magento run all tests

[eduard13]

The failing tests aren't related to these changes.
Thank you for your collaboration.

[magento-engcom-team]

Hi @eduard13, thank you for the review.
ENGCOM-7144 has been created to process this Pull Request

[engcom-Alfa]

✔️ QA Passed

Before:

After:

[engcom-Echo]

If possible not to use '// phpcs:ignore Magento2.Performance.ForeachArrayMerge'

 $arraysItemsToMerge = [];
 foreach ($this->_getProductCollection() as $item) {
     $arraysItemsToMerge[] = $item->getIdentities();
 }
 $identities = array_merge($identities, ...$arraysItemsToMerge);

[vasilii-b]

Hi @engcom-Echo,
Should we leave this part of the code as it was? We did not change it but because of this, there is a failed test. How we should proceed here? Any suggestions?
Thanks!

[engcom-Echo]

Using 'array_merge' function in a 'for/foreach/while'.
It's a very bad practice because it's a performance killer (especially in memory).

I think if we can change it for the better, it will be very good.

[vasilii-b]

@engcom-Echo why should we change it in the scope of this PR if we didn't introduce it or modified it in the scope of this PR?

[engcom-Echo]

@eduard13

[eduard13]

Hi @engcom-Echo, I have briefly checked, and looks like we aren't able replace it with anything. I was checking maybe we could use + operator here, but it may bring some unexpected behavior.
The main reason, being that + will remove the duplicated array values, in comparison with array_merge which just pushes the new array at the end of another one.

So I'd suggest to proceed with another investigation (separately), of how and where exactly these identities are used, and see if we are able to remove the duplicated array values.

What do you think?
Thank you.

[engcom-Echo]

So I'd suggest to proceed with another investigation (separately)....

Here I agree with you. Thanks for the discussion.

[engcom-Echo]

Failed functional tests not related to the changes in this PR

[m2-assistant]

Hi @vasilii-b, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.