Skip automatic creation of postgis extension if it already exists

[sphrak]

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.

[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.

[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

[sphrak]

​PR

[sphrak]

This issue is currently stalled while waiting on response here: ​https://groups.google.com/forum/#!topic/django-developers/EjouxahkqUc

[Tim Graham]

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.

[Mariusz Felisiak]

#33885 was a duplicate. Maybe we should skip creation 🤔, if not needed, as we did for CreationExtension() operation in d693a086def21d843c03ec3f5b5e697ed2463d45.

[Johannes Maron]

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.

[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 proposition?

[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.

[Mariusz Felisiak]

Replying to 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.

[Mariusz Felisiak]

Rust, Do you have time to work on this?

[amureki]

Replying to 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?

[Mariusz Felisiak]

Do you mind if I submit a PR and ask for your assistance?

Of course not, please do.

[amureki]

Replying to 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

[GitHub <noreply@…>]

In 8403da36:

Fixed #28975 -- Made PostGIS backend skip extension creation if installed.