Skip to content

Keep form-feed blank lines out of zero-copy NDJSON docs - #6959

Draft
Mallets wants to merge 1 commit into
quickwit-oss:mainfrom
Mallets:mallets/ndjson-form-feed-blank-lines
Draft

Mallets wants to merge 1 commit into
quickwit-oss:mainfrom
Mallets:mallets/ndjson-form-feed-blank-lines

Conversation

@Mallets

@Mallets Mallets commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #6869. doc_batch_v2_from_ndjson stays zero-copy by attaching skipped blank lines to neighboring docs. Blank lines are detected with is_ascii_whitespace, which includes form feed (\x0c), but JSON does not accept form feed as whitespace.

Before #6869, the ingest v2 REST handler dropped blank lines, so \x0c\n{"id":1}\n ingested one valid doc. Now the doc is \x0c\n{"id":1}\n, which fails validation as invalid JSON. Trailing form-feed lines have the same effect on the last doc.

Fix: when a blank line contains a form feed, fall back to copying only the non-blank lines (same output as split_ndjson_lines). Bodies without form feeds keep the zero-copy path; the check is a single memchr for them.

How was this PR tested?

  • Added test_doc_batch_v2_from_ndjson_form_feed_blank_lines (leading, inner, and trailing form-feed blank lines produce valid JSON docs with increasing doc UIDs) and a form-feed-only body case to test_doc_batch_v2_from_blank_ndjson.
  • cargo test -p quickwit-ingest ndjson --lib: 6 passed.
  • cargo clippy -p quickwit-ingest --tests and cargo fmt --check: clean.

doc_batch_v2_from_ndjson attaches skipped blank lines to neighboring
docs to stay zero-copy. Blank lines are detected with is_ascii_whitespace,
which includes form feed, but JSON does not accept form feed as
whitespace: a body with a form-feed-only line made its neighboring doc
invalid JSON, whereas the previous line splitter dropped such lines.

Fall back to copying the non-blank lines when a blank line contains a
form feed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant