[perfloop-agent]

Fixes #1845.

Description

Return Arrow's JSON buffer directly for streamed SQL batches instead of decoding it into a Value tree and encoding it again. Each batch still owns its buffer and ends with a newline. The field-name prefix and non-streaming responses are unchanged.

sendNull already has no effect on streaming responses because the per-batch field list is empty; null properties remain omitted. Remove the unused argument rather than change that behavior.

Numeric formatting now follows Arrow directly. For example, Float64 1e+300 becomes 1.0e300. Decimal output retains its exact precision and scale instead of passing through an f64 value (9.468 becomes 9.4680000000 in the covered example). Clients comparing raw JSON text may notice these differences.

A local paired run used the in-process query handler and drained the complete response, with eight default-size, 20,000-row batches per request. The constructed input has message, nullable tag, integer and boolean columns. The same release profile, inputs and one-request-at-a-time execution were used on both sides. The table includes all ten measured pairs after an excluded warmup; negative paired changes mean lower cost.

Metric	Before median	After median	Paired median change	Pairs
ns_per_batch	26717459.562	2340867.438	-24396936.188	10
allocation_calls_per_batch	180831.750	796.000	-180035.938	10

End-to-end check

Two release builds (cargo build --release) of 335835a (base) and this branch ran one at a time against the same data directory: parseable local-store <dir>, admin/admin, on 4 vCPUs (Intel Xeon @ 2.60GHz) and 8 GB RAM, Linux 6.1. One stream, access, holds 1,000,000 synthetic access-log rows (timestamp, method, path, status, bytes, latency_ms, user_agent, a 30%-null tag and a float), ingested through POST /api/v1/ingest in batches of 10,000. Each query drains the full response:

curl -s -u admin:admin -H 'Content-Type: application/json' \
  -d '{"query":"SELECT * FROM access","startTime":"2020-01-01T00:00:00.000Z","endTime":"2035-01-01T00:00:00.000Z"}' \
  -o /dev/null -w '%{time_total}\n' 'http://127.0.0.1:8000/api/v1/query?streaming=true'

7 interleaved base/branch pairs per query, one warmup per server start excluded:

Query	Base median	Branch median	Paired change
Streamed SELECT * (346 MB, 53 batches)	4.05 s	0.87 s	−78% (every pair −78% or −79%)
Non-streamed SELECT *	5.23 s	5.20 s	−1% (noise)
Streamed SELECT * LIMIT 1000	92 ms	89 ms	noise
GROUP BY status, streamed and non-streamed	87–89 ms	83 ms	noise

With ORDER BY, streamed output was byte-identical to base at 0 rows, 1 row, 20,000 rows (one batch), 20,001 and 40,000 rows, and for SELECT *. Without ORDER BY, every run returned all 1,000,000 rows with no duplicates. Peak server RSS on the streamed query fell from about 485 MB to about 400 MB; the non-streamed peak was unchanged. Four and eight concurrent streamed queries returned complete, identical results. A client that disconnects mid-stream and an execution error mid-stream behave as on base. cargo test passes (549 passed, 0 failed).

Streaming is enabled by the streaming=true query parameter on /api/v1/query.

Client applications, such as the Prism UI, were not measured.

These measurements cover elapsed handler-and-drain time and allocator calls, not network latency or storage scans. The outer runtime and table setup are outside timing; the first query initializes the dedicated query runtime inside the measured operation.

The query-handler unit tests passed: 3 passed, 0 failed, 0 ignored. They cover escaped rows, null omission, newline framing, direct numeric output, empty batches and error status. Library lint and formatting checks passed.

---

This PR has:

• been tested to ensure log ingestion and log query works.
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added documentation for new or modified features or behaviors.

Case and evidence

---

Made by Perfloop. Human sponsor: Tomás Senart. To ask for a change, comment or review in plain words. How Perfloop handles pull requests.

Summary by CodeRabbit

• Bug Fixes
  • Improved streaming query response serialization for null values, escaped strings, numeric values, and empty batches.
  • Streaming and serialization errors now return internal server errors.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The streaming query handler now serializes Arrow record batches with Arrow’s JSON array writer and appends a newline to each output. It maps serialization and query errors to internal-server errors. Tests cover JSON values, formatting, empty batches, and errors.

Changes

Streamed Query Serialization

Layer / File(s)	Summary
Serialize streamed batches src/handlers/http/query.rs	The streaming branches no longer pass send_null to the batch processor. The processor serializes batches with Arrow’s JSON array writer, appends a newline, and maps serialization and query errors to internal-server errors. Tests cover nulls, escaped and Unicode strings, numeric formatting, empty batches, and errors.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: nikhilsinhaparseable, parmesant

Merge Risk: Minimal · up to e9497

Streamed batches now use Arrow’s JSON output directly. No actionable code risk is established, but contributors still need to satisfy the CLA check.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change: avoiding re-parsing of streamed query batches.
Linked Issues check		Issue #1845 is the only directly linked issue, and it is open. Its coding requirements are met by the diff to src/handlers/http/query.rs (base 335835a to head e94970e). 1. Return the owned Arrow JSON…
Out of Scope Changes check		All changes are in src/handlers/http/query.rs and connect to issue #1845: - Import of info and an info!("Returning query results") call per streamed batch. This is a logging line in the changed ba…
Description check		The description is detailed and follows the required template. It states the issue, intent, design, behavior changes, testing, performance results, and checklist status. The log-ingestion/log-query an…

Full details: Linked Issues check

Explanation

Issue #1845 is the only directly linked issue, and it is open. Its coding requirements are met by the diff to src/handlers/http/query.rs (base 335835a to head e94970e).

1. Return the owned Arrow JSON buffer per streamed batch, with the batch array and terminal newline: create_batch_processor runs arrow_json::ArrayWriter on each RecordBatch, calls finish, takes into_inner(), and appends b'\n'. The serde decode and re-encode through QueryResponse::to_json is removed from this path.
2. Null properties stay omitted, including when sendNull is set: the streaming handler no longer passes send_null to the batch processor. The per-batch field list is empty, so the old path also omitted nulls. The new test streaming_batch_preserves_nulls_and_newline checks output equality with the previous QueryResponse output for both send_null values.
3. Non-streaming conversion and the field-name prefix are unchanged: the with_fields branch still builds fields_chunk and only the batch processor call changes. No non-streaming code appears in the diff.
4. Scope limits: the diff does not change storage, execution, or response buffering. The issue notes that Arrow number formatting may differ. The new test streaming_batch_keeps_arrow_numbers covers 1.0e300 and decimal 9.4680000000, which matches that note.

Automated tests were added for nulls, escaped strings, newline framing, Arrow numeric output, empty batches, and error status. I did not run them. The test expectations are consistent with the code.

Full details: Out of Scope Changes check

Explanation

All changes are in src/handlers/http/query.rs and connect to issue #1845:

• Import of info and an info!("Returning query results") call per streamed batch. This is a logging line in the changed batch processor. It is minor and does not alter behavior beyond log volume.
• Removal of the send_null variable and parameter from the streaming path. This follows the issue requirement and the PR description, which says the argument has no effect on streaming responses.
• Replacement of QueryResponse serialization with ArrayWriter in create_batch_processor. This is the requested change.
• Unit tests in a new tests module. They support the requested behavior and cover the stated edge cases.

No unrelated files or behavior changes were found in the reviewed diff. The raw summary lists only query.rs as changed, and I did not separately enumerate every file in the whole-PR diff.

Full details: Description check

Explanation

The description is detailed and follows the required template. It states the issue, intent, design, behavior changes, testing, performance results, and checklist status. The log-ingestion/log-query and documentation checklist items remain unchecked, although the description reports query and ingestion validation, so the checklist should be aligned with that evidence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

_{A rabbit watched the batches flow,
With newline trails in tidy rows.
Arrow wrote each value bright,
While escaped strings took flight.
The rabbit thumped: “The stream looks right!”}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1f420a4d-fc8b-4f95-bb4f-d53a909ef4cc

Commits

Reviewing files that changed from the base of the PR and between 335835a and e94970e.

Files selected for processing: 1

• src/handlers/http/query.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

[perfloop-agent]

I have read the CLA Document and I hereby sign the CLA