Magento2: Missing unique key on eav_attribute_option_value table

[tzyganu]

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>

[m2-assistant[bot]]

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 label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: 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 branch

  Details

  - Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced 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.

[magento-engcom-team]

: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._

[magento-engcom-team]

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):

• 502146f7f2d086b3a1beaea41de3edc488f78b3c

The fix will be available with the upcoming 2.3.4 release.

[stkrelax]

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 ?

[ihor-sviziev]

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.

[tzyganu]

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>

[ihor-sviziev]

@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

[tzyganu]

@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.

[ihor-sviziev]

@tzyganu anyway, I created PR with the same changes, so we'll see when someone from maintainers will approve / reject it.

[dfelton]

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.

[ihor-sviziev]

@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?

[ihor-sviziev]

@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

[sidolov]

Hi @ihor-sviziev , it makes sense, I have changed the priority, thank you for the input!