Fix write query failing when a named parameter is called :sql (#2765)

Closes #2761

datasette/templates/_sql_parameter_scripts.html

@@ -27,16 +27,20 @@ window.datasetteSqlParameters = (() => {
     manager.section
       .querySelectorAll("[data-parameter-control]")
       .forEach((control) => {
-        manager.parameterState.set(control.name, controlState(control));
+        manager.parameterState.set(
+          control.dataset.parameterName,
+          controlState(control)
+        );
       });
   }
 
-  function createControl(parameter, id, state) {
+  function createControl(parameter, id, state, namePrefix) {
     const control = document.createElement(state.expanded ? "textarea" : "input");
     control.id = id;
-    control.name = parameter;
+    control.name = `${namePrefix || ""}${parameter}`;
     control.value = state.value;
     control.setAttribute("data-parameter-control", "");
+    control.dataset.parameterName = parameter;
     if (state.expanded) {
       control.rows = 5;
     } else {
@@ -53,10 +57,16 @@ window.datasetteSqlParameters = (() => {
     value,
     selectionStart
   ) {
-    const replacement = createControl(control.name, control.id, {
-      value: value === undefined ? control.value : value,
-      expanded: expand,
-    });
+    const parameter = control.dataset.parameterName;
+    const replacement = createControl(
+      parameter,
+      control.id,
+      {
+        value: value === undefined ? control.value : value,
+        expanded: expand,
+      },
+      manager.namePrefix
+    );
     button.textContent = expand ? "Collapse" : "Expand";
     button.setAttribute("aria-expanded", expand ? "true" : "false");
     control.replaceWith(replacement);
@@ -64,7 +74,7 @@ window.datasetteSqlParameters = (() => {
     if (selectionStart !== undefined && replacement.setSelectionRange) {
       replacement.setSelectionRange(selectionStart, selectionStart);
     }
-    manager.parameterState.set(replacement.name, controlState(replacement));
+    manager.parameterState.set(parameter, controlState(replacement));
   }
 
   function renderParameters(manager, parameters) {
@@ -99,7 +109,7 @@ window.datasetteSqlParameters = (() => {
       label.htmlFor = id;
       label.textContent = parameter;
 
-      const control = createControl(parameter, id, state);
+      const control = createControl(parameter, id, state, manager.namePrefix);
 
       row.append(label, control);
       if (manager.allowExpand) {
@@ -124,7 +134,10 @@ window.datasetteSqlParameters = (() => {
       if (!control.matches || !control.matches("[data-parameter-control]")) {
         return;
       }
-      manager.parameterState.set(control.name, controlState(control));
+      manager.parameterState.set(
+        control.dataset.parameterName,
+        controlState(control)
+      );
     });
 
     if (!manager.allowExpand) {
@@ -230,6 +243,7 @@ window.datasetteSqlParameters = (() => {
             ? section.dataset.allowExpand === "1"
             : false
           : options.allowExpand,
+      namePrefix: section ? section.dataset.parameterNamePrefix || "" : "",
       parameterState: new Map(),
     };
     if (section) {

datasette/templates/_sql_parameters.html

@@ -1,9 +1,10 @@
-<div id="{{ sql_parameters_section_id|default("sql-parameters-section") }}" class="sql-parameters-section" data-sql-parameters-section{% if sql_parameters_allow_expand|default(false) %} data-allow-expand="1"{% endif %}>
+{% set sql_parameter_name_prefix = sql_parameter_name_prefix|default("") %}
+<div id="{{ sql_parameters_section_id|default("sql-parameters-section") }}" class="sql-parameters-section" data-sql-parameters-section{% if sql_parameter_name_prefix %} data-parameter-name-prefix="{{ sql_parameter_name_prefix }}"{% endif %}{% if sql_parameters_allow_expand|default(false) %} data-allow-expand="1"{% endif %}>
     {% if parameter_names %}
         <h2>Parameters</h2>
         {% for parameter in parameter_names %}
             {% set parameter_id = (sql_parameter_id_prefix|default("qp")) ~ loop.index %}
-            <p class="sql-parameter-row"><label for="{{ parameter_id }}">{{ parameter }}</label> <input type="text" id="{{ parameter_id }}" name="{{ parameter }}" value="{{ parameter_values.get(parameter, "") }}" data-parameter-control>{% if sql_parameters_allow_expand|default(false) %} <button type="button" class="sql-parameter-toggle" data-parameter-toggle aria-controls="{{ parameter_id }}" aria-expanded="false">Expand</button>{% endif %}</p>
+            <p class="sql-parameter-row"><label for="{{ parameter_id }}">{{ parameter }}</label> <input type="text" id="{{ parameter_id }}" name="{{ sql_parameter_name_prefix }}{{ parameter }}" value="{{ parameter_values.get(parameter, "") }}" data-parameter-control data-parameter-name="{{ parameter }}">{% if sql_parameters_allow_expand|default(false) %} <button type="button" class="sql-parameter-toggle" data-parameter-toggle aria-controls="{{ parameter_id }}" aria-expanded="false">Expand</button>{% endif %}</p>
         {% endfor %}
     {% endif %}
 </div>

datasette/views/execute_write.py

@@ -9,6 +9,7 @@ from .base import BaseView, _error
 from .database import display_rows as display_query_rows
 from .query_helpers import (
     QueryValidationError,
+    SQL_PARAMETER_FORM_PREFIX,
     _analysis_is_write,
     _analysis_rows,
     _analysis_rows_with_permissions,
@@ -295,6 +296,7 @@ class ExecuteWriteView(BaseView):
                 "execute_write_columns": execute_write_columns,
                 "execute_write_display_rows": execute_write_display_rows,
                 "execute_write_truncated": execute_write_truncated,
+                "sql_parameter_name_prefix": SQL_PARAMETER_FORM_PREFIX,
                 "execute_disabled": bool(execute_disabled_reason),
                 "execute_disabled_reason": execute_disabled_reason,
                 "table_columns": table_columns,

datasette/views/query_helpers.py

@@ -49,6 +49,8 @@ _query_write_fields = {
     "on_error_redirect",
 }
 
+SQL_PARAMETER_FORM_PREFIX = "_sql_param_"
+
 
 class QueryValidationError(Exception):
     def __init__(self, message, status=400, *, flash=False):
@@ -289,11 +291,13 @@ def _coerce_execute_write_payload(data, is_json):
             )
         params = data.get("params") or {}
     else:
-        params = {
-            key: value
-            for key, value in data.items()
-            if key not in {"sql", "csrftoken", "_json"}
-        }
+        params = {}
+        for key, value in data.items():
+            if key in {"sql", "csrftoken", "_json"}:
+                continue
+            if key.startswith(SQL_PARAMETER_FORM_PREFIX):
+                key = key[len(SQL_PARAMETER_FORM_PREFIX) :]
+            params[key] = value
     if not isinstance(params, dict):
         raise QueryValidationError("params must be a dictionary")
     return data.get("sql"), params

tests/test_api_write.py

@@ -794,6 +794,44 @@ async def test_update_row_alter(ds_write):
     assert response.json() == {"ok": True}
 
 
+@pytest.mark.asyncio
+async def test_execute_write_form_parameter_called_sql():
+    ds = Datasette(memory=True, default_deny=True)
+    ds.root_enabled = True
+    db = ds.add_memory_database("execute_write_parameter_sql", name="data")
+    await db.execute_write("create table docs (id integer primary key, title text)")
+    await db.execute_write("insert into docs (id, title) values (1, 'Initial')")
+    await ds.invoke_startup()
+
+    form_response = await ds.client.get(
+        "/data/-/execute-write",
+        actor={"id": "root"},
+        params={"sql": "update docs set title = :sql where id = :id"},
+    )
+    assert form_response.status_code == 200
+    assert 'data-parameter-name-prefix="_sql_param_"' in form_response.text
+    assert '<label for="qp1">sql</label>' in form_response.text
+    assert 'name="_sql_param_sql"' in form_response.text
+    assert 'data-parameter-name="sql"' in form_response.text
+    assert 'name="_sql_param_id"' in form_response.text
+
+    response = await ds.client.post(
+        "/data/-/execute-write",
+        actor={"id": "root"},
+        data={
+            "sql": "update docs set title = :sql where id = :id",
+            "_sql_param_sql": "Updated",
+            "_sql_param_id": "1",
+        },
+    )
+
+    assert response.status_code == 200
+    assert "Query executed, 1 row affected" in response.text
+    assert (await db.execute("select title from docs where id = 1")).first()[
+        0
+    ] == "Updated"
+
+
 @pytest.mark.asyncio
 @pytest.mark.parametrize(
     "input,expected_errors",

tests/test_html.py

@@ -342,7 +342,7 @@ async def test_query_parameter_form_fields(ds_client):
     response = await ds_client.get("/fixtures/-/query?sql=select+:name")
     assert response.status_code == 200
     assert (
-        '<label for="qp1">name</label> <input type="text" id="qp1" name="name" value="" data-parameter-control>'
+        '<label for="qp1">name</label> <input type="text" id="qp1" name="name" value="" data-parameter-control data-parameter-name="name">'
         in response.text
     )
     assert 'data-parameters-url="/fixtures/-/query/parameters"' in response.text
@@ -351,7 +351,7 @@ async def test_query_parameter_form_fields(ds_client):
     response2 = await ds_client.get("/fixtures/-/query?sql=select+:name&name=hello")
     assert response2.status_code == 200
     assert (
-        '<label for="qp1">name</label> <input type="text" id="qp1" name="name" value="hello" data-parameter-control>'
+        '<label for="qp1">name</label> <input type="text" id="qp1" name="name" value="hello" data-parameter-control data-parameter-name="name">'
         in response2.text
     )
 

tests/test_stored_queries.py

@@ -201,7 +201,7 @@ def test_error_in_on_success_message_sql(stored_write_client):
 def test_custom_params(stored_write_client):
     response = stored_write_client.get("/data/update_name?extra=foo")
     assert (
-        '<input type="text" id="qp3" name="extra" value="foo" data-parameter-control>'
+        '<input type="text" id="qp3" name="extra" value="foo" data-parameter-control data-parameter-name="extra">'
         in response.text
     )