feat(databases): load a json file in whatever shape it holds - #294
Conversation
`--file data.json` only worked when the file was already newline-delimited. The json people actually have is usually an array of objects — what an HTTP API returns, what `jq` writes — or a single pretty-printed document, and both failed with a parser message about the bytes rather than the shape: "failed to infer JSON schema: EOF while parsing a list". Dropping the extension didn't help either; the server reads unrecognised bytes as csv, and a leading `[` then fails as a one-column csv. So loading json meant converting it first, where a csv or parquet of the same data needed nothing. A json source is now reshaped locally before upload: an array's elements, a pretty-printed document, and concatenated values all become one compact object per line, which is what the load reads. The rewrite streams — rows are read and written one at a time, so a 28 MB array costs one row of memory, not the document. An already-newline-delimited file is uploaded untouched: the shape is read off the first record, so a large `.jsonl` is never rewritten to say the same thing. A file that plainly opens a json array is taken as json even with no extension to say so, which is the `--url`-without-a-file-name case; the load then sends `format: json` for the rewrite it actually uploaded. Two shapes a load can't take are now refused before anything uploads, naming the shape rather than the parse: a row that is not an object, and a file with no rows at all.
README, SKILL.md, and the workflow guide all described json as newline-delimited only, which was the constraint the reshape removes. The `databases create` hint still suggested `--file <path.parquet>` as the one thing to load. Adds a worked example loading a csv and a json array on the same command, run verbatim against the API.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Review follow-ups.
The reshape round-tripped every row through `serde_json::Value`, which changes
two things a load can see. It sorts an object's keys — a `BTreeMap`, since
`preserve_order` is off — so `{"z":1,"a":2}` uploaded as `{"a":2,"z":1}` and
the columns came back alphabetical instead of in source order. And it parses a
number wider than i64/u64 into f64, so `{"id":123456789012345678901}` uploaded
as `1.2345678901234568e20`.
Rows are now written as the JSON text they already are, with only the
whitespace between tokens dropped: key order, number digits, and string
contents all survive. `serde_json`'s `raw_value` feature carries each row's own
text; unlike `arbitrary_precision`, it changes nothing for code that does not
name `RawValue`.
Verified against the API: a json array holding
`{"id": <21 digits>, "d": <23 digits>, "z": 1, "a": 2}` now lands with the same
columns in the same order as the same record loaded as untouched ndjson. The
values still round there — the load itself reads json numbers as f64, whatever
the shape — which is now recorded in SKILL.md rather than being something the
reshape adds.
That also settles what the 64 KiB sniff can cost. A first record wider than the
sniff reads as a stream of values and is rewritten, and the rewrite now
reproduces those rows exactly, so the fast path is about speed alone.
Documents the other refusal a caller can hit: a source with no rows (`[]`, or
an empty file) is refused locally, since a load cannot infer a schema from
nothing.
There was a problem hiding this comment.
All three prior threads are resolved in 00c6b1b9: the rewrite now copies each row's own JSON text, so key order and number digits survive, and SKILL.md records both the empty-source refusal and the load's f64 number handling.
The new write_compact string/escape tracking is correct for valid JSON: a multi-byte UTF-8 byte never matches ", \\, or ASCII whitespace, and a raw form feed cannot appear outside a string. One super nit inline.
CI test and fmt had not reported when this review started, so the new tests are unverified here.
There was a problem hiding this comment.
Prior threads are resolved: rows are now rewritten as their own JSON text (key order and number digits preserved), the empty-source refusal is documented in SKILL.md, and the unreachable clamp is gone. One non-blocking nit on temp-file lifetime for --url.
Review follow-up. A rewritten `--url` load kept both copies on disk for the length of the upload: `upload_temp_file` deletes the download when its closure returns, and the closure held the whole upload. A 5 GB json array therefore needed 10 GB of temp space and failed with ENOSPC on a small runner. `download_to_upload` now hands back the file to upload and deletes the other one first, so only the rewrite is on disk while it goes up. The download is kept when nothing replaced it — it is then the thing being uploaded, which is why this is not simply a delete-before-upload — and it is deleted before an error returns, since the caller exits without unwinding. `--file` keeps both, deliberately: there the second copy is the user's own source file, not one this code staged.
There was a problem hiding this comment.
Prior threads are all resolved in code. download_to_upload handles the three arms correctly: a rewrite drops the download first, a download that was not rewritten survives to be uploaded, and an error drops the download before returning. Content type resolution matches the file that goes up on both paths.
Note: CI / test was still queued when this review ran, so the new lifetime tests have no reported result.
--file data.jsononly worked when the file was already newline-delimited; an array of objects or a pretty-printed document failed with a server-side parse error. A json source is now reshaped to newline-delimited json locally before upload (streaming, so a large array costs one row of memory), and an already-newline-delimited file is uploaded untouched.