mirror of
https://github.com/simonw/sqlite-utils.git
synced 2026-10-23 01:37:11 +02:00
insert --convert: validate the result instead of falling back on falsy
The JSON/CSV branch used 'fn(doc) or doc', so any falsy result was
discarded and the unconverted row inserted. Exit code 0, no warning:
$ echo '[{"id": 1, "name": "Cleo"}]' | sqlite-utils insert d.db t - --convert '{}'
$ sqlite-utils 'select * from d.db' # -> the original row, not {}
The realistic shape of this is a filter:
--convert 'row if row["ok"] else {}' # meant to drop the bad rows
# every row inserted, including the ones meant to be dropped
'--lines' runs a different expression twelve lines up and has no such
fallback, so the same --convert code is rejected there and silently
accepted here:
--convert 0 JSON input -> exit 0, rows inserted
--convert 0 --lines -> exit 1, Rows must all be dictionaries, got: 0
test_insert_convert_error_messages already asserts that a non-dict
return is rejected (--convert 1); 0, False, [] and "" were the outliers,
purely 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
test_insert_convert_row_modifying_in_place covers - so the fallback is
now keyed on 'is None' rather than truthiness.
Tests: 7 new cases in tests/test_cli_insert.py, all failing on 6bc1d33.
Suite 1505 passed, 16 skipped. black, flake8, mypy clean. pyright
reports one error and ty two diagnostics, both identical on unmodified
main (tests/test_constructor.py:74 and two unused-type-ignore comments
in db.py/utils.py) and none in the files this touches.
This commit is contained in:
parent
6bc1d33d58
commit
210c119907
2 changed files with 80 additions and 1 deletions
|
|
@ -1312,7 +1312,19 @@ def insert_upsert_implementation(
|
|||
"--convert must return dict or iterator"
|
||||
)
|
||||
else:
|
||||
docs = (fn(doc) or doc for doc in docs)
|
||||
|
||||
def convert_doc(doc, fn=fn):
|
||||
# A --convert expression that is a statement returns
|
||||
# None, and means "modify doc in place", so None is
|
||||
# the one falsy result that falls back to the
|
||||
# original row. Any other falsy result is a value
|
||||
# the caller returned deliberately and has to be
|
||||
# validated like --lines already does, rather than
|
||||
# silently inserting the row unconverted.
|
||||
result = fn(doc)
|
||||
return doc if result is None else result
|
||||
|
||||
docs = (convert_doc(doc) for doc in docs)
|
||||
|
||||
_insert_docs(docs, tracker=tracker)
|
||||
|
||||
|
|
|
|||
|
|
@ -530,6 +530,73 @@ def test_insert_convert_row_modifying_in_place(db_path):
|
|||
assert rows == [{"name": "Azi", "is_chicken": 1}]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"convert,expected_value",
|
||||
(
|
||||
("0", "0"),
|
||||
('""', "''"),
|
||||
("[]", "[]"),
|
||||
("False", "False"),
|
||||
),
|
||||
)
|
||||
def test_insert_convert_falsy_result_is_rejected(db_path, convert, expected_value):
|
||||
# A --convert expression that is a statement returns None and is
|
||||
# documented to modify row in place. Every other falsy value used to
|
||||
# be treated the same way, so the row was inserted unconverted.
|
||||
result = CliRunner().invoke(
|
||||
cli.cli,
|
||||
["insert", db_path, "rows", "-", "--convert", convert],
|
||||
input='{"name": "Azi"}',
|
||||
)
|
||||
assert result.exit_code == 1
|
||||
assert result.output == (
|
||||
f"Error: Rows must all be dictionaries, got: {expected_value}\n"
|
||||
)
|
||||
|
||||
|
||||
def test_insert_convert_returning_none_still_inserts_the_row(db_path):
|
||||
# None stays the in-place signal: a --convert that is a bare
|
||||
# statement returns it, and the row is inserted as it arrived.
|
||||
result = CliRunner().invoke(
|
||||
cli.cli,
|
||||
["insert", db_path, "rows", "-", "--convert", "None"],
|
||||
input='{"name": "Azi"}',
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
db = Database(db_path)
|
||||
assert list(db.query("select * from rows")) == [{"name": "Azi"}]
|
||||
|
||||
|
||||
def test_insert_convert_returning_empty_dict_does_not_insert_the_original(db_path):
|
||||
# {} is a dict, so it is accepted - but it used to be discarded as
|
||||
# falsy and the unconverted row inserted in its place.
|
||||
result = CliRunner().invoke(
|
||||
cli.cli,
|
||||
["insert", db_path, "rows", "-", "--convert", 'row if row["id"] else {}'],
|
||||
input='[{"id": 1, "name": "Azi"}, {"id": 0, "name": "Not me"}]',
|
||||
)
|
||||
assert result.exit_code == 0, result.output
|
||||
db = Database(db_path)
|
||||
# no ORDER BY: the empty row sorts first under "order by id"
|
||||
assert list(db.query("select * from rows")) == [
|
||||
{"id": 1, "name": "Azi"},
|
||||
{"id": None, "name": None},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("convert", ("0", "{}"))
|
||||
def test_lines_and_json_convert_reject_the_same_falsy_result(db_path, convert):
|
||||
# The two input formats run different expressions, so the same
|
||||
# --convert code has to behave the same way in both
|
||||
for options in ([], ["--lines"]):
|
||||
result = CliRunner().invoke(
|
||||
cli.cli,
|
||||
["insert", db_path, "rows", "-", *options, "--convert", convert],
|
||||
input='{"name": "Azi"}',
|
||||
)
|
||||
assert result.exit_code == 1, (options, convert, result.output)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"options,expected_error",
|
||||
(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue