skeema / tengo

Go La Tengo: a MySQL automation library
Apache License 2.0
27 stars 18 forks source link

Column reordering: only move each col once per ALTER #10

Closed evanelias closed 6 years ago

evanelias commented 6 years ago

As described in https://github.com/skeema/skeema/issues/36#issuecomment-418090748, there are column-reordering situations in which Table.Diff() may attempt to reposition the same col multiple times, generating illegal SQL. One such case is covered by a new test in 1272a54a311; this hasn't been merged to master yet since the test currently fails.

The root cause is the current algorithm for handling column reorders doesn't guarantee that a column is only moved once. A different algorithm entirely should be used here. I believe this approach would provide the right guarantee:

In theory, the optimal approach (in terms of minimum number of MODIFY COLUMN clauses) would also break ties in the first step by examining the number of other non-reorder modifications in each tied longest-subsequence candidate, to minimize the number of clauses generated in the third step. But that may wait until a future revision.

evanelias commented 6 years ago

Main issue fixed in bca198ed45475, and verified with new test coverage in 1272a54a311a and a41ba075709a8ef4.

Punting on the optimal tie-breaker logic until a future release, since this doesn't actually impact functionality and would greatly complicate the algorithm.