test(qwp): assert proper treatment of empty and NULL arrays on egress

[mtopolnik]

What

Bug discovered in Java client: it didn't accept empty arrays from server on egress. The tandem PR in client repo addresses that. This PR contributes an end-to-end test for the behavior, and must also bump the client module pointer to the main branch after merging that PR.

Changes

• QwpEgressBootstrapTest.testEmptyAndNullArrayColumns: ingest a regular, an empty (ARRAY[]), and a NULL DOUBLE[] into a booted server and read them back via QwpQueryClient, asserting the empty array decodes as a non-null 0-element value distinct from NULL. Without the decoder fix this reproduces the egress rejection (ARRAY dim 0 must be >= 1: 0).

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e0245c85-738e-4440-ac1b-81f93315b4d1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mt_cleanup-null-empty-array

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[mtopolnik]

Review: PR #7276

Level 2. Scope: one new test method in QwpEgressBootstrapTest (in-diff) + a java-questdb-client submodule pointer bump that locks in the client-side decoder fix (out-of-diff, in the submodule). I reviewed both the test and the client fix it ships.

The code is essentially correct. The decoder fix is sound, the test exercises real server behavior, and resource handling is clean.

Critical

None.

Minor

M1 — testEmptyAndNullArrayColumns is in the wrong alphabetical position (in-diff)

It's inserted at line ~208, right after testArrayColumns (grouped by topic). Alphabetically it belongs between testDropRecreateRecompilesStaleFactory and testEmptyResultSet (i.e. just before line 709): testEmpty**A**ndNull… < testEmpty**R**esultSet. This violates the project rule ("insert new methods in the correct alphabetical position", "never group methods by logical category") — and your own recorded preference to sort test methods alphabetically rather than grouping by category. Move it down to the testEmpty… cluster.

M3 — Nit: optional multi-dimensional empty-array coverage

The e2e test covers the 1-D empty array. The decoder fix also added the per-dimension cap that matters for multi-dim frames with a 0 in one dimension and a large value in another. That hostile path is already covered by the client-side QwpResultBatchDecoderHardeningTest, so this is not a gap — but an end-to-end ARRAY[]::DOUBLE[][] case (if SQL-creatable) would exercise the multi-dim empty encoding through the real server. Optional, low value.

M4 — Nit: unbounded index into fixed-size result arrays

isNull[count[0]++] writes into length-3 arrays with no bound. If a server bug ever streamed >3 rows, this throws ArrayIndexOutOfBoundsException inside onBatch — which the outer catch (Throwable) converts to errMsg, so the test still fails (just with a murkier message than the explicit assertEquals(3, count[0])). Not a real risk; noting for completeness.

Summary

Verdict: approve the code

• The decoder fix is correct and complete; the test exercises genuine server behavior (server emits empty DOUBLE[] inline as non-null nDims=1, dim[0]=0; storage preserves empty-vs-null), is deterministic (monotonic designated timestamps + single partition), is a valid regression (fails on the old decoder with "ARRAY dim 0 must be >= 1: 0"), and adds value over the client-only unit test by validating the server↔client contract end to end.
• Findings: 1 critical (release sequencing, already on the author's checklist), 4 minor/nits. All in-diff or metadata. 6 draft concerns verified and dropped as false positives.
• No correctness, concurrency, resource-leak, or performance issues in the test or the fix.

[mtopolnik]

[PR Coverage check]

😍 pass : 5 / 8 (62.50%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/std/ConcurrentLongHashMap.java	0	1	00.00%
🔵	io/questdb/std/ConcurrentIntHashMap.java	0	1	00.00%
🔵	io/questdb/std/ConcurrentHashMap.java	0	1	00.00%
🔵	io/questdb/griffin/SqlException.java	5	5	100.00%

[mtopolnik]

/azp run macwin

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[bluestreak01]

/azp run macwin

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mtopolnik]

/azp run macwin

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).