Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for BigAutoField #48

Closed
hannseman opened this issue May 7, 2020 · 5 comments
Closed

Add support for BigAutoField #48

hannseman opened this issue May 7, 2020 · 5 comments

Comments

@hannseman
Copy link
Contributor

hannseman commented May 7, 2020

It would be nice to have support for BigAutoField primary keys.

Would it be possible to just use the IntPacker for this field or do we have to introduce a LongPacker:

class LongPacker(object):
    @staticmethod
    def pack_pk(user_pk):
        return struct.pack(str("!l"), user_pk)

    @staticmethod
    def unpack_pk(data):
        return struct.unpack(str("!l"), data[:4])[0], data[4:]
@aaugustin
Copy link
Owner

You need 8 bytes for 64-bits integers in BigAutoField. This requires a new packer.

@aaugustin
Copy link
Owner

aaugustin commented May 10, 2020

Oooooops — I was working on a patch on my end and didn't notice your pull request, which was perfectly correct, until I pushed my changes. Sorry :'(

@aaugustin
Copy link
Owner

It doesn't seem useful to run all tests in test_backends.py with all possible primary key types, but it may be interesting to run the most common ones: AutoField, BigAutoField, UUIDField. So I'm going to grab these changes from your PR.

aaugustin added a commit that referenced this issue May 10, 2020
Also support IntegerField and friends, in case someone wants to use them
with primary_key=True, for some reason.

Fix #48.
aaugustin pushed a commit that referenced this issue May 10, 2020
@hannseman
Copy link
Contributor Author

Oooooops — I was working on a patch on my end and didn't notice your pull request, which was perfectly correct, until I pushed my changes. Sorry :'(

No problem! I should've been clear about that I intended to create a PR. Thanks for the quick fix! 👍

@aaugustin
Copy link
Owner

FYI version 1.8 is released on PyPI with this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants