WIP better SQL errors, refs #619

datasette/database.py

@@ -16,6 +16,7 @@ from .utils import (
     sqlite_timelimit,
     sqlite3,
     table_columns,
+    QueryInterrupted,
 )
 from .inspect import inspect_hash
 
@@ -376,10 +377,6 @@ class WriteTask:
         self.reply_queue = reply_queue
 
 
-class QueryInterrupted(Exception):
-    pass
-
-
 class MultipleValues(Exception):
     pass
 

datasette/templates/query.html

@@ -32,6 +32,10 @@
 
 {% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %}
 
+{% if error %}
+    <p class="message-error">{{ error }}</p>
+{% endif %}
+
 <form class="sql" action="{{ database_url(database) }}{% if canned_query %}/{{ canned_query }}{% endif %}" method="{% if canned_write %}post{% else %}get{% endif %}">
     <h3>Custom SQL query{% if display_rows %} returning {% if truncated %}more than {% endif %}{{ "{:,}".format(display_rows|length) }} row{% if display_rows|length == 1 %}{% else %}s{% endif %}{% endif %} <span class="show-hide-sql">{% if hide_sql %}(<a href="{{ path_with_removed_args(request, {'_hide_sql': '1'}) }}">show</a>){% else %}(<a href="{{ path_with_added_args(request, {'_hide_sql': '1'}) }}">hide</a>){% endif %}</span></h3>
     {% if not hide_sql %}
@@ -76,7 +80,7 @@
     </tbody>
 </table>
 {% else %}
-    {% if not canned_write %}
+    {% if not canned_write and not error %}
         <p class="zero-results">0 results</p>
     {% endif %}
 {% endif %}

datasette/utils/__init__.py

@@ -158,6 +158,10 @@ class InvalidSql(Exception):
     pass
 
 
+class QueryInterrupted(Exception):
+    pass
+
+
 allowed_sql_res = [
     re.compile(r"^select\b"),
     re.compile(r"^explain select\b"),

datasette/views/base.py

@@ -276,7 +276,7 @@ class DataView(BaseView):
             if isinstance(response_or_template_contexts, Response):
                 return response_or_template_contexts
             else:
-                data, _, _ = response_or_template_contexts
+                data = response_or_template_contexts[0]
         except (sqlite3.OperationalError, InvalidSql) as e:
             raise DatasetteError(str(e), title="Invalid SQL", status=400)
 
@@ -307,7 +307,8 @@ class DataView(BaseView):
                     if next:
                         kwargs["_next"] = next
                     if not first:
-                        data, _, _ = await self.data(request, database, hash, **kwargs)
+                        bits = await self.data(request, database, hash, **kwargs)
+                        data = bits[0]
                     if first:
                         await writer.writerow(headings)
                         first = False
@@ -398,9 +399,18 @@ class DataView(BaseView):
             )
             if isinstance(response_or_template_contexts, Response):
                 return response_or_template_contexts
-
             else:
-                data, extra_template_data, templates = response_or_template_contexts
+                if len(response_or_template_contexts) == 3:
+                    data, extra_template_data, templates = response_or_template_contexts
+                elif len(response_or_template_contexts) == 4:
+                    (
+                        data,
+                        extra_template_data,
+                        templates,
+                        status_code,
+                    ) = response_or_template_contexts
+                else:
+                    assert False, "response_or_template_contexts should be 3 or 4 items"
         except QueryInterrupted:
             raise DatasetteError(
                 """

datasette/views/database.py

@@ -11,6 +11,7 @@ from datasette.utils import (
     is_url,
     path_with_added_args,
     path_with_removed_args,
+    QueryInterrupted,
 )
 from datasette.utils.asgi import AsgiFileDownload, Response, Forbidden
 from datasette.plugins import pm
@@ -34,7 +35,6 @@ class DatabaseView(DataView):
 
         if request.args.get("sql"):
             sql = request.args.get("sql")
-            validate_sql_select(sql)
             return await QueryView(self.ds).data(
                 request, database, hash, sql, _size=_size, metadata=metadata
             )
@@ -207,6 +207,12 @@ class QueryView(DataView):
 
         templates = ["query-{}.html".format(to_css_class(database)), "query.html"]
 
+        error = None
+        truncated = False
+        rows = []
+        columns = []
+        http_status = 200
+
         # Execute query - as write or as read
         if write:
             if request.method == "POST":
@@ -242,6 +248,17 @@ class QueryView(DataView):
                     message_type = self.ds.INFO
                     redirect_url = metadata.get("on_success_redirect")
                     ok = True
+                except QueryInterrupted:
+                    raise DatasetteError(
+                        """
+                        SQL query took too long. The time limit is controlled by the
+                        <a href="https://docs.datasette.io/en/stable/config.html#sql-time-limit-ms">sql_time_limit_ms</a>
+                        configuration option.
+                    """,
+                        title="SQL Interrupted",
+                        status=400,
+                        messagge_is_html=True,
+                    )
                 except Exception as e:
                     message = metadata.get("on_error_message") or str(e)
                     message_type = self.ds.ERROR
@@ -288,10 +305,22 @@ class QueryView(DataView):
                 params_for_query = MagicParameters(params, request, self.ds)
             else:
                 params_for_query = params
-            results = await self.ds.execute(
-                database, sql, params_for_query, truncate=True, **extra_args
-            )
-            columns = [r[0] for r in results.description]
+            ok = False
+            try:
+                validate_sql_select(sql)
+                results = await self.ds.execute(
+                    database, sql, params_for_query, truncate=True, **extra_args
+                )
+                rows = results.rows
+                truncated = results.truncated
+                columns = [r[0] for r in results.description]
+                ok = True
+            except Exception as e:
+                rows = []
+                columns = []
+                error = str(e)
+                ok = False
+                http_status = 400
 
         if canned_query:
             templates.insert(
@@ -303,9 +332,9 @@ class QueryView(DataView):
 
         async def extra_template():
             display_rows = []
-            for row in results.rows:
+            for row in rows:
                 display_row = []
-                for column, value in zip(results.columns, row):
+                for column, value in zip(columns, row):
                     display_value = value
                     # Let the plugins have a go
                     # pylint: disable=no-member
@@ -345,10 +374,13 @@ class QueryView(DataView):
 
         return (
             {
+                "ok": ok,
+                "error": error,
                 "database": database,
+                "error": error,
                 "query_name": canned_query,
-                "rows": results.rows,
-                "truncated": results.truncated,
+                "rows": rows,
+                "truncated": truncated,
                 "columns": columns,
                 "query": {"sql": sql, "params": params},
                 "private": private,
@@ -358,6 +390,7 @@ class QueryView(DataView):
             },
             extra_template,
             templates,
+            http_status,
         )