Thanks for the feedback.

Aymeric, yes, I left out modification until I knew there was some interest
as that code seemed more impenetrable to me than the field addition. I've
added this now, does it seem like the right approach? I've tested it on
everything but Oracle and it seems to work as I'd expect. If it generally
looks good (and when/if the ticket
<https://code.djangoproject.com/ticket/31777> is accepted) I'll write up
tests and docs... I'm not sure how tests are going to work though. Since
the collation names are different I think I need to write some interesting
migrations that depend on the database vendor... do we do anything like
this anywhere else?

Adam, I want to go with a followup ticket for these reasons:

1. I'm new to the schema migration code and I don't want to take on too
much at once.
2. I haven't used MySQL in a long time so I'm not sure how to test it.
3. The code here works across all backends, I'm not sure how it'd work for
the charset. Would we add a new kwarg to Char/TextField just for MySQL? I
think there are a few more questions to answer before tackling it, but I
agree that if we add collations, we should add charsets for MySQL too, as
they're so closely related.

Tom

On Sun, 19 Jul 2020 at 09:55, Adam Johnson <m...@adamj.eu> wrote:

> Yes I'd also like to lend my support.
>
> In MySQL (at least) columns also have a charset property that goes
> hand-in-hand with which collations are available. I know it expands the
> scope, but I think it would be good to get that in here. Perhaps it could
> be a follow-up ticket though.
>
> On Sun, 19 Jul 2020 at 08:28, Aymeric Augustin <
> aymeric.augus...@polytechnique.org> wrote:
>
>> Hello Tom,
>>
>> Just wanted to give you some encouragement here, as you've been
>> monologuing for two months ;-) The PR looks promising!
>>
>> Regarding migrations, I'm not seeing how the collation for a column will
>> be set or unset in the database if you change it in the code e.g.:
>>
>> MySQL : ALTER TABLE ... MODIFY ... VARCHAR(...) COLLATE ...
>> PostgreSQL : ALTER TABLE ... ALTER COLUMN ... TYPE varchar(...) COLLATE
>> ...
>>
>> I think this needs to be in scope, else it will be hard for maintainers
>> of existing projects to take advantage of this new feature.
>>
>> --
>> Aymeric.
>>
>>
>>
>> On 18 Jul 2020, at 13:29, Tom Carrick <t...@carrick.eu> wrote:
>>
>> I've written a proof of concept:
>> https://github.com/django/django/pull/13207
>>
>> The diff is quite small, though I'm not sure if there's something I'm
>> doing wrong - this is my first foray into schema migration internals so I'm
>> learning as I read the codebase.
>>
>> Tom
>>
>> On Fri, 17 Jul 2020 at 13:55, Tom Carrick <t...@carrick.eu> wrote:
>>
>>> I've had a deeper look at this now and think I have an API proposal.
>>> First, the state of supported vendors:
>>>
>>> 1. All vendors support adding a collation to text/varchar fields.
>>> 2. The syntax is more or less the same.
>>> 3. However, the collation names themselves are different.
>>> 4. PostgreSQL is the only vendor that allows creating custom collations
>>> at runtime.
>>>
>>> So I'm thinking we add a new `collation` parameter to `CharField` and
>>> `TextField`, that simply takes a string of the collation ID. I'm not quite
>>> sure on the implementation as I don't know the ORM that well, but my naive
>>> approach would be to just add a new format string to the `data_types` dict
>>> that is calculated during the field __init__(), either an empty string to
>>> use the default collation, or e.g. ' collate <collation_id>'. There's may
>>> well be a better approach.
>>>
>>> By using this - because the collation names are not the same across
>>> vendors - the user is saying "I'm okay with this only working on one
>>> database vendor", so there should be a warning in the docs. There is
>>> perhaps some scope in the future to make this take a callable that can
>>> figure out the collation per-database. This would be useful for getting
>>> case-insensitive lookups working across all backends, for example. But I
>>> want to keep that out of the scope because it's some extra work and I'm not
>>> sure on the implementation.
>>>
>>> Another downside is that people like to use CharField as a base class
>>> for other column types that might not support collations, but I think this
>>> should be in the user's hands to make sure they aren't doing that.
>>>
>>> We should also add a `CreateCollation` operation for Postgres, similar
>>> to the `CreateExtension` operation that currently exists. If the user wants
>>> to use a custom collation they must create it first, similar to using
>>> extensions currently.
>>>
>>> The advantage to this is that users can use collations without having to
>>> make SQL migrations, which I think would be nice. The really nice thing is
>>> the ability to have case-insensitive lookups that work across all database
>>> vendors, rather than only Postgres as it currently is. And as I mentioned
>>> in the previous message, Postgres is discouraging our current method of
>>> using the citext extension in favour of this approach.
>>>
>>> Cheers,
>>> Tom
>>>
>>> On Wed, 27 May 2020 at 14:17, Tom Carrick <t...@carrick.eu> wrote:
>>>
>>>> I think it would be useful to be able to create collations and use them
>>>> with model fields. The motivation here is mostly that citext is somewhat
>>>> discouraged <https://www.postgresql.org/docs/12/citext.html> in favour
>>>> of creating a collation. I'm not sure how this would work on other
>>>> databases,or what the API would look like. I didn't see a ticket for it or
>>>> another discussion, but maybe there is one.
>>>>
>>>> Perhaps a Collation class would make sense, and it could be added to a
>>>> Field with a new parameter. I'm not sure how easy it would be to only
>>>> create a collation once, so perhaps it would need a CreateCollation
>>>> migration as well, but I know very little about the internals of 
>>>> migrations.
>>>>
>>>> Cheers,
>>>> Tom
>>>>
>>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/CAHoz%3DMbyJ1qLNCxmTh6JdsP-e1UoYwdWJm8KL3q8MgRAQaD5sA%40mail.gmail.com
>> <https://groups.google.com/d/msgid/django-developers/CAHoz%3DMbyJ1qLNCxmTh6JdsP-e1UoYwdWJm8KL3q8MgRAQaD5sA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/7441F0D5-B658-4FC5-BA13-C183C470A3CB%40polytechnique.org
>> <https://groups.google.com/d/msgid/django-developers/7441F0D5-B658-4FC5-BA13-C183C470A3CB%40polytechnique.org?utm_medium=email&utm_source=footer>
>> .
>>
>
>
> --
> Adam
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAMyDDM1pmp7Cw5igDaOf6KbHOYWY_HdgE550ia8W7X_9iSRWRQ%40mail.gmail.com
> <https://groups.google.com/d/msgid/django-developers/CAMyDDM1pmp7Cw5igDaOf6KbHOYWY_HdgE550ia8W7X_9iSRWRQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAHoz%3DMbD_hpTLObeG3QdMoid0zi1gfxODCe7CfSQ1tspaJ7%3Dsw%40mail.gmail.com.

Reply via email to