Skip to content

Conversation

shlomi-noach
Copy link
Contributor

Storyline: #446

This PR will fix the scenario where a column is changes its case. Example:

create table gh_ost_test (
  id int auto_increment,
  c1 int not null default 0,
  c2 int not null default 0,
  primary key (id)
) auto_increment=1;

and

--alter="modify C2 int not null default 0"

leading to a C2 column in altered table.

Right now gh-ost does not detect c2 and C2 are the same.

initial commit: an integration test to catch the error (confirmed)

@shlomi-noach
Copy link
Contributor Author

shlomi-noach commented Jul 12, 2017

added tests confirms and reproduces the issue; test result is:

$ ./localtests/test.sh ^modify
Building
Testing: modify-change-case......ERROR modify-change-case: checksum mismatch
---
3	20	26
4	21	27
5	22	28
6	23	29
7	24	30
8	25	31
9	26	32
---
3	20	0
4	21	0
5	22	0
6	23	0
7	24	0
8	25	0
9	26	0
+ FAIL

@shlomi-noach
Copy link
Contributor Author

effae08 fixes the issue as follows:

  • if a column's name only changes upper/lower case, gh-ost identifies it to be same column in original and ghost tables
  • gh-ost will not pick a shared key where such a name change takes place. As an example, if your table has an id int primary key, you won't be able to modify ID bigint unless there's another unqiue key to be used.

We can potentially rediuce this shared key limitation, at the cost of more complexity. I think it's not worth it, for now, unless a strong case comes by.

@shlomi-noach shlomi-noach temporarily deployed to production/mysql_role=ghost_testing July 16, 2017 03:56 Inactive
@shlomi-noach
Copy link
Contributor Author

This is being tested right now.

@shlomi-noach shlomi-noach merged commit 94a325c into master Jul 19, 2017
@shlomi-noach shlomi-noach deleted the case-insensitive-columns branch July 19, 2017 12:59
@shlomi-noach
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants