Skip to content

Commit 051a292

Browse files
authored
Merge pull request #120 from ambitioninc/develop
3.1.0
2 parents 0632531 + 141646a commit 051a292

19 files changed

+179
-74
lines changed

.github/workflows/tests.yml

+11-11
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,15 @@ jobs:
1818
# AttributeError: module 'collections' has no attribute 'Callable'
1919
# https://github.com/nose-devs/nose/issues/1099
2020
django:
21-
- 'Django~=2.2.0'
22-
- 'Django~=3.0.0'
23-
- 'Django~=3.1.0'
2421
- 'Django~=3.2.0'
2522
- 'Django~=4.0.0'
2623
- 'Django~=4.1.0'
24+
- 'Django~=4.2.0'
2725
experimental: [false]
28-
include:
29-
- python: '3.9'
30-
django: 'https://github.com/django/django/archive/refs/heads/main.zip#egg=Django'
31-
experimental: true
26+
# include:
27+
# - python: '3.9'
28+
# django: 'https://github.com/django/django/archive/refs/heads/main.zip#egg=Django'
29+
# experimental: true
3230
# NOTE this job will appear to pass even when it fails because of
3331
# `continue-on-error: true`. Github Actions apparently does not
3432
# have this feature, similar to Travis' allow-failure, yet.
@@ -38,6 +36,8 @@ jobs:
3836
django: 'Django~=4.0.0'
3937
- python: '3.7'
4038
django: 'Django~=4.1.0'
39+
- python: '3.7'
40+
django: 'Django~=4.2.0'
4141
services:
4242
postgres:
4343
image: postgres:latest
@@ -53,8 +53,8 @@ jobs:
5353
--health-timeout 5s
5454
--health-retries 5
5555
steps:
56-
- uses: actions/checkout@v2
57-
- uses: actions/setup-python@v2
56+
- uses: actions/checkout@v3
57+
- uses: actions/setup-python@v3
5858
with:
5959
python-version: ${{ matrix.python }}
6060
- name: Setup
@@ -69,7 +69,7 @@ jobs:
6969
env:
7070
DB_SETTINGS: >-
7171
{
72-
"ENGINE":"django.db.backends.postgresql_psycopg2",
72+
"ENGINE":"django.db.backends.postgresql",
7373
"NAME":"querybuilder",
7474
"USER":"postgres",
7575
"PASSWORD":"postgres",
@@ -78,7 +78,7 @@ jobs:
7878
}
7979
DB_SETTINGS2: >-
8080
{
81-
"ENGINE":"django.db.backends.postgresql_psycopg2",
81+
"ENGINE":"django.db.backends.postgresql",
8282
"NAME":"querybuilder2",
8383
"USER":"postgres",
8484
"PASSWORD":"postgres",

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,4 @@ docs/_build/
3838
test_ambition_dev
3939

4040
.tox/
41+
tmp/

README.rst

+2-3
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ where django querysets simplify the sql generation process for simple queries.
3434

3535
Requirements
3636
------------
37-
* Python 2.7
38-
* Python 3.3, 3.4
39-
* Django 1.7+
37+
* Python 3.7 - 3.9
38+
* Django 2.2 - 4.1
4039
* Postgres 9.3+
4140

4241
Installation

docs/release_notes.rst

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,23 @@
11
Release Notes
22
=============
33

4+
v3.1.0
5+
------
6+
* django 4.2 support
7+
8+
v3.0.4
9+
------
10+
* Adjusted querybuilder select functionality to process json values as needed in the result set
11+
rather than tweak the deep cursor settings. This was observed to interfere with complex query chains.
12+
13+
v3.0.3
14+
------
15+
* Addressed bug in `json_cursor` if Django cursor has extra wrappers
16+
417
v3.0.2
518
------
6-
* Add `json_cursor` to handle django no longer automatically parsing json fields
19+
* Add `json_cursor` context to handle Django3.1.1+ no longer automatically parsing json fields
20+
* Adjusted query functionality also to handle jsonb columns correctly
721

822
v3.0.1
923
------

manage.py

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
#!/usr/bin/env python
22
import sys
33

4+
# Show warnings about django deprecations - uncomment for version upgrade testing
5+
import warnings
6+
from django.utils.deprecation import RemovedInNextVersionWarning
7+
warnings.filterwarnings('always', category=DeprecationWarning)
8+
warnings.filterwarnings('always', category=PendingDeprecationWarning)
9+
warnings.filterwarnings('always', category=RemovedInNextVersionWarning)
10+
411
from settings import configure_settings
512

613

querybuilder/__init__.py

-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
# flake8: noqa
22
from .version import __version__
3-
4-
default_app_config = 'querybuilder.apps.QueryBuilderConfig'

querybuilder/cursor.py

+113-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,60 @@
11
import contextlib
22
import json
3-
import psycopg2
3+
from psycopg2.extras import register_default_jsonb
4+
from psycopg2._json import JSONB_OID
5+
6+
7+
def jsonify_cursor(django_cursor, enabled=True):
8+
"""
9+
Adjust an already existing cursor to ensure it will return structured types (list or dict)
10+
from jsonb columns instead of strings. Django 3.1.1+ returns strings for raw queries.
11+
https://code.djangoproject.com/ticket/31956
12+
https://code.djangoproject.com/ticket/31973
13+
https://www.psycopg.org/docs/extras.html#psycopg2.extras.register_default_jsonb
14+
"""
15+
16+
# The thing that is returned by connection.cursor() is (normally) a Django object
17+
# of type CursorWrapper that itself has the "real" cursor as a property called cursor.
18+
# However, it could be a CursorDebugWrapper instead, or it could be an outer wrapper
19+
# wrapping one of those. For example django-debug-toolbar wraps CursorDebugWrapper in
20+
# a NormalCursorWrapper. The django-db-readonly package wraps the Django CursorWrapper
21+
# in a ReadOnlyCursorWrapper. I'm not sure if they ever nest multiple levels. I tried
22+
# looping with `while isinstance(inner_cursor, CursorWrapper)`, but it seems that the
23+
# outer wrapper is not necessarily a subclass of the Django wrapper. My next best option
24+
# is to make the assumption that we need to get to the last property called `cursor`,
25+
# basically assuming that any wrapper is going to have a property called `cursor`
26+
# that is the real cursor or the next-level wrapper.
27+
# Another option might be to check the class of inner_cursor to see if it is the real
28+
# database cursor. That would require importing more django libraries, and probably
29+
# having to handle some changes in those libraries over different versions.
30+
31+
# This register_default_jsonb functionality in psycopg2 does not itself have a "deregister"
32+
# capability. So to deregister, we pass in a different value for the loads method; in this
33+
# case just the str() built-in, which just returns the value passed in. Note that passing
34+
# None for loads does NOT do a deregister; it uses the default value, which as it turns out
35+
# is json.loads anyway!
36+
loads_func = json.loads if enabled else str
37+
38+
# We expect that there is always at least one wrapper, but we might as well handle
39+
# the possibility that we get passed the inner cursor.
40+
inner_cursor = django_cursor
41+
42+
while hasattr(inner_cursor, 'cursor'):
43+
inner_cursor = inner_cursor.cursor
44+
45+
# Hopefully we have the right thing now, but try/catch so we can get a little better info
46+
# if it is not. Another option might be an isinstance, or another function that tests the cursor?
47+
try:
48+
register_default_jsonb(conn_or_curs=inner_cursor, loads=loads_func)
49+
except TypeError as e:
50+
raise Exception(f'jsonify_cursor: conn_or_curs was actually a {type(inner_cursor)}: {e}')
51+
52+
53+
def dejsonify_cursor(django_cursor):
54+
"""
55+
Re-adjust a cursor that was "jsonified" so it no longer performs the json.loads().
56+
"""
57+
jsonify_cursor(django_cursor, enabled=False)
458

559

660
@contextlib.contextmanager
@@ -12,5 +66,62 @@ def json_cursor(django_database_connection):
1266
https://www.psycopg.org/docs/extras.html#psycopg2.extras.register_default_jsonb
1367
"""
1468
with django_database_connection.cursor() as cursor:
15-
psycopg2.extras.register_default_jsonb(conn_or_curs=cursor.cursor, loads=json.loads)
69+
jsonify_cursor(cursor)
1670
yield cursor
71+
# This should really not be necessary, because the cursor context manager will
72+
# be closing the cursor on __exit__ anyway. But just in case.
73+
dejsonify_cursor(cursor)
74+
75+
76+
def json_fetch_all_as_dict(cursor):
77+
"""
78+
Iterates over a result set and converts each row to a dictionary.
79+
The cursor passed in is assumed to have just executed a raw Postgresql query.
80+
If the cursor's columns include any with the jsonb type, the process includes
81+
examining every value from those columns. If the value is a string, a json.loads()
82+
is attempted on the value, because in Django 3.1.1 and later, this is not
83+
handled automatically for raw sql as it was before. There is no compatibility
84+
issue running with older Django versions because if the value is not a string,
85+
(e.g. it has already been converted to a list or dict), the loads() is skipped.
86+
Note that JSON decoding errors are ignored (and the original result value is provided)
87+
because it is possible that the query involved an actual json query, say on a single
88+
string property of the underlying column data. In that case, the column type is
89+
still jsonb, but the result value is a string as it should be. This ignoring of
90+
errors is the same logic used in json handling in Django's from_db_value() method.
91+
92+
:return: A list of dictionaries where each row is a dictionary
93+
:rtype: list of dict
94+
"""
95+
96+
colnames = [col.name for col in cursor.description]
97+
coltypes = [col.type_code for col in cursor.description]
98+
# Identify any jsonb columns in the query, by column index
99+
jsonbcols = [i for i, x in enumerate(coltypes) if x == JSONB_OID]
100+
101+
# Optimize with a simple comprehension if we know there are no jsonb columns to handle.
102+
if not jsonbcols:
103+
return [
104+
dict(zip(colnames, row))
105+
for row in cursor.fetchall()
106+
]
107+
108+
# If there are jsonb columns, intercept the result rows and run a json.loads() on any jsonb
109+
# columns that are presenting as strings.
110+
# In Django 3.1.0 they would already be a json type (e.g. dict or list) but in Django 3.1.1 it changes
111+
# and raw sql queries return strings for jsonb columns.
112+
# https://docs.djangoproject.com/en/4.0/releases/3.1.1/
113+
results = []
114+
115+
for row in cursor.fetchall():
116+
rowvals = list(row)
117+
for colindex in jsonbcols:
118+
if type(rowvals[colindex]) is str: # need to check type to avoid attempting to jsonify a None
119+
try:
120+
rowvals[colindex] = json.loads(rowvals[colindex])
121+
# It is possible that we are selecting a sub-value from the json in the column. I.e.
122+
# we got here because it IS a jsonb column, but what we selected is not json and will
123+
# fail to parse. In that case, we already have the value we want in place.
124+
except json.JSONDecodeError:
125+
pass
126+
results.append(dict(zip(colnames, rowvals)))
127+
return results

querybuilder/query.py

+8-7
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99
get_model = apps.get_model
1010
import six
1111

12-
1312
from querybuilder.fields import FieldFactory, CountField, MaxField, MinField, SumField, AvgField
1413
from querybuilder.helpers import set_value_for_keypath, copy_instance
1514
from querybuilder.tables import TableFactory, ModelTable, QueryTable
15+
from querybuilder.cursor import json_fetch_all_as_dict
16+
1617

1718
SERIAL_DTYPES = ['serial', 'bigserial']
1819

@@ -640,7 +641,11 @@ def get_cursor(self):
640641
:rtype: :class:`CursorDebugWrapper <django:django.db.backends.util.CursorDebugWrapper>`
641642
:returns: A database cursor
642643
"""
643-
return self.connection.cursor()
644+
cursor = self.connection.cursor()
645+
# Do not set up the cursor in psycopg2 to run json.loads on jsonb columns here. Do it
646+
# right before we run a select, and then set it back after that.
647+
# jsonify_cursor(cursor)
648+
return cursor
644649

645650
def from_table(self, table=None, fields='*', schema=None, **kwargs):
646651
"""
@@ -1936,11 +1941,7 @@ def _fetch_all_as_dict(self, cursor):
19361941
:return: A list of dictionaries where each row is a dictionary
19371942
:rtype: list of dict
19381943
"""
1939-
desc = cursor.description
1940-
return [
1941-
dict(zip([col[0] for col in desc], row))
1942-
for row in cursor.fetchall()
1943-
]
1944+
return json_fetch_all_as_dict(cursor)
19441945

19451946

19461947
class QueryWindow(Query):

querybuilder/tables.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import abc
1+
from abc import ABCMeta
22

33
from django.db.models.base import ModelBase
4-
from six import string_types, with_metaclass
54

65
import querybuilder
76
from querybuilder.fields import FieldFactory
@@ -31,7 +30,7 @@ def __new__(cls, table, *args, **kwargs):
3130
kwargs.update(alias=list(table.keys())[0])
3231
table = list(table.values())[0]
3332
table_type = type(table)
34-
if isinstance(table, string_types):
33+
if isinstance(table, str):
3534
return SimpleTable(table, **kwargs)
3635
elif issubclass(table_type, ModelBase):
3736
return ModelTable(table, **kwargs)
@@ -44,7 +43,7 @@ def __new__(cls, table, *args, **kwargs):
4443
return None
4544

4645

47-
class Table(with_metaclass(abc.ABCMeta, object)):
46+
class Table(object, metaclass=ABCMeta):
4847
"""
4948
Abstract table class that all table types extend.
5049
@@ -259,7 +258,7 @@ def add_fields(self, fields):
259258
or ``Field`` instance
260259
:type fields: str or tuple or list of str or list of Field or :class:`Field <querybuilder.fields.Field>`
261260
"""
262-
if isinstance(fields, string_types):
261+
if isinstance(fields, str):
263262
fields = [fields]
264263
elif type(fields) is tuple:
265264
fields = list(fields)

querybuilder/tests/__init__.py

-1
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
default_app_config = 'querybuilder.tests.apps.QuerybuilderTestConfig'

querybuilder/tests/json_tests.py

+10-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import unittest
2-
from django import VERSION
32
from django.test.utils import override_settings
43
from querybuilder.fields import JsonField
54
from querybuilder.query import Query, JsonQueryset
@@ -47,11 +46,7 @@ def test_one(self):
4746
)
4847
)
4948

50-
# Django 3.1 changes the raw queryset behavior so querybuilder isn't going to change that behavior
51-
if self.is_31_or_above():
52-
self.assertEqual(query.select(), [{'my_two_alias': '"two"'}])
53-
else:
54-
self.assertEqual(query.select(), [{'my_two_alias': 'two'}])
49+
self.assertEqual(query.select(), [{'my_two_alias': 'two'}])
5550

5651
query = Query().from_table(MetricRecord, fields=[one_field]).where(**{
5752
one_field.get_where_key(): '1'
@@ -64,11 +59,7 @@ def test_one(self):
6459
)
6560
)
6661

67-
# Django 3.1 changes the raw queryset behavior so querybuilder isn't going to change that behavior
68-
if self.is_31_or_above():
69-
self.assertEqual(query.select(), [{'my_one_alias': '1'}])
70-
else:
71-
self.assertEqual(query.select(), [{'my_one_alias': 1}])
62+
self.assertEqual(query.select(), [{'my_one_alias': 1}])
7263

7364
query = Query().from_table(MetricRecord, fields=[one_field]).where(**{
7465
one_field.get_where_key(): '2'
@@ -82,12 +73,14 @@ def test_one(self):
8273
)
8374
self.assertEqual(query.select(), [])
8475

85-
def is_31_or_above(self):
86-
if VERSION[0] == 3 and VERSION[1] >= 1:
87-
return True
88-
elif VERSION[0] > 3:
89-
return True
90-
return False
76+
# Currently unused (but maybe again sometime) function to check django version for test results
77+
# from django import VERSION
78+
# def is_31_or_above(self):
79+
# if VERSION[0] == 3 and VERSION[1] >= 1:
80+
# return True
81+
# elif VERSION[0] > 3:
82+
# return True
83+
# return False
9184

9285

9386
@override_settings(DEBUG=True)

0 commit comments

Comments
 (0)