fix issue #3704 regarding integer attribute values being cast to decimal #3705
Conversation
| { | ||
| $select = implode(' UNION ALL ', $selects); | ||
| } | ||
| else $select = $selects; |
There was a problem hiding this comment.
please fix code style:
if (..) {
} else {
}
|
@digitalpianism Thank you for your contribution. Please accept the contributors license agreement so this PR can be further processed. This can be done by clicking the "Details" link next to the "license/cla" check below. |
|
@antonkril I've provided a quick test to show how to replicate the bug in the issue: #3704 |
|
@davidalger I've done that about 3 different times on 2 different browsers but it doesn't seem to work even if it tells me "thanks for signing CLA, we're redirecting you to github" I've tried revoking the OAuth access from my account but still doesn't work |
|
@davidalger ok done, the email address I'm using to commit code had not been verified my bad ;) |
|
@digitalpianism Thank you for so deep investigation of the issue and provided fix. But please pay attention that Magento\Eav\Model\ResourceModel\Helper::getLoadAttributesSelectGroups() is also used in Magento\Eav\Model\Entity\AbstractEntity so it should be also modified as Magento\Eav\Model\Entity\Collection\AbstractCollection. However this fix may decrease performance as count of requests to DB will be increased. To improve performance Magento\Eav\Model\ResourceModel\Helper::getLoadAttributesSelectGroups() may combine selects for varchar, text and datetime in one group. Also as issue appears only in case when only decimal and integers attributes are present due to MySQL casting logic for union statement fix without performance degradation may be provided. For this we need to use "CAST(value AS char)" expression. |
|
@vkublytskyi thanks for your comment, I'm about to make another commit regarding the I agree with you regarding the performance issue, my main goal was to fix the bug, I did not focus on how to refactor the framework to get that bug fix optimized, I guess there's people more experienced than me to do so. |
|
@digitalpianism Thanks for your contribution and for catching this bug. |
|
The issue is being processed now. |
…ery-core [Owls] Product associated to website & MFTF migration to core
No description provided.