A data-loss case occurs when chunk-key contains decimal-column in MySQL 5.6 #1119

[wangzhanbing]

Issue

Related issue: #1119

That's is an issue, describing a data-loss case caused by decimal-type column in chunk-key.
We also describe how to reproduce the case and how to fix it.

Description

This PR solves the bug by adding cast to decimal-type-column in values-stmt\set-stmt\where-stmt
for example: If type of column (InstanceID ) is decimal(30, 0) , the corresponding set-stmt and where-stmt as follows:

# where stmt in insert-select/select/delete sql
# ---- before commit----
where (((`InstanceID` > _binary'2708201202204012300000012')) and ((`InstanceID` < _binary'2708201202204062200000015') or ((`InstanceID` = _binary'2708201202204062200000015'))))

# -- after commit ----
where (((`InstanceID` > cast(_binary'2708201202204012300000012' as decimal(30, 0)))) and ((`InstanceID` < cast(_binary'2708201202204062200000015' as decimal(30, 0))) or ((`InstanceID` = cast(_binary'2708201202204062200000015' as decimal(30, 0))))))

# set stmt in update sql
# ---- before commit ----
set `InstanceID`=_binary'2708201202204062200000015'

# -- after commit ----
set `InstanceID`=cast(_binary'2708201202204062200000015' as decimal(30, 0))

# values stmt in replace-sql
# ---- before commit ----
values(_binary'2708201202204062200000015')

# -- after commit ----
values(cast(_binary'2708201202204062200000015' as decimal(30, 0)))

In case this PR introduced Go code changes:

• [ ok ] contributed code is using same conventions as original code
• [ ok ] script/cibuild returns with no formatting errors, build errors or unit test errors.

[timvaillancourt]

@wangzhanbing thanks for this PR! I think this fix makes sense 👍

Is it possible to add a "localtest" for this scenario under the localtests/ subdirectory?

[wangzhanbing]

@wangzhanbing thanks for this PR! I think this fix makes sense 👍

Is it possible to add a "localtest" for this scenario under the localtests/ subdirectory?

OK， That's what I am doing

[timvaillancourt]

@wangzhanbing LGTM. Thanks for this contribution! 🙏

I see you're still running MySQL 5.6. A heads-up that 5.6 was recently removed from gh-ost CI jobs as it is now an end-of-life MySQL version. This means future changes to gh-ost are not tested or guaranteed to work on that version

[wangzhanbing]

one extra question:
do we meet some trouble on continuing to support for old version （5.5 5.6） ?

In my company, most of mysql-instances will still be 5.6 in next one or two years. maybe so do other companies.

Is there another more elegant solution than abandoning history supports ?

[wangzhanbing]

Can someone help me review the code? @rashiq @dm-2

[dm-2]

do we meet some trouble on continuing to support for old version （5.5 5.6） ?

In my company, most of mysql-instances will still be 5.6 in next one or two years. maybe so do other companies.

Is there another more elegant solution than abandoning history supports ?

We have decided to only develop and test against currently supported versions of MySQL as we're unable to support old versions indefinitely. We'll continue to accept contributed fixes for MySQL 5.6 for the time being, but will only test that these fixes work against 5.7 and 8.0. New features will only be tested against 5.7 and 8.0.

In a future release, we will be dropping support for MySQL 5.5 entirely (including code specific to 5.5).

Can someone help me review the code? @rashiq @dm-2

I will take a look this week 👍

[dm-2]

@wangzhanbing please could you take a look at the merge conflict? Thank you! 🙇