[Django] #28975: Postgis create extension runs regardless if in migration file or not
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.0
Severity: Normal | Keywords: create extension,
Triage Stage: | postgis, psycopg2, postgresql
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Hello,
I have found an issue with the documentation and/or the behaivor of the
postgis extension create method.
**Background**
I am trying to migrate my app that uses the postgis extension inside a
docker container. I do create the extension beforehand as superuser since
I dont intend to run the database connection as a postgres superuser.
**Problem**
As per the documentation found here:
https://docs.djangoproject.com/en/2.0/ref/contrib/gis/install/postgis/
I find it confusing that the documentation states "The command is run
during the migrate process." and "An alternative is to use a migration
operation in your project:". Because what seems to be the problem is that
even if you remove the create extension statement from the migration file,
it still runs regardless - rendering the option to put the create
extension statement inside the migration file somewhat redundant and
probably not very useful.
**Possible solution**
Ideally the behaivor would be only create extension if its in a migration
file and if its not - its the users concern to create it however it see
fit.
The relevant part is this:
{{{
testing.api | File "/usr/lib/python3.6/site-
packages/django/contrib/gis/db/backends/postgis/base.py", line 26, in
prepare_database
testing.api | cursor.execute("CREATE EXTENSION IF NOT EXISTS postgis")
}}}
Full trace:
{{{
testing.api | POSTGRES: I'm still down. Standby.
testing.api | List of databases
testing.api | Name | Owner | Encoding | Collate | Ctype |
Access privileges
testing.api |
-----------+----------+----------+---------+---------+-----------------------
testing.api | postgres | postgres | UTF8 | C | C.UTF-8 |
testing.api | template0 | postgres | UTF8 | C | C.UTF-8 |
=c/postgres +
testing.api | | | | | |
postgres=CTc/postgres
testing.api | template1 | postgres | UTF8 | C | C.UTF-8 |
=c/postgres +
testing.api | | | | | |
postgres=CTc/postgres
testing.api | testing | postgres | UTF8 | C | C.UTF-8 |
=Tc/postgres +
testing.api | | | | | |
postgres=CTc/postgres+
testing.api | | | | | |
testing=CTc/postgres
testing.api | (4 rows)
testing.api |
testing.api | POSTGRES: I'm up, I'm up, I'm f***** up - but I'm up
testing.api | Collecting selenium==3.3.1 (from -r
/app/src/core/requirements/testing.txt (line 1))
testing.api | Downloading selenium-3.3.1-py2.py3-none-any.whl (930kB)
testing.api | Collecting django-debug-toolbar==1.8 (from -r
/app/src/core/requirements/testing.txt (line 2))
testing.api | Downloading django_debug_toolbar-1.8-py2.py3-none-any.whl
(205kB)
testing.api | Requirement already satisfied: Django>=1.8 in
/usr/lib/python3.6/site-packages (from django-debug-toolbar==1.8->-r
/app/src/core/requirements/testing.txt (line 2))
testing.api | Collecting sqlparse>=0.2.0 (from django-debug-
toolbar==1.8->-r /app/src/core/requirements/testing.txt (line 2))
testing.api | Downloading sqlparse-0.2.4-py2.py3-none-any.whl
testing.api | Requirement already satisfied: pytz in /usr/lib/python3.6
/site-packages (from Django>=1.8->django-debug-toolbar==1.8->-r
/app/src/core/requirements/testing.txt (line 2))
testing.api | Installing collected packages: selenium, sqlparse, django-
debug-toolbar
testing.api | Successfully installed django-debug-toolbar-1.8
selenium-3.3.1 sqlparse-0.2.4
testing.api | Traceback (most recent call last):
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 83, in _execute
testing.api | return self.cursor.execute(sql)
testing.api | psycopg2.ProgrammingError: permission denied to create
extension "postgis"
testing.api | HINT: Must be superuser to create this extension.
testing.api |
testing.api |
testing.api | The above exception was the direct cause of the following
exception:
testing.api |
testing.api | Traceback (most recent call last):
testing.api | File "manage.py", line 22, in <module>
testing.api | execute_from_command_line(sys.argv)
testing.api | File "/usr/lib/python3.6/site-
packages/django/core/management/__init__.py", line 371, in
execute_from_command_line
testing.api | utility.execute()
testing.api | File "/usr/lib/python3.6/site-
packages/django/core/management/__init__.py", line 365, in execute
testing.api | self.fetch_command(subcommand).run_from_argv(self.argv)
testing.api | File "/usr/lib/python3.6/site-
packages/django/core/management/base.py", line 288, in run_from_argv
testing.api | self.execute(*args, **cmd_options)
testing.api | File "/usr/lib/python3.6/site-
packages/django/core/management/base.py", line 335, in execute
testing.api | output = self.handle(*args, **options)
testing.api | File "/usr/lib/python3.6/site-
packages/django/core/management/commands/migrate.py", line 77, in handle
testing.api | connection.prepare_database()
testing.api | File "/usr/lib/python3.6/site-
packages/django/contrib/gis/db/backends/postgis/base.py", line 26, in
prepare_database
testing.api | cursor.execute("CREATE EXTENSION IF NOT EXISTS postgis")
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 100, in execute
testing.api | return super().execute(sql, params)
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 68, in execute
testing.api | return self._execute_with_wrappers(sql, params,
many=False, executor=self._execute)
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 77, in _execute_with_wrappers
testing.api | return executor(sql, params, many, context)
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 85, in _execute
testing.api | return self.cursor.execute(sql, params)
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/utils.py", line 89, in __exit__
testing.api | raise dj_exc_value.with_traceback(traceback) from
exc_value
testing.api | File "/usr/lib/python3.6/site-
packages/django/db/backends/utils.py", line 83, in _execute
testing.api | return self.cursor.execute(sql)
testing.api | django.db.utils.ProgrammingError: permission denied to
create extension "postgis"
testing.api | HINT: Must be superuser to create this extension.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28975>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: nobody
Type: Bug | Status: new
Component: GIS | Version: 2.0
Keywords: create extension, | Triage Stage:
postgis, psycopg2, postgresql | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Curtis Maloney):
Either we should allow the migration, or have it in the db backend but
gated somehow...
Having the migration (my preference) but ALWAYS performing the action
_anyway_ is just pointless.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: GIS | Version: 2.0
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage:
postgis, psycopg2, postgresql | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => sphrak
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage:
postgis, psycopg2, postgresql | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* component: GIS => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Cleanup/optimization |
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ramiro Morales):
I think the issue reported isn't that creation of the extension must be
skipped if it already exist (as I think that's what the `IF NOT EXISTS
postgis` part of `CREATE EXTENSION IF NOT EXISTS postgis` is about) But
rather that the query is tried unconditionally.
I'd vote for removing the automatic creation of the postgis extension from
`postgis backend DatabaseWrapper.prepare_databse()` we added in 1.8 and
move to having users to perform it explicitly either:
- Manually, externally -- For cases like the OP describes
- Via a migration -- For cases in which the user Django connects to
Postgres as has DB superuser rights there as it's assumed
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by sphrak):
This issue is currently stalled while waiting on response here:
https://groups.google.com/forum/#!topic/django-developers/EjouxahkqUc
Replying to [ticket:28975 sphrak]:
any.whl (205kB)
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Cleanup/optimization |
Component: GIS | Version: 2.0
Severity: Normal | Resolution:
Keywords: create extension, | worksforme
postgis, psycopg2, postgresql | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* component: Database layer (models, ORM) => GIS
* resolution: => worksforme
Comment:
As Shai said on the mailing list, `CREATE EXTENSION IF NOT EXISTS
postgis;` doesn't seem to fail if the extension already exists, even if
the user isn't a superuser. I see `NOTICE: extension "postgis" already
exists, skipping` in this case. Unless I'm misunderstanding the original
report, I don't see a need to make any changes.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: closed
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | worksforme
postgis, psycopg2, postgresql | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
#33885 was a duplicate. Maybe we should skip creation 🤔, if not needed,
as we did for `CreationExtension()` operation in
d693a086def21d843c03ec3f5b5e697ed2463d45.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Cleanup/optimization |
Component: GIS | Version: 2.0
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* has_patch: 1 => 0
* resolution: worksforme =>
* needs_tests: 1 => 0
Comment:
As mentioned in #33885 the current behavior of implicitly creating the PG
extensions, where all other PG extensions need to be created manually or
via a migration seems inconsistent.
Therefore, I'd recommend deprecating the behavior and schedule it for
deletion in Django 5.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: new
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Other extensions are needed to unlock various features on some PostgreSQL
versions. `postgis` is always required for
`django.contrib.gis.db.backends.postgis`, so IMO it's not the same case.
We would force all users to manually create a migration with
`PostGISExtension()` when starting to work with GIS on PostgreSQL. What do
you think about my [https://code.djangoproject.com/ticket/28975#comment:9
proposition]?
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: sphrak
Type: | Status: new
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: amureki (added)
Comment:
As I am also affected by this, I would be happy to introduce a fix for
that, which would be following Mariusz's suggestion.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:12>
Django
-------------------------------------+-------------------------------------
Type: | Status: assigned
Cleanup/optimization |
Component: GIS | Version: 2.0
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: sphrak => amureki
* status: new => assigned
Comment:
Replying to [comment:12 amureki]:
> As I am also affected by this, I would be happy to introduce a fix for
that, which would be following Mariusz's suggestion.
Thanks, feel-free to submit a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Rust, Do you have time to work on this?
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by amureki):
Replying to [comment:14 Mariusz Felisiak]:
> Rust, Do you have time to work on this?
I did prepare a patch. However, I am struggling with the tests a bit.
Do you mind if I submit a PR and ask for your assistance?
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
> Do you mind if I submit a PR and ask for your assistance?
Of course not, please do.
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Accepted
postgis, psycopg2, postgresql |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by amureki):
Replying to [comment:16 Mariusz Felisiak]:
> > Do you mind if I submit a PR and ask for your assistance?
>
> Of course not, please do.
<3
Not sure, how to modify this ticket to catch the new PR (as there already
was one that is closed), but here is a link:
https://github.com/django/django/pull/15941
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Type: | Status: assigned
Cleanup/optimization |
Severity: Normal | Resolution:
Keywords: create extension, | Triage Stage: Ready for
postgis, psycopg2, postgresql | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: sphrak | Owner: amureki
Cleanup/optimization |
Component: GIS | Version: 2.0
Severity: Normal | Resolution: fixed
Keywords: create extension, | Triage Stage: Ready for
postgis, psycopg2, postgresql | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"8403da3696730f8b9e7d2ad20c920fcd7c52b62c" 8403da36]:
{{{
#!CommitTicketReference repository=""
revision="8403da3696730f8b9e7d2ad20c920fcd7c52b62c"
Fixed #28975 -- Made PostGIS backend skip extension creation if installed.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28975#comment:19>