Re: Problems around SchemaEditor._alter_field

2017-05-11 Thread 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 
> 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

2017-05-10 Thread 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=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

2017-05-09 Thread 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 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

2017-05-09 Thread 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

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

2017-05-09 Thread 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 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.