simonw / django-sql-dashboard

Django app for building dashboards using raw SQL queries
https://django-sql-dashboard.datasette.io/
Apache License 2.0
437 stars 37 forks source link

Support named parameters as form fields #15

Closed simonw closed 3 years ago

simonw commented 3 years ago

Like Datasette, except they will apply to all of the queries on the page.

simonw commented 3 years ago

People won't be able to use sql as a parameter, I think that's OK.

I'm going to limit named parameters in the querystring to single values, no .getlist() (as is used with ?sql=).

simonw commented 3 years ago

Documentation: https://www.psycopg.org/docs/usage.html#passing-parameters-to-sql-queries

I'm just going to extract %(date)s style parameters and turn those into form fields - I won't support %s positional parameters.

simonw commented 3 years ago
In [1]: class CaptureDict:
   ...:     def __init__(self):
   ...:         self.accessed = set()
   ...:     def __getitem__(self, key):
   ...:         self.accessed.add(key)
   ...:         return ''
   ...: 
In [2]: d = CaptureDict()
In [3]: 'foo %(name)s and %(name)s and %(bar)s' % d
Out[3]: 'foo  and  and '
In [5]: d.accessed
Out[5]: {'bar', 'name'}
simonw commented 3 years ago

Surprising bug: In this block of code: https://github.com/simonw/django-sql-dashboard/blob/87fc304754d73f2eb6a3610a1296394f36e07746/django_sql_dashboard/views.py#L48-L55

Switching to using cursor.execute(sql, params) causes a new exception I haven't seen before:

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
>               return self.cursor.execute(sql)
E               django.db.utils.InternalError: current transaction is aborted, commands ignored until end of transaction block
../../../.local/share/virtualenvs/django-sql-dashboard-ZqMBSvf9/lib/python3.9/site-packages/django/db/backends/utils.py:82: InternalError

Here's that code in Django: https://github.com/django/django/blob/3.1.7/django/db/backends/utils.py#L77-L84

simonw commented 3 years ago

The query being executed at this point which raises the error is:

-> return self.cursor.execute(sql, params)
(Pdb) sql
'RELEASE SAVEPOINT "s4610936256_x20"'
simonw commented 3 years ago

So I think this confusion is caused because I've ended up running a savepoint, when I really wanted to be manipulating a standard transaction.

simonw commented 3 years ago

Since I'm PostgreSQL only maybe I can work around this by digging down through Django's wrappers to get a raw psycopg2 cursor?

simonw commented 3 years ago

How about using a separate database alias which sets that READ ONLY thing on every new connection? Something like this:

import psycopg2.extensions

DATABASES = {
    # ...
    'OPTIONS': {
        'isolation_level': psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE,
    },
}

https://docs.djangoproject.com/en/3.1/ref/databases/#postgresql-notes

https://nejc.saje.info/django-postgresql-readonly.html suggests:

'OPTIONS': {
    'options': '-c default_transaction_read_only=on'
}