Re: Problems around SchemaEditor._alter_field
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/ On Tuesday, May 9, 2017 at 3:24:46 PM UTC+2, Florian Apolloner wrote: > > 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 > -- 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 post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3562f71f-4298-454e-832f-22c6324ae4e0%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: Problems around SchemaEditor._alter_field
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 -- 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 post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/501dd0f8-caf1-4aae-b56f-00a46768d41c%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Re: Problems around SchemaEditor._alter_field
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 On 05/09/2017 09:23 PM, charettes wrote: > 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 > > Le mardi 9 mai 2017 09:24:46 UTC-4, Florian Apolloner a écrit : >> >> 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 >> > -- 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 post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f69a6590-f882-8e1b-230b-117b60a4fc73%40markusholtermann.eu. For more options, visit https://groups.google.com/d/optout.
Re: Problems around SchemaEditor._alter_field
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 Le mardi 9 mai 2017 09:24:46 UTC-4, Florian Apolloner a écrit : > > 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 > -- 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 post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9c5cc446-bed9-40c2-a938-60e2cc45a96d%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Problems around SchemaEditor._alter_field
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 -- 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 post to this group, send email to django-developers@googlegroups.com. Visit this group at https://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/692f0912-92a9-4c6f-82ad-5a83bf559bf9%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
