issues

(I originally opened this in https://github.com/simonw/datasette-insert/issues/20, but I see that that library depends on sqlite-utils)

In the case of a new row, upsert first adds the row, specifying only its pkeys: https://github.com/simonw/sqlite-utils/blob/965ca0d5f5bffe06cc02cd7741344d1ddddf9d56/sqlite_utils/db.py#L2783-L2787

This means that a table with NON NULL (or other constraint) columns that aren't part of the pkey can't have new rows upserted.

This fixes #514 by making the initial insert for upserts include all columns, so that new rows can be added to tables with non-pkey columns that have constraints.

(aside: I'm not a python programmer. pip? pipenv? venv? These are mystical incantations to me. The process to set up this repo for local development and testing was so easy. Thank you for the excellent contributing documentation!)

:books: Documentation preview :books:: https://sqlite-utils--515.org.readthedocs.build/en/515/

By default, SQLite does not enforce foreign key constraints. I typically enable these checks by running:

sql PRAGMA foreign_keys = ON;

inside of a prepare_connection hook.

If a plugin causes the schema to change (eg datasette-scraper creating a new table, or datasette-edit-schema changing a column), then https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/utils/internal_db.py#L71-L77 will fail with:

FOREIGN KEY constraint failed

This could be resolved by either: - deleting from the tables column last - changing the schema so that the foreign keys have ON DELETE CASCADE

Let me know if you'd be open to a PR that addresses this -- since foreign key constraints aren't enabled by default, I guess it's questionable whether this is a bug. I think I can workaround this by inspecting the database parameter in prepare_connection and trying not to enable fkey checks on the _internal database.

I'm trying to gate access to an edit UI on the user having update-row on the underlying view or table.

I expected datasette.check_visibility to be a good way to do this:

```python visible, private = await datasette.check_visibility( request.actor, permissions=[ ("update-row", (database, table)), ], )

if not visible:
    return None

```

But visible is returning true, even when there is no explicit update-row permission. (In this case, request.actor is None.)

Based on the update-row permissions docs, I expected this to be default deny, and so no explicit permission would result in false.

I think the root cause is that check_visibility calls ensure_permissions and expects it to throw if the permission is not available.

But ensure_permissions does not throw when permission_allowed returns None: https://github.com/simonw/datasette/blob/1.0a2/datasette/app.py#L825-L829

This modernizes some uses of double quotes for string literals to use only single quotes, fixes simonw/datasette#2001

While developing it, I manually enabled the stricter mode by using the code snippet at https://gist.github.com/cldellow/85bba507c314b127f85563869cd94820

I think that code snippet isn't generally safe/portable, so I haven't tried to automate it in the tests.

:books: Documentation preview :books:: https://datasette--2004.org.readthedocs.build/en/2004/

Related to #2001, but where #2001 was about literals, this is about identifiers

From https://www.sqlite.org/lang_keywords.html:

"keyword" A keyword in double-quotes is an identifier. [keyword] A keyword enclosed in square brackets is an identifier. This is not standard SQL. This quoting mechanism is used by MS Access and SQL Server and is included in SQLite for compatibility.

Datasette uses this quoting here -- https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/utils/init.py#L345-L349, in some of the other DB access code, and in some of the test fixtures.

Migrating to standard double quote identifiers would make it easier to get Datasette working with alternative backends

The presence of inner.* causes SQLite to materialize a row with all the columns. Those columns will be discarded later.

Instead, we can select only the column we'll use. This lets SQLite's optimizer realize that the other columns in the CTE definition aren't needed.

On a test table with 278K rows, 98K of which had an array, this speeds up the facet calculation from 4 sec to 1 sec.

:books: Documentation preview :books:: https://datasette--2008.org.readthedocs.build/en/2008/

From https://docs.datasette.io/en/stable/plugin_hooks.html#render-cell-row-value-column-table-database-datasette:

row - sqlite.Row The SQLite row object that the value being rendered is part of

This appears to actually be a CustomRow, but I think that's unrelated to my issue.

I have a table:

sql CREATE TABLE IF NOT EXISTS "dss_job_stats"( job_id integer not null references dss_job(id) on delete cascade, host text not null, // other columns elided as irrelevant primary key (job_id, host) );

On datasette 0.63.2, the render_cell hook receives a row value that looks like:

CustomRow([('job_id', {'value': 2, 'label': '2'}), ('host', 'cldellow.com')])

I expected the job_id value to be 2, but it's actually {'value': 2, 'label': '2'}.

I can work around this, but was wondering if this was intended behaviour?

I'm not sold that this is a good idea, but thought it'd be worth writing up a ticket. Proposal: add a hook like

python def rewrite_sql(datasette, database, request, fn, sql, params)

It would be called from Database.execute, Database.execute_write, Database.execute_write_script, Database.execute_write_many before running the user's SQL. fn would indicate which method was being used, in case that's relevant for the SQL inspection -- for example execute only permits a single statement.

The hook could return a SQL statement to be executed instead, or an async function to be awaited on that returned the SQL to be executed.

Plugins that could be written with this hook:

• https://github.com/cldellow/datasette-ersatz-table-valued-functions would use this to avoid monkey-patching
• a plugin to inspect and reject unsafe Spatialite function calls (reported by Simon in Discord)
• a plugin to do more general rewrites of queries to enforce table or row-level security, for example, based on the currently logged in actor's ID
• a plugin to maintain audit tables when users write to a table
• a plugin to cache expensive queries (eg the queries that drive facets) - these could allow stale reads if previously cached, then refresh them in an offline queue

Flaws with this idea:

execute_fn and execute_write_fn would not go through this hook, which limits the guarantees you can make about it for security purposes.

The ASGI spec (https://asgi.readthedocs.io/en/latest/specs/www.html#http) seems to imply that None is a valid value, so we need to check the value itself, not just whether the key is present.

In particular, the mangum adapter passes None for this key's value. This change permits mangum to be used to front datasette in Amazon API Gateway + AWS Lambda deployments.