Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #27321 -- Added detection for table case name sensitivity on MySQL #7607

Merged
merged 1 commit into from
Nov 28, 2016

Conversation

adamchainz
Copy link
Sponsor Member

@adamchainz adamchainz commented Nov 23, 2016

It depends on the value of the lower_case_table_names setting, plus the case sensitivity of the filesystem that the table is on.

@adamchainz
Copy link
Sponsor Member Author

Ticket

@adamchainz
Copy link
Sponsor Member Author

This works but it's not strictly true. The description for the feature is:

Does the backend consider quoted identifiers with different casing to be equal?

MySQL is not case sensitive for many kinds of quotable identifiers, as per the docs:

Column, index, stored routine, and event names are not case sensitive on any platform, nor are column aliases.

It's just table names that are case sensitive, depending on the filesystem.

In practice Django is only using ignores_quoted_identifier_case for table name case sensitivity. I suggest renaming it to ignores_table_name_case - is this acceptable since it's not part of the public API?

@cached_property
def ignores_quoted_identifier_case(self):
"""
See https://dev.mysql.com/doc/refman/5.7/en/identifier-case-sensitivity.html
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omit 5.7 in the URL, although this URL is the first result for searching "LOWER_CASE_TABLE_NAMES MySQL" so maybe there's not much value?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know you could skip the version number from the url's, nice.

Maybe there isn't much value, but I hope it's useful for review at least

@timgraham
Copy link
Member

Not sure about renaming the feature (added in c2e62fd). Any opinion @charettes?

@charettes
Copy link
Member

Since the flag is only part of 1.10 and describes more accurately the checks we perform against it I wouldn't be opposed to rename it.

@adamchainz
Copy link
Sponsor Member Author

Should I add a deprecation shim or are database backends too internal to be subject to the deprecation policy?

@charettes
Copy link
Member

@adamchainz I don't think a shim is required. If a third party database backend was relying on this flag the tests for this feature will fail and they'll be pointed in the right direction. What do you think @manfre?

@timgraham
Copy link
Member

We could add the change in the "Database backend API" section of the backwards-incompatibel changes in the release notes. That said, a Google search for ignores_quoted_identifier_case doesn't give any results showing that this feature is used in backends outside of Django.

@adamchainz adamchainz changed the title Fixed #27321 -- Added detection for the ignores_quoted_identifier_case database feature on MySQL Fixed #27321 -- Added detection for table case name sensitivity on MySQL Nov 26, 2016
@@ -240,6 +240,9 @@ Database backends
* Added the :setting:`TEST['TEMPLATE'] <TEST_TEMPLATE>` setting to let
PostgreSQL users specify a template for creating the test database.

* Renamed the ``ignores_quoted_identifier_case`` feature to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes in "Backwards-incompatible changes" not "MInor features".

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops thx.

It depends on the value of the `lower_case_table_names` setting, plus the case sensitivity of the filesystem that the table is on.
@manfre
Copy link
Contributor

manfre commented Nov 29, 2016

A bit late, but the release notes are plenty for the rename, especially since it doesn't appear that it is in use (at least for the open sourced backends).

@adamchainz
Copy link
Sponsor Member Author

Thx @manfre

@adamchainz adamchainz deleted the ticket_27321 branch August 28, 2024 09:53
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.

4 participants