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

Why use PostgresqlUUIDField from django_extension instead of UUIDField? #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

reason-systems
Copy link

The original code make the app unusable on a MySQL database.
Not sure if it is going to work on a postgresql db though.
Hope this helps.

…rovided?

The original code make the app unusable on a MySQL database.
Not sure if it is going to work on a postgresql db though.
Hope this helps.
@Tyrdall
Copy link
Member

Tyrdall commented Nov 10, 2015

@dkaufhold Guess we can merge tis request, right?

@mbrochh
Copy link
Member

mbrochh commented Nov 11, 2015

When we change a field on the model, don't we also have to create a schemamigration? I'm not sure what South would do about such a change, i.e. if the fields are compatible with each other and all the existing values can be migrated into the new field (I would HOPE that it just works).

But I think we will definitely need a schemamigration here, amiright?

@reason-systems
Copy link
Author

Maybe you should wait a bit. I may have a the opportunity to test it on a real pg setup soon. Will post an update.

@dkaufhold
Copy link

@Tyrdall I'd expect the storage to be the same, but looks as the postgresql version gives some advantages in terms of indexing. Note, that from Django 1.8 onwards Django provides its own UUID field and so django extensions will no longer support it.

@mbrochh
Copy link
Member

mbrochh commented Nov 12, 2015

@reason-systems awesome! We will wait for your report :)

@reason-systems
Copy link
Author

I have two live projects using the package (very handy btw, well done).
The first has an old py26/dj14/south setup. I deployed it using the unpatched version, and everything went well. I reproduced a fresh install (new mysql db and env/) and it got it right:

mysql> describe newsletter_signup_newslettersignup;
+--------------------+-------------+------+-----+---------+----------------+
| Field              | Type        | Null | Key | Default | Extra          |
+--------------------+-------------+------+-----+---------+----------------+
| id                 | int(11)     | NO   | PRI | NULL    | auto_increment |
| user_id            | int(11)     | YES  | MUL | NULL    |                |
| email              | varchar(64) | NO   |     | NULL    |                |
| signup_date        | datetime    | NO   |     | NULL    |                |
| verification_token | varchar(36) | NO   |     | NULL    |                |
| verification_date  | datetime    | YES  |     | NULL    |                |
+--------------------+-------------+------+-----+---------+----------------+
6 rows in set (0,00 sec)

The other project is a py27/dj18. So it first complains on south migrations/ directory. If you rename migrations/ you get this error message (I created a minimal dj18 test project):

$ python manage.py syncdb
/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/syncdb.py:24: RemovedInDjango19Warning: The syncdb command will be removed in Django 1.9
  warnings.warn("The syncdb command will be removed in Django 1.9", RemovedInDjango19Warning)
Operations to perform:
  Synchronize unmigrated apps: staticfiles, newsletter_signup, messages
  Apply all migrations: admin, contenttypes, auth, sessions
Synchronizing apps without migrations:
  Creating tables...
    Creating table newsletter_signup_newslettersignup
Traceback (most recent call last):
  File "manage.py", line 10, in 
    execute_from_command_line(sys.argv)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line
    utility.execute()
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 346, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 394, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/syncdb.py", line 25, in handle
    call_command("migrate", **options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 120, in call_command
    return command.execute(*args, **defaults)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 179, in handle
    created_models = self.sync_apps(connection, executor.loader.unmigrated_apps)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 310, in sync_apps
    editor.create_model(model)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 286, in create_model
    self.execute(sql, params or None)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 111, in execute
    cursor.execute(sql, params)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/utils.py", line 98, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 62, in execute
    return self.cursor.execute(sql)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 124, in execute
    return self.cursor.execute(query, args)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 205, in execute
    self.errorhandler(self, exc, value)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
django.db.utils.ProgrammingError: (1064, "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'UUID NOT NULL, `verification_date` datetime(6) NULL)' at line 1")

It then recreated the same minimal project using the patched version and get this error (then running again the migrate command seem to work):

$ python manage.py migrate
Operations to perform:
  Synchronize unmigrated apps: staticfiles, newsletter_signup, messages
  Apply all migrations: admin, contenttypes, auth, sessions
Synchronizing apps without migrations:
  Creating tables...
    Creating table newsletter_signup_newslettersignup
    Running deferred SQL...
Traceback (most recent call last):
  File "manage.py", line 10, in 
    execute_from_command_line(sys.argv)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line
    utility.execute()
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 346, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 394, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/base.py", line 445, in execute
    output = self.handle(*args, **options)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 179, in handle
    created_models = self.sync_apps(connection, executor.loader.unmigrated_apps)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 318, in sync_apps
    cursor.execute(statement)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/utils.py", line 98, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 62, in execute
    return self.cursor.execute(sql)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/django/db/backends/mysql/base.py", line 124, in execute
    return self.cursor.execute(query, args)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/MySQLdb/cursors.py", line 205, in execute
    self.errorhandler(self, exc, value)
  File "/tmp/newsletter_signup_test/env/local/lib/python2.7/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue
django.db.utils.IntegrityError: (1215, 'Cannot add foreign key constraint')
$ python manage.py migrate
Operations to perform:
  Synchronize unmigrated apps: staticfiles, newsletter_signup, messages
  Apply all migrations: admin, contenttypes, auth, sessions
Synchronizing apps without migrations:
  Creating tables...
    Running deferred SQL...
  Installing custom SQL...
Running migrations:
  Rendering model states... DONE
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying sessions.0001_initial... OK

It got it right at db level (I guess ; any hint about the constraint?):

mysql> describe newsletter_signup_newslettersignup;
+--------------------+-------------+------+-----+---------+----------------+
| Field              | Type        | Null | Key | Default | Extra          |
+--------------------+-------------+------+-----+---------+----------------+
| id                 | int(11)     | NO   | PRI | NULL    | auto_increment |
| user_id            | int(11)     | YES  |     | NULL    |                |
| email              | varchar(64) | NO   |     | NULL    |                |
| signup_date        | datetime(6) | NO   |     | NULL    |                |
| verification_token | varchar(36) | NO   |     | NULL    |                |
| verification_date  | datetime(6) | YES  |     | NULL    |                |
+--------------------+-------------+------+-----+---------+----------------+
6 rows in set (0,00 sec)

The 1.8 migration system seem to work OK:

$ python manage.py makemigrations newsletter_signup
Migrations for 'newsletter_signup':
  0001_initial.py:
    - Create model NewsletterSignup
$ python manage.py migrate -l
admin
 [X] 0001_initial
auth
 [X] 0001_initial
 [X] 0002_alter_permission_name_max_length
 [X] 0003_alter_user_email_max_length
 [X] 0004_alter_user_username_opts
 [X] 0005_alter_user_last_login_null
 [X] 0006_require_contenttypes_0002
contenttypes
 [X] 0001_initial
 [X] 0002_remove_content_type_name
newsletter_signup
 [ ] 0001_initial
sessions
 [X] 0001_initial
$ python manage.py migrate newsletter_signup --fake
(...)
$ python manage.py migrate newsletter_signup zero
(...)
$ python manage.py migrate newsletter_signup 
Operations to perform:
  Apply all migrations: newsletter_signup
Running migrations:
  Rendering model states... DONE
  Applying newsletter_signup.0001_initial... OK

So I think this is finally more a mysql + django1.8 compatibility issue.

Next week I'm doing a big update on the second project, which I had to deploy on postgres at the last minute to work around the issue. I'm moving back to mysql, but I'll have a spare pg db to play with on preprod. Hope I'll have time to do some tests.

NB: I had no issue with a sqlite db.

FTR, simple procedure to create a test project

$  django-admin startproject newsletter_signup_test
$  cd newsletter_signup_test/
$  virtualenv env
$  source ./env/bin/activate
$  pip install -U django
$  pip install git+https://github.com/reason-systems/django-newsletter-signup/@patch-1#egg=newsletter_signup
$  vi newsletter_signup_test/settings.py # add newsletter_signup to INSTALLED_APPS + setup DB
$  python manage.py migrate

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.

4 participants