Pickling support for Oracle Query classes

[Malcolm Tredinnick]

We need to add specific code to handle pickling of the Query class used by the Oracle backend. The attached patch is most of the work need to do that. I can't test it on Oracle, but it should mostly work (similar code works in test situations).

If somebody with access to Oracle could test this and report any errors (or success), I can fine-tune it appropriately.

[Malcolm Tredinnick]

Pickling support for Oracle

[jbronn]

I applied the patch and ran the queries regression tests. It failed with the following error:

File "C:\django\trunk\tests\regressiontests\queries\models.py", line ?, in regressiontests.queries.models.__test__.API_TESTS
Failed example:
    out = pickle.dumps(Item.objects.all())
Exception raised:
    Traceback (most recent call last):
      File "C:\django\trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest regressiontests.queries.models.__test__.API_TESTS[212]>", line 1, in <module>
        out = pickle.dumps(Item.objects.all())
      File "C:\Python25\lib\pickle.py", line 1366, in dumps
        Pickler(file, protocol).dump(obj)
      File "C:\Python25\lib\pickle.py", line 224, in dump
        self.save(obj)
      File "C:\Python25\lib\pickle.py", line 331, in save
        self.save_reduce(obj=obj, *rv)
      File "C:\Python25\lib\pickle.py", line 419, in save_reduce
        save(state)
      File "C:\Python25\lib\pickle.py", line 286, in save
        f(self, obj) # Call unbound method with explicit self
      File "C:\Python25\lib\pickle.py", line 649, in save_dict
        self._batch_setitems(obj.iteritems())
      File "C:\Python25\lib\pickle.py", line 663, in _batch_setitems
        save(v)
      File "C:\Python25\lib\pickle.py", line 331, in save
        self.save_reduce(obj=obj, *rv)
      File "C:\Python25\lib\pickle.py", line 401, in save_reduce
        save(args)
      File "C:\Python25\lib\pickle.py", line 286, in save
        f(self, obj) # Call unbound method with explicit self
      File "C:\Python25\lib\pickle.py", line 562, in save_tuple
        save(element)
      File "C:\Python25\lib\pickle.py", line 286, in save
        f(self, obj) # Call unbound method with explicit self
      File "C:\Python25\lib\pickle.py", line 753, in save_global
        (obj, module, name))
    PicklingError: Can't pickle <class 'django.db.models.sql.query.Query'>: it's not the same object as django.db.models.sql.query.Query

[jbronn]

With the updated patch the queries regression tests pass on Oracle.

The problem was that in __reduce__ the (QueryClass,) arguments refer to the original Query class, which is ​overridden in the query module. Thus, the "original" Query class (I called it BaseQuery) needs to be exposed for custom backend querysets to be pickled. I think we used to do this in the old queryset-refactor branch; I think it was named _Query.

[Malcolm Tredinnick]

Urgh. I really, really want to avoid that type of code and I removed it for that reason. It's ugly. The normal class is Query. It shouldn't have to enter the witness-protection program and undergo a name change just because we need to override it somewhere else.

There must be some other solution that is local to the Oracle (and other custom) backend code. I'll try to work out a better way to test this locally, perhaps by making a fake PostgreSQL class or something to see what I can come up with. I hadn't thought of the name clash issue and didn't check that out in my parallel development attempt.

Thanks for testing so quickly and giving feedback, though. That helps.

[Malcolm Tredinnick]

I forgot to mention, the more technical reason for wanting to avoid that type of code is that the current implementation is sort of poor form. It ties the query class to the connection at import time and I want to avoid those sort of module-level dependencies on connections so that we can talk to multiple storage locations at once, long-term. So removing those lines of code, rather than adding to them, is the longer-term plan. I just haven't worked out a non-sucky way to do that yet. Might be time to do that now.

[anonymous]

Replying to mtredinnick:

I forgot to mention, the more technical reason for wanting to avoid that type of code is that the current implementation is sort of poor form. It ties the query class to the connection at import time and I want to avoid those sort of module-level dependencies on connections so that we can talk to multiple storage locations at once, long-term. So removing those lines of code, rather than adding to them, is the longer-term plan. I just haven't worked out a non-sucky way to do that yet. Might be time to do that now.

Yes... It's time to do this! O;-)

[Malcolm Tredinnick]

Attempts to try and do this using a __new__() method, which felt cleaner, were measurably slower. Given the number of times we create new Query classes, I've decided that pragmatism beats theoretical perfection here. So I'll go with Justin's latest patch.

[Malcolm Tredinnick]

(In [9272]) Fixed #9307 -- Added the ability to pickle the Query class used by the Oracle
backend.

This allows Querysets to be cached for Oracle and should provide a model for
adding pickling support to other (external) database backends that need a
custom Query class.

Thanks to Justin Bronn for some assistance with this patch.

[Malcolm Tredinnick]

(In [9273]) [1.0.X] Fixed #9307 -- Added the ability to pickle the Query class used by the
Oracle backend.

This allows Querysets to be cached for Oracle and should provide a model for
adding pickling support to other (external) database backends that need a
custom Query class.

Thanks to Justin Bronn for some assistance with this patch.

Backport of r9272 from trunk.