insert --convert: validate the result instead of falling back on falsy

[feiiiiii5]

The JSON/CSV branch of --convert used fn(doc) or doc, so any falsy result was thrown away and the unconverted row inserted instead — exit code 0, no warning.

$ echo '[{"id": 1, "name": "Cleo"}]' | sqlite-utils insert d.db t - --convert '{}'
$ sqlite-utils d.db "select * from t"
[{"id": 1, "name": "Cleo"}]        # the original row, not {}

The realistic shape of this is a filter:

$ echo '[{"id":1,"ok":true},{"id":2,"ok":false},{"id":3,"ok":true}]' \
    | sqlite-utils insert d.db t - --convert 'row if row["ok"] else {}'
$ sqlite-utils d.db "select id, ok from t"
1, 1
2, 0                            # meant to be dropped
3, 1

Every row is inserted, including the ones the filter was written to drop, and nothing says so.

--lines already rejects the same input

Twelve lines earlier the --lines branch has no such fallback, so the identical --convert behaves differently depending only on the input format:

$ ... --convert 0   # JSON input   -> exit 0, rows inserted
$ ... --lines --convert 0           -> exit 1, "Rows must all be dictionaries, got: 0"

Only None should fall back

test_insert_convert_error_messages already asserts that a non-dict return is rejected — --convert 1 errors. 0, False, [] and "" were the outliers, and only because they are falsy.

None is the one falsy result that has to keep meaning "modify doc in place": a --convert that is a bare statement returns it, which is what test_insert_convert_row_modifying_in_place covers with --convert 'row["is_chicken"] = True'. So the fallback is keyed on is None rather than truthiness:

def convert_doc(doc, fn=fn):
    result = fn(doc)
    return doc if result is None else result

What changes for existing users

A --convert that returned a truthy dict, or that mutated in place, is unaffected. A --convert that returned {}, 0, "", [] or False previously inserted the input row unchanged and will now either insert what you returned or fail with the same error --convert 1 already produced. That is a behaviour change for anyone who relied on the silent fallback — I cannot think of a case where relying on it was intended, but it is a change and worth stating.

One thing this does not solve: there is no way to drop a row. --convert 'row if row["ok"] else {}' inserts a row of nulls rather than skipping it, both before and after. I have left that alone as it is a separate design question.

Testing

Testing

Seven cases in tests/test_cli_insert.py, all failing on 6bc1d33:

• test_insert_convert_falsy_result_is_rejected — 0, "", [], False
• test_insert_convert_returning_none_still_inserts_the_row — pins the in-place signal
• test_insert_convert_returning_empty_dict_does_not_insert_the_original
• test_lines_and_json_convert_reject_the_same_falsy_result — the two formats agree

Suite: 1505 passed, 16 skipped. black --check, flake8 and mypy clean. pyright reports one error and ty two diagnostics, but all three are identical on unmodified main (tests/test_constructor.py:74, and two unused-type: ignore comments in db.py/utils.py) and none are in the files this touches.

---

📚 Documentation preview 📚: https://sqlite-utils--889.org.readthedocs.build/en/889/

[feiiiiii5]

Hi @simonw — three of mine across your repos have been open since 26–28 September, all green, so this is one message rather than three pings.

sqlite-utils #889 — insert --convert used fn(doc) or doc, so a conversion returning a falsy value (0, {}, "", []) was thrown away and the unconverted row inserted instead, exit code 0, no warning. The realistic shape is a filter: --convert 'row if row["ok"] else {}' currently inserts every row, including the ones the filter was written to drop. The fix validates the result and keeps falsy values that the conversion produced on purpose. --lines already rejects the same input, which is the symmetry the description points at.

sqlite-utils #888 — analyze_column() divides by the non-null count but indexes the per-value histogram by the raw row index, so a null anywhere before a value shifts every common-value percentage; with a null in the first row the most common value's percentage is attributed to a different value.

datasette #2968 — an empty-string keyset pagination cursor is dropped by if last_id: instead of if last_id is not None, so a table keyed on an empty string silently restarts from the first page and serves the same rows again.

The question I would most like settled is on #889, because it is the one with two defensible answers: right now a conversion that returns a falsy value is kept, which means a filter that wants to drop a row has no way to say so (None is the "drop" marker, and {} is a legitimately empty document). Should None stay the only drop marker with everything else kept — which is what the PR does — or should there also be a documented way to drop, e.g. raising? I chose the former because it is what the CLI can express today and it makes the current behaviour lossless; happy to add a drop marker if you would rather have one.