Crash on nullable foreign keys with slices

[Wilfred]

Given the models:

from django.db import models

class Foo(models.Model):
    name = models.CharField(max_length=100, blank=True, null=True)

class PointsToFoo(models.Model):
    foo = models.ForeignKey(Foo, null=True)

And given the following settings (note the FkNullFix):

DATABASES['default'] = {'ENGINE': 'dbindexer', 'TARGET': 'native'}

DBINDEXER_BACKENDS = (
    'dbindexer.backends.BaseResolver',
    'dbindexer.backends.FKNullFix',
    'dbindexer.backends.InMemoryJOINResolver',
)

Querying for null foreign keys generates the following crash:

from django.test import TestCase
from artproject_debug.models import PointsToFoo

class DBIndexerNullableFK(TestCase):
    def test_dbindexer(self):
        filtered_queryset = PointsToFoo.objects.filter(foo=None)

        # force query to occur (yes, twice is necessary)
        list(filtered_queryset[:500])
        list(filtered_queryset[:500])

Traceback:

======================================================================
ERROR: test_dbindexer (artproject_debug.tests.DBIndexerNullableFK)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/wilfred/work/gap2/artproject_debug/tests.py", line 10, in test_dbindexer
    list(filtered_queryset[:500])
  File "/home/wilfred/work/gap2/django/db/models/query.py", line 85, in __len__
    self._result_cache.extend(self._iter)
  File "/home/wilfred/work/gap2/django/db/models/query.py", line 276, in iterator
    for row in compiler.results_iter():
  File "/home/wilfred/work/gap2/dbindexer/compiler.py", line 27, in results_iter
    self.convert_filters()
  File "/home/wilfred/work/gap2/dbindexer/compiler.py", line 19, in convert_filters
    resolver.convert_filters(self.query)
  File "/home/wilfred/work/gap2/dbindexer/resolver.py", line 31, in convert_filters
    backend.convert_filters(query)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 68, in convert_filters
    self._convert_filters(query, query.where)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 75, in _convert_filters
    self._convert_filters(query, child)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 78, in _convert_filters
    self.convert_filter(query, filters, child, index)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 167, in convert_filter
    self.fix_fk_null_filter(query, constraint)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 184, in fix_fk_null_filter
    self.unref_alias(query, alias)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 170, in unref_alias
    unref_alias(query, alias)
  File "/home/wilfred/work/gap2/dbindexer/backends.py", line 145, in unref_alias
    query.table_map[table_name].remove(alias)
ValueError: list.remove(x): x not in list

[wrwrwr]

The FKNull fix modifies a lot of internal variables of the query (which is django.db.models.sql.Query here). When the query is reinitialized for the second call the table_map somehow reuses old columns lists (without the column that the fix removed) -- this already is bad.

Here's branch with a fix:
https://github.com/django-nonrel/django-dbindexer/tree/feature/nullable-foreign-keys-with-slices
It should be enough to change the table_map modification in unref_alias to modify a copy of list it contains:

query.table_map[table_name] = [t for t in query.table_map[table_name] if t != alias]

Moreover, it would be nice to just decrease the alias reference count in unref_alias (so we don't run into similar trouble with some future Django internal query code changes) -- but this would at least require more testing.

[Wilfred]

wrwrwr: are you happy with me merging this?

[wrwrwr]

I suppose yes, but if you wouldn't mind I'll take a closer look and retest after I'm done with the type-conversion resplitting (just a few thousands more of diff lines to go ;-) This one could use clean up too -- just the unref_alias changes and the test are meaningful for the issue.

[rob-b]

This fix won't work as-is with python2.5 as it uses collections.Mapping which was introduced in python 2.6

[wrwrwr]

I'm testing with 2.5 (2.6 for Mongo) / 2.7 before pushing / merging now, so nothing like this should go unnoticed again :-) Can't see any Mapping on this branch, but going to reroll it anyway because too many insignificant changes have crawled in.

[wrwrwr]

Retested and cleaned up:
https://github.com/django-nonrel/django-dbindexer/compare/feature/nullable-foreign-keys-with-slices-2

[petgru]

Does the patch still apply? I have the same problem and would really appreciate it if this patch got pushed.

[smeyfroi]

This patch is still required (in the django nonrel 1.4 branch anyway). Oh, and it works a treat: thanks Wilfred.

[limdauto]

This patch fixes the reported issue but it introduces a new one, i.e. some model is not validated. Does anyone have any idea why?

Error: One or more models did not validate:
admin.logentry: "idxf_object_id_l_exact": CharFields require a "max_length" attribute that is a positive integer.

[limdauto]

I'm not completely sure what's going on but if I only replaces the unref_alias function manually instead of downloading the patch's backends.py, this error doesn't appear.