Fix length of index names for a create table statement #133
Conversation
Fix index names Fix unit tests
jeroenpf
changed the title
jeroenpf
changed the title
|
@bgrgicak Could you have a look at this one? |
|
Thank you @jeroenpf for contributing these fixes! |
| * @return string | ||
| */ | ||
| private function generate_index_name( $table, $original_index_name ) { | ||
| // Strip the occurrences of 2 or more consecutive underscores to allow easier splitting on __ later. |
There was a problem hiding this comment.
@jeroenpf it looks like this makes it so we cannot derive the original mysql index name from sqlite index name because we cannot tell which underscores used to be double-underscores.
So import/export operations are not symmetrical/reversible. Do we care about that? Offhand, I think maybe we should.
There was a problem hiding this comment.
@brandonpayton for this reason i strip out double underscores from the table name alone. Later we can split on the first occurrence of double underscores. The idea was that it does not matter if the index name itself has double underscores so they would be preserved. Maybe I am overlooking something?
Example:
table__name and some_index__name becomes table_name__some_index__name and later we split on the first occurrence of __ which which leaves us with [table_name, some_index__name].
I do however see one error in the explode logic that is used to get the index name; i should have passed in a limit so that it only separates on the first occurrence of __. I will create a new PR to address this.
There was a problem hiding this comment.
for this reason i strip out double underscores from the table name alone.
@jeroenpf thank you for pointing that out. I had missed that this was just for the separator between table name and index. The naming scheme makes sense to me now.
See: #133 (comment) When creating new tables in SQLite, original index names are prefixed with the table name. This is because unlike MySQL, SQLite does not allow identical index names across the database. Adding a table prefix ensures the index names are unique. When running a `show create table` query, we restore the original index name by removing the table prefix. We split on double underscores but we did not take into account that the original index name can also contain `__`. This PR fixes an issue where we explode without a limit. We only want to explode on the first occurrence of `__`. Unit tests have been added to ensure this behavior.
Fixes: #124
This PR introduces some changes to how we determine the name of an index. SQLite requires that each index is uniquely named across the database (not just the table). This causes issues when multiple tables have the same key names when importing from MySQL. To avoid these issues, the index name is prefixed with the table name when it is created. This however results in the indexes having names that are too long for MySQL so when we run
show create tablethe create statements have these prefixed keys that are too long.To solve this, we keep adding the table name as a prefix but we remove it when we generate the create statement. Furthermore, we ensure that the table name itself has no double underscores in it so we can be sure that we can split on
__safely. The result is that each create table statement will have the original index name and prevents it from being too long.