Question for Oracle Boulder sprinters

[Malcolm Tredinnick]

Given the observation that Martin Winkler made in a recent thread ([1]),
pointing out Oracles '' = NULL addiction, it made me think of something
related: there are a lot of models that have fields with blank=True and
null=False, even in Django core. However, this is kind of nonsensical in
Oracle-land; it fails the constraint checking at insert/update time,
presumably.

So should the Oracle branch be ignoring the null attribute when
blank=True and assuming null=True in that case?

I can't see anything in the current Oracle changes that accomodates this
case, but that might because I suspect the workflow in a lot of "Oracle
cases" is going to involve retrofitting model classes to existing
database tables, so the "null" attribute is kind of ignorable anyway.

[1]
http://groups.google.com/group/django-developers/browse_thread/thread/deec18ebfb5cc16a

Regards,
Malcolm

[Matt Boersma]

On May 11, 8:37 am, Malcolm Tredinnick <malc...@pointy-stick.com>
wrote:
> ...

> So should the Oracle branch be ignoring the null attribute when
> blank=True and assuming null=True in that case?

I think that's reasonable. It may mean a couple of unit tests fail,
so we can either modify them or at least document that this is
expected with the Oracle backend. We had considered doing this
before, but probably opted against it since we were focused on passing
all the tests.

Our current approach in this case is to map an empty string to a
single space char when storing, and the opposite when loading.

We'll try to implement your suggestion above ASAP unless someone has
major objections.

Matt

[Ian Kelly]

On 5/11/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
>
> Given the observation that Martin Winkler made in a recent thread ([1]),
> pointing out Oracles '' = NULL addiction, it made me think of something
> related: there are a lot of models that have fields with blank=True and
> null=False, even in Django core. However, this is kind of nonsensical in
> Oracle-land; it fails the constraint checking at insert/update time,
> presumably.
>
> So should the Oracle branch be ignoring the null attribute when
> blank=True and assuming null=True in that case?

Hi Malcolm,

I've made this change and committed it. I'm glad you brought it up,
because I think we're handling this problem much more cleanly now, on
multiple levels.

I made a small change to what you suggested: instead of matching the
null option to the blank option, we're now coercing null=True for all
fields that store a character string representation. This is because I
found there were multiple models used in the test cases (including
django.contrib.sessions) that use empty default values without
specifying blank=True, causing database errors when the default values
were used. I suspect this kind of inconsistency is more widespread
than just the test cases.

As expected, this is causing some failures in the serializers_regress
tests; specifically #s 15, 41, 51, 61, 91, 102, 141, 162, 181, and
191. Each of these is the result of trying to store the value None and
getting back the empty string, which is to be expected with this
implementation. I propose that we simply bypass this subset of tests
for Oracle and add new empty-string tests for those test models that
don't already have one. I'd like to hear your thoughts, though.

Thanks,
Ian Kelly

[Malcolm Tredinnick]

On Tue, 2007-05-15 at 17:07 -0600, Ian Kelly wrote:
> On 5/11/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
> >
> > Given the observation that Martin Winkler made in a recent thread ([1]),
> > pointing out Oracles '' = NULL addiction, it made me think of something
> > related: there are a lot of models that have fields with blank=True and
> > null=False, even in Django core. However, this is kind of nonsensical in
> > Oracle-land; it fails the constraint checking at insert/update time,
> > presumably.
> >
> > So should the Oracle branch be ignoring the null attribute when
> > blank=True and assuming null=True in that case?
>
> Hi Malcolm,
>
> I've made this change and committed it. I'm glad you brought it up,
> because I think we're handling this problem much more cleanly now, on
> multiple levels.
>
> I made a small change to what you suggested: instead of matching the
> null option to the blank option, we're now coercing null=True for all
> fields that store a character string representation. This is because I
> found there were multiple models used in the test cases (including
> django.contrib.sessions) that use empty default values without
> specifying blank=True, causing database errors when the default values
> were used. I suspect this kind of inconsistency is more widespread
> than just the test cases.

Hmmm. <scratches head/>

I am still surprised from time to time by the behaviour of "blank". This
would be another one of those cases. I can always see in retrospect why
it behaves like that, but it's not always intuitive.

I haven't looked at the latest patch: did you remember to document this
in models-api.txt?

> As expected, this is causing some failures in the serializers_regress
> tests; specifically #s 15, 41, 51, 61, 91, 102, 141, 162, 181, and
> 191. Each of these is the result of trying to store the value None and
> getting back the empty string, which is to be expected with this
> implementation. I propose that we simply bypass this subset of tests
> for Oracle and add new empty-string tests for those test models that
> don't already have one. I'd like to hear your thoughts, though.

Makes sense. We don't want to remove those tests for database that have
implemented NULL properly, so conditional removal makes sense. Since
everything's just Python code and you will have access to the settings
module inside the test files, making it conditional seems like the right
thing.

Regards,
Malcolm

[Ian Kelly]

On 5/16/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
>
> I haven't looked at the latest patch: did you remember to document this
> in models-api.txt?

No, I'll have to take a look through the documentation some time and
figure out what needs to be added for the branch.

Thanks,
Ian Kelly

[Ian Kelly]

On 5/16/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
>

I've fixed the tests in the branch. One thing I just noticed is that
NullBooleanField.empty_strings_allowed is True, which is going to
encourage the Oracle backend to return the empty string when fetching
null booleans from the database. Unless there's some context that I'm
missing, I think this value should be changed to False.

Thanks,
Ian Kelly