magento2
magento2 copied to clipboard
Missing unique key on eav_attribute_option_value table
This is similar to #24581 but for the table eav_attribute_option_value
Summary (*)
The table eav_attribute_option_value
does not have a unique index on the column pair (option_id
, store_id
).
There is no reason for an option to have 2 values for the same store so this should be restricted.
Also layered navigation disappears on frontend.
Relates issues: https://github.com/magento/magento2/issues/24581 https://github.com/magento/magento2/issues/24718
Examples (*)
Same as #24581, if by any faulty code an option gets 2 values for the same store view this causes a lot of issues when editing product / attributes / maybe others.
Proposed solution
add a unique key on the column pair (option_id
, store_id
) on the table eav_attribute_option_value
<table name="eav_attribute_option_value">
......
<constraint xsi:type="unique" referenceId="EAV_ATTRIBUTE_OPTION_VALUE_OPTION_ID_STORE_ID_UNIQUE">
<column name="store_id"/>
<column name="option_id"/>
</constraint>
</table>
Hi @engcom-Charlie. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:
-
[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the labelIssue: Format is valid
will be added to the issue automatically. Please, edit issue description if needed, until labelIssue: Format is valid
appears. -
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add
Issue: Clear Description
label to the issue by yourself. -
[ ] 3. Add
Component: XXXXX
label(s) to the ticket, indicating the components it may be related to. -
[ ] 4. Verify that the issue is reproducible on
2.3-develop
branchDetails
- Add the comment@magento give me 2.3-develop instance
to deploy test instance on Magento infrastructure.
- If the issue is reproducible on2.3-develop
branch, please, add the labelReproduced on 2.3.x
.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here! -
[ ] 5. Add label
Issue: Confirmed
once verification is complete. -
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.
:white_check_mark: Confirmed by @engcom-Charlie
Thank you for verifying the issue. Based on the provided information internal tickets MC-20408
were created
Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.
Hi @tzyganu. Thank you for your report. The issue has been fixed in magento/magento2#24720 by @UncleTioma in 2.3-develop branch Related commit(s):
The fix will be available with the upcoming 2.3.4 release.
Reopening... either still missing or fix deleted.
When this is available in upcoming Release 2.3.4, why am I not seeing this fix in module version magento/module-eav 102.0.5 which is used in magento 2.3.5.-p1 ?
Where is it gone ?
Fix for this issue was reverted in https://github.com/magento/magento2/commit/e1958fece28f2726d2d7cb6a48b1b25cb5b3862e
Explanation from @lenaorobei in https://github.com/magento/magento2/issues/28703#issuecomment-645357575
internal ticket says that it was reverted due to backward incompatible change that was not approved.
I re-opened this issue. Just checked - fix for this issue wasn't added to 2.3.4 and 2.3.5 releases.
I don't understand where this is breaking BC, but anyway, the beauty of the declarative schema is that it does not matter if it's in the core or not.
If you have this problem, just add the constraint in one of your custom modules and it will work. And if this reaches the core it will still work even if it's duplicated in your module
<constraint xsi:type="unique" referenceId="EAV_ATTRIBUTE_OPTION_VALUE_STORE_ID_OPTION_ID">
<column name="store_id"/>
<column name="option_id"/>
</constraint>
@tzyganu this is backward incompatible change in terms that some DBs could have duplicated data, and adding unique key might potentially fail.
Seems like there should be some migration that should be executed before adding this unique key, but not sure how to do that
@ihor-sviziev Thanks for the explanation. You are right... from the abstract code point of view.
But if there are duplicate values nothing works. You will get errors when retrieving attribute options. You can try it and see. I think it is completely safe to fix this.
@tzyganu anyway, I created PR with the same changes, so we'll see when someone from maintainers will approve / reject it.
this is backward incompatible change in terms that some DBs could have duplicated data, and adding unique key might potentially fail.
@ihor-sviziev:
I encountered this just today actually, while writing a Data patch I had made an assumption that such a key would exist and therefore was performing $connection->insertOnDuplicate($table, $data, $fields)
. Due to the lack of the UNIQUE
constraint however, duplicated rows of store_id
and option_id
combinations were created.
In any case, I'd also like to report that any store out there that does have such records in their database, already have a broken installation. As a collection object for the attribute's options will fail when \Magento\Framework\Data\Collection::addItem()
is invoked on a secondary record with the same store_id
and option_id
. This is because the query for the collection:
SELECT `main_table`.*, `sort_alpha_value`.`value`
FROM `eav_attribute_option` AS `main_table`
LEFT JOIN `eav_attribute_option_value` AS `sort_alpha_value`
ON sort_alpha_value.option_id = main_table.option_id
AND sort_alpha_value.store_id = 0
WHERE (`main_table`.`attribute_id` = 'REPLACE_THIS_WITH_ATTRIBUTE_ID_VALUE')
ORDER BY
main_table.sort_order ASC,
sort_alpha_value.value ASC;
Will return two records per option_id in this scenario. Example:
+-----------+--------------+------------+-------------------+
| option_id | attribute_id | sort_order | value |
+-----------+--------------+------------+-------------------+
| 5 | 872 | 10 | Black |
| 5 | 872 | 10 | Black |
+-----------+--------------+------------+-------------------+
Since option_id
is the unique identifier for the collection's item model object, an exception is thrown.
So... Long story short... while I technically agree that adding such a UNIQUE constraint breaks backwards incompatibility... any installation with such data, would have had to gone through some wild extra lengths to have their site continue to work with such junk data.
@tzyganu thank you for the suggested fix, applying to my project in 2.4.0.
@sidolov @gabrieldagama could we increase P3 -> P2, as we have basically the same issue for another table and it has higher priority - https://github.com/magento/magento2/issues/24581?
@gabrieldagama as we discussed in Slack - this is really terrible issue, it's causing not obvious issues. In my case it was caused by some custom module, and behavior was like was "I just pressed some button and whole website went down", but real issue came from NOT OBVIOUS missing unique key.
We have 2 the same issues for two tables: https://github.com/magento/magento2/issues/24581 - P2 S2 (as for me this is right priority) https://github.com/magento/magento2/issues/24718 - P3 S3
Also for them we have open PRs: https://github.com/magento/magento2/pull/28796 (in "to approval" status since end of July) https://github.com/magento/magento2/pull/30981 (in review currently, waiting for test results, but I'm ready to approve it)
Would be good to move them forward together
BTW one issue was already fixed in October 2019, but then fix was reverted https://github.com/magento/magento2/issues/24581#issuecomment-537933794
Hi @ihor-sviziev , it makes sense, I have changed the priority, thank you for the input!
Hi @engcom-November. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:
-
[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the labelIssue: Format is valid
will be added to the issue automatically. Please, edit issue description if needed, until labelIssue: Format is valid
appears. -
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add
Issue: Clear Description
label to the issue by yourself. -
[ ] 3. Add
Component: XXXXX
label(s) to the ticket, indicating the components it may be related to. -
[ ] 4. Verify that the issue is reproducible on
2.4-develop
branchDetails
- Add the comment@magento give me 2.4-develop instance
to deploy test instance on Magento infrastructure.
- If the issue is reproducible on2.4-develop
branch, please, add the labelReproduced on 2.4.x
.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here! -
[ ] 5. Add label
Issue: Confirmed
once verification is complete. -
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.
Verified the issue on latest Magento 2.4-develop instance and the issue is reproducible. Hence confirming this issue.
There is no restriction to add duplicate values for option_id. Adding duplicate values for option_id with same store_id is returning no error in eav_attribute_option_value DB table. Adding so is breaking Layered Navigation on Front end Search Results page. Hence confirming this issue.
Layered navigation disappeared on store front.
:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-6738 is successfully created for this GitHub issue.
:white_check_mark: Confirmed by @engcom-November. Thank you for verifying the issue.
Issue Available: @engcom-November, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.