Problems around SchemaEditor._alter_field

[Florian Apolloner]

I am currently trying to (again?) write a database backend for Informix. So far so good, but I am running into one major issue: All of Django's other databases support changing null/default properties via "ALTER TABLE ALTER col DROP NULL/DEFAULT" or what not. In Informix I can only use "ALTER TABLE MODIFY" and rewrite the column completely [1]. This means that I would need more information in [2] (which I initially tried in https://github.com/django/django/commit/2b3a9414570af623853ca0f819c7d77d0511f22c before noticing that I need to repeat the whole column declaration). I am looking into options to extend _alter_field [3] in a way that I can either add a database feature that says something along the line of: "field property changes need full remake" and then call into _alter_column_type_sql directly, or at least factor the (for me) annoying parts out into _alter_column_properties.

Which of the two would you prefer?

Thanks,
Florian

[1] https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.sqls.doc/ids_sqs_0290.htm
[2] https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L39-L42
[3] https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L581-L640

[charettes]

Hey Florian,

I think both options are viable.

The main issue with a feature flag would be that it isn't tested by our CI. I guess we

could make it True on SQLite and have it call remake_table to have some tests rely

on it or mock it to True in tests and make sure everything goes well.

In all cases breaking the humongous _alter_field method into smaller ones can't

hurt.

Cheers,

Simon

[Markus Holtermann]

Agreed. As mentioned on IRC, _alter_field() should really be cleaned up.
It's also private API and only called from alter_field() I think. And as
long alter_field()'s API stays backwards compatible you're pretty much
free to do what you need with _alter_field().

/Markus

[Mariusz Felisiak]

I agree that _alter_field should be refactored, because currently is unmaintainable or at least really hard to maintain. I checked and some of the official 3rd-party database backends [1] use this private API i.e. django-mssql [2], django-firebird [3]. Maybe we should take that into account.

[1]: https://docs.djangoproject.com/en/1.11/ref/databases/#using-a-3rd-party-database-backend
[2]: https://bitbucket.org/Manfre/django-mssql/src/d44721ba17acf95da89f06bd7270dabc1cd33deb/sqlserver_ado/schema.py?at=master&fileviewer=file-view-default#schema.py-152
[3]: https://github.com/maxirobaina/django-firebird/blob/master/firebird/schema.py#L112

[Florian Apolloner]

I've pushed an initial PR [1], sadly I had to change one of the signatures. But I think this is an okayish change for 2.0 given that it is internal API anyways.

Cheers,
Florian

https://github.com/django/django/pull/8487/