Skip to content

Commit

Permalink
Fix bugs with non default hll fields
Browse files Browse the repository at this point in the history
The implementation of hll fields with non default values was broken
as it passed the hll kwargs down to the Field without removing them.

Note there is a wider batch of work to support non default values in
the library as new hll fields created, such as HllEmpty need to match
the settings of the field being written to. I have not attempted to
address this, and have simply used the hll_set_defaults to control
things in the tests.
  • Loading branch information
markallanson committed Sep 8, 2020
1 parent 1ea3849 commit 0cb887e
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
10 changes: 5 additions & 5 deletions src/django_pg_hll/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ class HllField(BinaryField):
}

def __init__(self, *args, **kwargs):
self._log2m = kwargs.get('log2m', self.custom_params['log2m'])
self._regwidth = kwargs.get('regwidth', self.custom_params['regwidth'])
self._expthresh = kwargs.get('expthresh', self.custom_params['expthresh'])
self._sparseon = kwargs.get('sparseon', self.custom_params['sparseon'])
self._log2m = kwargs.pop('log2m', self.custom_params['log2m'])
self._regwidth = kwargs.pop('regwidth', self.custom_params['regwidth'])
self._expthresh = kwargs.pop('expthresh', self.custom_params['expthresh'])
self._sparseon = kwargs.pop('sparseon', self.custom_params['sparseon'])

super(HllField, self).__init__(*args, **kwargs)

Expand All @@ -40,7 +40,7 @@ def deconstruct(self):
# Only include kwarg if it's not the default
for param_name, default in self.custom_params.items():
if getattr(self, '_%s' % param_name) != default:
kwargs[name] = getattr(self, '_%s' % param_name)
kwargs[param_name] = getattr(self, '_%s' % param_name)

return name, path, args, kwargs

Expand Down
10 changes: 10 additions & 0 deletions tests/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,15 @@ class Migration(migrations.Migration):
options={
'abstract': False,
}
),
migrations.CreateModel(
name='TestConfiguredModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('hll_field', HllField(log2m=13, regwidth=2, expthresh=1, sparseon=0)),
],
options={
'abstract': False,
}
)
]
4 changes: 4 additions & 0 deletions tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ class TestModel(models.Model):
hll_field = HllField()
fk = models.ForeignKey(FKModel, null=True, blank=True, on_delete=models.CASCADE)


class TestConfiguredModel(models.Model):
hll_field = HllField(log2m=13, regwidth=2, expthresh=1, sparseon=0)

10 changes: 9 additions & 1 deletion tests/test_hll_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django_pg_hll.bulk_update import HllConcatFunction

from django_pg_hll.compatibility import django_pg_bulk_update_available
from tests.models import TestModel, FKModel
from tests.models import TestConfiguredModel, TestModel, FKModel


class HllFieldTest(TestCase):
Expand Down Expand Up @@ -40,6 +40,14 @@ def test_combine_auto_parse(self):
def test_create(self):
TestModel.objects.create(hll_field=HllEmpty())

def test_create_custom_params(self):
with connection.cursor() as cursor:
cursor.execute('select hll_set_defaults(13,2,1,0);')
try:
TestConfiguredModel.objects.create(hll_field=HllEmpty())
finally:
cursor.execute('select hll_set_defaults(11,5,-1,1);')

def test_migration(self):
query = "SELECT hll_cardinality(hll_field) FROM tests_testmodel;"
self.cursor.execute(query)
Expand Down

0 comments on commit 0cb887e

Please sign in to comment.