On Sun, Nov 2, 2014 at 3:27 PM, Michael Widenius <monty@mariadb.org> wrote:

...

...

This is the same order that keys are presented in SHOW CREATE TABLE

Pavel> Why is it safe to assume that keys have the same order on master and on slave?

Primary and Unique keys should always be ordered the same way on master and slave. This is handled by the 'sort_keys()' function that you can find at: sql/sql_table.cc, line 2770.

This function seem to sort only by "type" of the key. If several unique keys have the same "type" according to its classification then they will be in the "original order" as the comment in the function says. And the original order may be different.

The only case when this is not the case, is if you add an unique key on the slave that was not on the master and there is conflicts when you insert things because of this key.

However, if you do this, I would say that you are on your own.

...

...

To help engine writes we are constantly adding new test in the storage engine test suite, to make it easier for storage engine vendors to test that their storage engine follows the interface. We have now added a test to check for the correct behaviour for the case of INSERT ... ON DUPLICATE KEY.

Pavel> This is very interesting. Can you say which test it is and what Pavel> exactly does it test? Is it present in 10.0.14 or it's only at the Pavel> latest 10.0 branch?

I added a test for this to: mysql-test/t/insert.test

--echo # --echo # MDEV-5168: Ensure that we can disable duplicate key warnings --echo # from INSERT IGNORE --echo #

This test doesn't check which duplicate key is asserted to execute ON DUPLICATE KEY action.

Elena added another test for this in mysql-test/suite/storage_engine/insert_with_keys.test

See line 142, that start with: --let $create_definition = a $int_indexed_col UNIQUE KEY, b $int_indexed_col UNIQUE KEY, c $int_col

This test seem to check that UPDATE is executed on the correct duplicate key, but it's not executed during regular mtr run. How and when it's executed in MariaDB?

...

...

Do you have or use a storage engine that doesn't provide the expected value for the conflicting key ?

Pavel> I don't have a specific example of storage engine that detects Pavel> duplicate key in different order, but I'm not convinced and don't have Pavel> hard proof that such engine cannot exist and that InnoDB itself is not Pavel> susceptible to this problem. I can easily imagine though that even Pavel> given the particular order of keys in table definition the key Pavel> reported as having duplicate can depend on the way storage engine Pavel> stores those keys.

As key order is defined on the upper level, above InnoDB, this should be 100 % safe (at least for InnoDB).

For other store engines, this is safe for any engine that test for duplicate key in key order.

If we ever find a case where it's not safe with any storage engine that we deliver with MariaDB, we will fix the storage engine.

It's ok if they do thing differently so that they can detect duplicate key for other keys faster.

However, it has been required from all storage engines from start that if one calls 'handler->info()' to request which key was duplicated, they must return the first key.

If not, they are breaking the storage engine interface (both for MySQL and MariaDB) as there is no way the upper level can guarantee that things will work identically between storage engines.

Pavel> BTW, can you point to MySQL documentation and/or comment in the code Pavel> that indicates this requirement to storage engine implementation?

Most of the storage engine interface is not 100 % documented. This is one of the case. However, I wrote the code related to errkey and this is how it was supposed to work. In general if something is not documented, then it's up to the people that created the interface or are actively working on it to define how it should work.

What is important is that all storage engine should work the same way. If one returns something different compared to any of the existing engines then it's a bug.

In this case the implicit documentation is how MyISAM, Aria and InnoDB works. All of them returns the first conflicting key.

I'm sorry to say, but you've explained exactly the reason why this behavior is not guaranteed and can be changed at any time. Yes, I understand that when you wrote the interface you had some particular requirements in mind. But ultimately it's up to people actively working on the code (who I believe are counted by hundreds now) to decide whether to maintain the particular behavior or change it. And as nobody knows the original requirements and the reasons why they can't be changed, and when the behavior is not tested anywhere and doesn't have any documentation even in the weakest form of comments or asserts, the decision is usually very easy -- change at will, nobody should ever depend on implementation details. Even if you promise to fix storage engines that change this behavior, by the time you notice the change you'll likely won't be able to fix it (as e.g. in case of https://mariadb.atlassian.net/browse/MDEV-6916) or someone will already experience a data corruption. I'd strongly recommend to revert this change. Thank you, Pavel