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

Fix: UUID field support #54

Merged
merged 4 commits into from Jan 21, 2019
Merged

Conversation

nockty
Copy link
Contributor

@nockty nockty commented Nov 16, 2018

Subject: Fix for the UUIDField support

Feature or Bugfix

  • Bugfix

Purpose

  • This PR should fix the bug I reported here when I tried to insert a value for a UUIDField in Redshift.
  • I also added a test for the insertion of a UUIDField. We should check that the type of the Python argument is str (as opposed to uuid.UUID) and its length is 32.

Detail

  • Fix the Python value of UUIDField when used in Redshift (should be a str, i.e. the value of UUID.hex)
  • Fix a typo in the documentation

Relates

@shimizukawa
Copy link
Member

Thanks! I'll take a look in this weekend.

Copy link
Member

@shimizukawa shimizukawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -36,6 +36,7 @@ class DatabaseFeatures(BasePGDatabaseFeatures):
supports_column_check_constraints = False
can_distinct_on_fields = False
allows_group_by_selected_pks = False
has_native_uuid_field = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -20,7 +20,7 @@ Setup development environment
* Requires supported Python version
* do setup under django-redshift-backend.git repository root as::

$ pip install -U pip setuptools wheel setuotools_scm
$ pip install -U pip setuptools wheel setuptools_scm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've applied this line at ff5c1cb

# uuid is the last field of TestModel
uuid_insert_value = statements[0][1][-1]
# the Python value for insertion must be a string whose length is 32
self.assertEqual(type(uuid_insert_value), str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@shimizukawa shimizukawa merged commit ed918b4 into jazzband:master Jan 21, 2019
@shimizukawa
Copy link
Member

I'm sorry for my late reply.
Thanks for your contribution!

shimizukawa added a commit that referenced this pull request Jan 21, 2019
@nockty
Copy link
Contributor Author

nockty commented Jan 21, 2019

All right, thanks for merging this! :)

@nockty nockty deleted the fix-uuid-field-support branch January 21, 2019 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UUIDField support: How does it work?
3 participants