Only convert table name to lower-case when installation forces it #12891

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@MPLew-is
Contributor
MPLew-is commented Jan 13, 2017 edited

Issue #12861

A lower_case_table_names setting of 2 does not mean that the table needs to be forced to lower-case.
This preserves case unless the lower_case_table_names setting is 1, implying that lower-case should be forced.

Signed-off-by: Mike Lewis mike@mplew.is

Before submitting pull request, please check that every commit:

  • Has proper Signed-Off-By
  • Has commit message which describes it
  • Is needed on it's own, if you have just minor fixes to previous commits, you can squash them
  • Any new functionality is covered by tests

I left the last checkbox above blank because I have no idea if you all want tests for table renaming under different lower_case_table_names settings (currently, from your table testing file you only run your tests with lower_case_table_names=0). I can definitely add some tests to this PR if you want or need them.

@MPLew-is MPLew-is Only convert table name to lower-case when installation forces it
Issue #12861

A `lower_case_table_names` setting of `2` does not mean that the table needs to be forced to lower-case
This preserves case unless the `lower_case_table_names` setting is `1`, implying that lower-case should be forced

Signed-off-by: Mike Lewis <mike@mplew.is>
47781e4
@codecov-io

Current coverage is 54.21% (diff: 100%)

Merging #12891 into master will not change coverage

@@             master     #12891   diff @@
==========================================
  Files           465        465          
  Lines         69647      69647          
  Methods        2128       2128          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          37758      37758          
  Misses        31889      31889          
  Partials          0          0          

Powered by Codecov. Last update b40e9c1...47781e4

@MPLew-is
Contributor

And just to confirm, I've tested this code on installations with lower_case_table_names of 1 and 2, and confirmed that this change works on both. I can't speak for installations with a value of 0, but since the change is exceedingly minor, it should not affect the behavior with regards to those installations at all.

@nijel nijel self-assigned this Jan 18, 2017
@nijel nijel added a commit that referenced this pull request Jan 18, 2017
@nijel nijel Changelog entry for #12861 and #12891
Signed-off-by: Michal Čihař <michal@cihar.com>
f1aefb1
@nijel nijel added this to the 4.6.6 milestone Jan 18, 2017
@nijel
Member
nijel commented Jan 18, 2017

Thanks! I've rather merged your fix to QA_4_6 for inclusion in upcoming 4.6.6 release, it has landed as 3bf78af.

@nijel nijel closed this Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment