django-firebird backend: migrations and NULL fields

[maxi]

Hi,

I'm trying to implement the new schema migration of django 1.7 for django-firebird backend.

In column_sql method, when the field can be null, in firebird, is not necessary add the NULL keyword. So to change this behavior I need override the entire column_sql method just to change one line:

        if null:

            sql += " NULL"    <-- Here

        else:

            sql += " NOT NULL"

I just need:

        if not null:

            sql += " NOT NULL"

Is the correct approach rewrite the column_sql method? Are there other approach?

Regards.

[Andrew Godwin]

Hi,

That's currently the only approach I'm afraid - there's an open issue (raised by Shai Berger I believe) that column_sql should be broken down into more component pieces so it can be more easily changed, but that's a difficult task to do and not something I want to go and change completely as we're trying to release 1.7.

What we could do is change those NULL and NOT NULL strings to be defined on the schema editor class itself, and then you could set sql_null_suffix = "" or similar - do you think that would be a reasonable approach?

Andrew

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/e9f24444-a5bf-4e13-b7c1-0a08abd18a4a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Jeremy Dunck]

How about adding a flag to Operations? implied_null, perhaps.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1upTtLEmGjz5f2vUBV6kEFDqdiyRyz4GhM3i3WbSyot0aw%40mail.gmail.com.

[Andrew Godwin]

Hmm, I'm not sure about adding more features, but it might be cleaner...

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAM0i3f6Jw6Bs8FEeTmU7wVwA3SQKwcruKqtTReCa_-GrxbZmww%40mail.gmail.com.

[Aymeric Augustin]

For what it’s worth, I recently added a feature to add “FROM DUAL” where Oracle needs it.

This looks roughly similar. Maybe we could use the same approach. (I didn’t look closely.)

-- 

Aymeric.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1urWNpLhc4_1wc-dFVSJJDkdWvud0UaDnCc11suTE986vw%40mail.gmail.com.

[Andrew Godwin]

Let's go with a feature for this then. 

Maxi, did you want to submit a patch, or do you want me to add it at some point next week!

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/8F3A2FC4-6CA5-406A-90F5-63E1E4A6945E%40polytechnique.org.

[Maximiliano Robaina]

El viernes, 16 de mayo de 2014 09:49:15 UTC-3, Andrew Godwin escribió:

Let's go with a feature for this then. 

Maxi, did you want to submit a patch, or do you want me to add it at some point next week!

Andrew

I'll appreciate if you can add it.

BTW, are there other areas that need some work, for instance, add_field method [1], if I try to delete a default value on a column that does not have a previous default value definition I get an error on firebird, for which reason, I need to check if that field has got defined a default value before.

Then I need to rewrite add_field method too.

I think, for not to delay django 1.7 release, I'll do some workaround and I'll try to send you a pull request or a patch with some proposal later.

[1] https://github.com/django/django/blob/master/django/db/backends/schema.py#L401

Best regards.

[Andrew Godwin]

Yes, I'm aware add_field is also in need of expansion points - the same issue was raised with the MSSQL backend too.

I'm more than happy to add these in now or after 1.7 in a point release; the actual implementation of those methods is private so we can change it around a little (or at least, we can add to the available overrides; I wouldn't go removing those things on backend implementers like yourself).

I've also added the implied_column_null database feature in this commit: https://github.com/django/django/commit/1b07781292c2cef9367b521dc0f2b40f4d363083

Just set it to True and you should get the behaviour you need.

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9836fa63-1750-4b0b-bc7e-76efcd9316cd%40googlegroups.com.

[Maximiliano Robaina]

El martes, 20 de mayo de 2014 09:27:48 UTC-3, Andrew Godwin escribió:

Yes, I'm aware add_field is also in need of expansion points - the same issue was raised with the MSSQL backend too.

I'm more than happy to add these in now or after 1.7 in a point release; the actual implementation of those methods is private so we can change it around a little (or at least, we can add to the available overrides; I wouldn't go removing those things on backend implementers like yourself).

I've also added the implied_column_null database feature in this commit: https://github.com/django/django/commit/1b07781292c2cef9367b521dc0f2b40f4d363083

Just set it to True and you should get the behaviour you need.

Andrew

Hi Andrew,

Are you going to backport this to version 1.7?

[Tim Graham]

Backported in 30d8b95139a1fa3070f3b112115e624ffcf8e555, thanks for the reminder.

[Andrew Godwin]

Thanks Tim. I sometimes just forget to run the backport script...

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f89c4f01-36e4-493b-b176-ab37384569e4%40googlegroups.com.