Problems around SchemaEditor._alter_field

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

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

--
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Problems around SchemaEditor._alter_field

Simon Charette
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 <a href="https://github.com/django/django/commit/2b3a9414570af623853ca0f819c7d77d0511f22c" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fcommit%2F2b3a9414570af623853ca0f819c7d77d0511f22c\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFxhhUrPbNk8qje-Z0VS2nr2Cu6lQ&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fcommit%2F2b3a9414570af623853ca0f819c7d77d0511f22c\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFxhhUrPbNk8qje-Z0VS2nr2Cu6lQ&#39;;return true;">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] <a href="https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.sqls.doc/ids_sqs_0290.htm" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fwww.ibm.com%2Fsupport%2Fknowledgecenter%2Fen%2FSSGU8G_12.1.0%2Fcom.ibm.sqls.doc%2Fids_sqs_0290.htm\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEc0XDudG5zSGlV8LY85mCE21HgFw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fwww.ibm.com%2Fsupport%2Fknowledgecenter%2Fen%2FSSGU8G_12.1.0%2Fcom.ibm.sqls.doc%2Fids_sqs_0290.htm\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEc0XDudG5zSGlV8LY85mCE21HgFw&#39;;return true;">https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.sqls.doc/ids_sqs_0290.htm
[2] <a href="https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L39-L42" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L39-L42\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNETocLE0K4ThlWiWVsSl0c_xlSOhw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L39-L42\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNETocLE0K4ThlWiWVsSl0c_xlSOhw&#39;;return true;">https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L39-L42
[3] <a href="https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L581-L640" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L581-L640\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHJWPdbaa35VxOc2fP6pthoTr12yA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L581-L640\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHJWPdbaa35VxOc2fP6pthoTr12yA&#39;;return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Problems around SchemaEditor._alter_field

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

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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Problems around SchemaEditor._alter_field

Mariusz Felisiak
In reply to this post by Florian Apolloner
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 [hidden email].
To post to this group, send email to [hidden email].
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.
Reply | Threaded
Open this post in threaded view
|

Re: Problems around SchemaEditor._alter_field

Florian Apolloner
In reply to this post by 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/

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 <a href="https://github.com/django/django/commit/2b3a9414570af623853ca0f819c7d77d0511f22c" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fcommit%2F2b3a9414570af623853ca0f819c7d77d0511f22c\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFxhhUrPbNk8qje-Z0VS2nr2Cu6lQ&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fcommit%2F2b3a9414570af623853ca0f819c7d77d0511f22c\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNFxhhUrPbNk8qje-Z0VS2nr2Cu6lQ&#39;;return true;">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] <a href="https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.sqls.doc/ids_sqs_0290.htm" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fwww.ibm.com%2Fsupport%2Fknowledgecenter%2Fen%2FSSGU8G_12.1.0%2Fcom.ibm.sqls.doc%2Fids_sqs_0290.htm\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEc0XDudG5zSGlV8LY85mCE21HgFw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fwww.ibm.com%2Fsupport%2Fknowledgecenter%2Fen%2FSSGU8G_12.1.0%2Fcom.ibm.sqls.doc%2Fids_sqs_0290.htm\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEc0XDudG5zSGlV8LY85mCE21HgFw&#39;;return true;">https://www.ibm.com/support/knowledgecenter/en/SSGU8G_12.1.0/com.ibm.sqls.doc/ids_sqs_0290.htm
[2] <a href="https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L39-L42" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L39-L42\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNETocLE0K4ThlWiWVsSl0c_xlSOhw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L39-L42\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNETocLE0K4ThlWiWVsSl0c_xlSOhw&#39;;return true;">https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L39-L42
[3] <a href="https://github.com/django/django/blob/837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc/django/db/backends/base/schema.py#L581-L640" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L581-L640\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHJWPdbaa35VxOc2fP6pthoTr12yA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Fdjango%2Fdjango%2Fblob%2F837259a63ff03fbc0ca2bc2999a6fbc8c6c40bcc%2Fdjango%2Fdb%2Fbackends%2Fbase%2Fschema.py%23L581-L640\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNHJWPdbaa35VxOc2fP6pthoTr12yA&#39;;return true;">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 [hidden email].
To post to this group, send email to [hidden email].
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.