fix: make send_batch set-based

[NikolayS]

Summary

Replace pgque.send_batch()'s PL/pgSQL per-payload loop over pgque.insert_event() with set-based inserts into the current queue event table.

This makes batching real on the server side:

• resolve queue metadata once;
• allocate event ids with nextval(...) set-wise;
• insert all payloads with one dynamic insert into ... select from unnest(...);
• return event ids in input order;
• preserve JSON canonicalization (jsonb::text) and text fast-path semantics;
• preserve queue disabled / replica checks.

Why

Before this PR, send_batch() reduced client round-trips but still did N internal calls to insert_event(), so it could become the producer bottleneck.

Tests

RED/GREEN added to tests/test_api_send.sql:

• structural regression: function body must not contain FOREACH or call pgque.insert_event(...);
• functional regression: 1000 JSON payload batch inserts 1000 rows and preserves returned-id / payload order.

Local gates run:

psql -v ON_ERROR_STOP=1 -d "$DB" -f tests/test_api_send.sql
# PASS, including new set-based checks

psql -v ON_ERROR_STOP=1 -d "$DB" -f tests/run_all.sql
# === ALL TESTS PASSED ===

cd clients/go && go test ./...
# ok

cd clients/typescript && PGQUE_TEST_DSN=... npm test
# 22 passed

[NikolayS]

REV Code Review Report

• PR: NikolayS/pgque#159 — fix: make send_batch set-based
• Author: @NikolayS
• Branch: fix/send-batch-set-based
• CI: ✅ all 8 checks passed (pg14–18, verify, client-smoke, claude-review)

---

BLOCKING ISSUES (8)

HIGH tests/test_api_send.sql — text[] overload order preservation not tested

Test 3f validates 1000-item order preservation for the jsonb[] overload only. The text[] overload has its own unnest($2::text[]) path and is never verified for ev_id/payload order correctness at any batch size.
Fix: Add a parallel test calling send_batch('test_send', 'batch.text.large', array(select 'payload-'||g from generate_series(1,1000) g)::text[]), assert cardinality = 1000, and verify the suffix integer in each stored payload matches the ordinal from v_ids.

HIGH tests/test_api_send.sql — no test for queue-not-found error path

Both overloads raise exception 'queue not found: %' when the queue lookup returns not found. No test passes a bogus queue name and asserts the exception. Test 3c covers queue_disable_insert, not a missing queue.
Fix: Add a test wrapping send_batch('no_such_queue', 'x', array['p']::text[]) in a BEGIN/EXCEPTION block and asserting a P0001 / "queue not found" error is raised. Mirror for the jsonb[] overload.

MEDIUM tests/test_api_send.sql — NULL array input behavior untested

Passing NULL::jsonb[] or NULL::text[] causes unnest(NULL) to return zero rows, so the function silently returns '{}'::bigint[]. Test 3d only covers zero-length non-null arrays. The NULL-vs-empty distinction is a meaningful API contract.
Fix: Add tests calling send_batch('test_send', 'batch.null', NULL::jsonb[]) and NULL::text[], asserting = '{}'::bigint[] (or asserting an exception if the spec intends that).

MEDIUM tests/test_api_send.sql — ev_type round-trip not verified for text[] overload

Test 3f queries where ev_type = $2 for the jsonb[] path. The text[] happy-path test (Test 3c) only checks returned array length, not whether ev_type was written correctly. A swapped $4 bind parameter in the dynamic SQL would go undetected.
Fix: In Test 3c (or a new 3c2), after send_batch(text[]), query the event table with where ev_id = any(v_ids) and ev_type = 'batch.text' and assert count = 3.

MEDIUM tests/test_api_send.sql — replica-role bypass of disable_insert is untested

Both overloads allow inserts when queue_disable_insert = true AND session_replication_role = 'replica'. No test exercises this path, so the <> vs = guard logic could be inverted without CI catching it.
Fix: Add a test that sets session_replication_role = replica, calls send_batch() on a disabled queue, and asserts a non-empty id array is returned.

MEDIUM sql/pgque-api/send.sql — queue_id fetched into qstate but never used (both overloads)

Both send_batch overloads include select q.queue_id, ... but qstate.queue_id is never referenced after the into qstate. Only cur_table_name, queue_event_seq, and queue_disable_insert are used. Dead column select adds noise and misleads readers.
Fix: Remove q.queue_id, from the SELECT in both the jsonb[] and text[] overloads (and correspondingly in sql/pgque.sql).

MEDIUM docs/reference.md:53 — "one SQL statement / transaction" conflates two concepts

Current text: "inserts all elements of payloads into queue in one SQL statement / transaction." A transaction and a statement are not interchangeable; the function also executes a SELECT to fetch queue state, so it is not a single statement. The slash conflation is technically ambiguous.
Fix: "inserts all elements of payloads into queue in a single INSERT statement (no per-row function calls) within a single transaction."

MEDIUM docs/reference.md:53,63 — empty-array behavior not documented for either overload

The PR adds explicit coalesce(array_agg(…), '{}'::bigint[]) to guarantee empty input returns '{}'::bigint[] (not NULL), and Test 3d verifies this. Neither overload's description mentions this contract.
Fix: Add to each overload: "Passing an empty array returns '{}'::bigint[], not NULL."

---

NON-BLOCKING (2)

LOW tests/test_api_send.sql — single-element batch not covered

Smallest tested batch is 3 elements. A 1-element batch is a distinct boundary case for with ordinality (ord=1), array_agg of one row, and the join using (ev_id) CTE join.
Suggestion: Add send_batch('test_send', 'single', array['{"n":1}'::jsonb]) with assert cardinality(v_ids) = 1 and v_ids[1] is not null. Mirror for text[].

LOW docs/reference.md:21 — preamble claim "All send* functions reduce to pgque.insert_event" is now false

Test 3e explicitly asserts send_batch must NOT call insert_event. The preamble at line 21 is stale.
Suggestion: Update to: "Single-event send* functions reduce to pgque.insert_event. send_batch bypasses that wrapper and issues a single direct INSERT for throughput."

---

POTENTIAL ISSUES (2)

LOW tests/test_api_send.sql — TOCTOU race between metadata read and INSERT not tested (confidence: 6/10)

send_batch reads queue_cur_table then inserts into the computed table. If the ticker rotates the table between those steps, the insert lands in a stale table. No test exercises send_batch across a rotation boundary.
Suggestion: Sequential test: insert via send_batch, force rotation via pgque.maint(), call send_batch again, confirm the second batch's IDs appear in the new current table.

LOW docs/benchmarks.md:46–47 — "PL/pgSQL batched" rows predate the set-based optimization (confidence: 7/10)

The benchmarks show the old loop-based implementation numbers. No note flags them as pre-set-based.
Suggestion: Add a footnote marking those rows as pre-PR-159, or add a new row once set-based numbers are available.

---

Summary

Area	Blocking	Non-blocking	Potential	Filtered
CI	—	—	—	—
Security	0	0	0	0
Bugs	0	0	0	0
Tests	5	1	1	0
Guidelines	1	0	0	0
Docs	2	1	1	0

---

REV-assisted review (AI analysis by postgres-ai/rev)

[NikolayS]

REV Code Review Report

• PR: NikolayS/pgque#159 — fix: make send_batch set-based
• Author: @NikolayS
• Branch: fix/send-batch-set-based
• CI: ⏳ running (PG 14/15/17 + client-smoke in progress; PG 16/18 + verify passed)

---

BLOCKING ISSUES (8)

HIGH sql/pgque-api/send.sql — Design rule #3 violated: send_batch bypasses PgQ primitives entirely

Both overloads now directly query pgque.queue, replicate the disable_insert guard, and issue their own INSERT — logic that insert_event_raw already owns. CLAUDE.md §4 requires API functions to reduce cleanly to PgQ primitives. pgque.current_event_table(queue_name) already performs the disable-insert check and table-name lookup; it could anchor the set-based INSERT against a PgQ primitive rather than duplicating the guard logic inline.
Fix: Either call pgque.current_event_table(i_queue) to obtain the table name (routing through the PgQ primitive), or explicitly document in the function and CLAUDE.md that send_batch is a deliberate exception to rule #3 with a stated reason.

HIGH tests/test_api_send.sql — TDD rule violated: tests added in same commit as implementation

CLAUDE.md §"Red/green TDD" requires a failing test commit before the implementation. Test 3e (structural check) and Test 3f (1000-row order) both landed in the same commit as the new function bodies (bc0b112). No prior "red" commit exists in history for these tests.
Fix: For future changes of this class, push the failing assertion first (CI must be red), then push the implementation. Retroactive split of bc0b112 is optional.

HIGH tests/test_api_send.sql — No large-batch order/correctness test for the text[] overload

Test 3f sends 1000 jsonb[] payloads and verifies returned IDs and payload order. The text[] overload has its own independent CTE implementation in send.sql — its ordering and payload-integrity behaviour under large batches is never exercised. The existing 3-item Test 3c does not substitute.
Fix: Add a Test 3f-text equivalent: send array(select 'item-' || g from generate_series(1,1000) g)::text[], verify cardinality, and check that ev_data for v_ids[ord] equals 'item-' || ord.

HIGH tests/test_api_send.sql — The "queue not found" error path is never tested

Both Test 3d entries call send_batch('missing_empty_queue', ..., array[]::...) which hits the early-return guard at cardinality = 0 before the SELECT FROM pgque.queue is ever reached. The raise exception 'queue not found: %' branch is completely untested. Any regression there would go undetected.
Fix: Add a test that calls send_batch('nonexistent_queue', 'x', array['payload']::text[]) inside a BEGIN/EXCEPTION block and asserts sqlerrm = 'queue not found: nonexistent_queue'. Mirror for the jsonb[] overload.

MEDIUM docs/reference.md:53 — NULL rejection and empty-array semantics change not documented

Two behavior changes from this PR are not reflected in the reference: (1) NULL array now raises 'payloads must not be null' (the old FOREACH also errored, but the error message was different); (2) an empty array returns {} immediately without validating queue existence — send_batch('nonexistent', 'x', array[]::text[]) silently succeeds. These are API-level semantics that callers need to know.
Fix: Add to both send_batch entries: "Raises an exception if payloads is NULL. Returns {} immediately for an empty array (queue existence is not checked in that case)."

MEDIUM sql/pgque-api/send.sql — ~95% code duplication between the two overloads

The jsonb[] (lines ~86–148) and text[] (~150–213) variants are byte-for-byte identical except for 3 lines out of ~63: the comment, the parameter type, and the unnest cast. All guard logic, the pgque.queue SELECT, the disable_insert check, and the full CTE/INSERT/SELECT block are duplicated. Any future fix (e.g. to the TOCTOU or nextval ordering) must be applied twice.
Fix: Have the jsonb[] overload cast its input and delegate to the text[] overload: return pgque.send_batch(i_queue, i_type, array(select el::text from unnest(i_payloads) el)). Or extract a pgque._send_batch_internal helper.

MEDIUM tests/test_api_send.sql — Test 3d comment is misleading; missing complement test

The label "missing queue + empty input should return empty array" implies graceful handling of missing queues. It actually works only because the early-return fires before the queue lookup — the queue is never checked. If the guard order changes, the test comment describes wrong expected behavior.
Fix: Add a comment explaining the early-return mechanism. Add a companion test asserting send_batch('nonexistent_queue', ...) with a non-empty payload raises "queue not found" (covered by the HIGH finding above; fixing that also resolves this).

MEDIUM tests/test_api_send.sql:185 — Test 3f does not assert monotonically increasing IDs

The test joins v_ids[ord] to ev_data->>'n' == ord, confirming order-preservation, but never verifies that v_ids[1] < v_ids[2] < ... < v_ids[1000]. A broken implementation that shuffled IDs while still matching them to correct payloads would pass. The monotonic assignment is the key invariant of nextval() inside the numbered CTE.
Fix: After the v_bad_order assertion, add: assert (select bool_and(v_ids[i] < v_ids[i+1]) from generate_subscripts(v_ids,1) i where i < cardinality(v_ids)) = true, 'IDs must be monotonically increasing';

---

NON-BLOCKING (4)

LOW blueprints/SPECx.md — send_batch entry still says "Loop of insert_event() calls"

The spec table is now out of sync with the implementation.
Suggestion: Update to: "Single-statement INSERT via unnest + nextval, within the caller's transaction."

LOW docs/reference.md:53 — "in the same order" is ambiguous without a reference point

Both overload descriptions now say "in the same order" but omit "as the input array".
Suggestion: Change to "in the same order as the input array."

LOW tests/test_api_send.sql — queue_disable_insert guard in send_batch is untested

The if qstate.queue_disable_insert then raise exception 'Insert into queue disallowed' branch has no test coverage in the send-batch tests.
Suggestion: Add a test that sets queue_disable_insert = true, asserts the exception, then resets and confirms success.

LOW docs/reference.md:53 — "one SQL statement / transaction" phrasing is imprecise

The slash conflates two separate properties. The function executes within the caller's transaction; it issues one dynamic INSERT statement.
Suggestion: Rewrite as: "inserts all elements of payloads via a single INSERT statement, within the caller's transaction."

---

POTENTIAL ISSUES (6)

MEDIUM sql/pgque-api/send.sql — nextval() ordering relies on implicit unnest() array order (confidence: 7/10)

nextval($1::regclass) is called per-row during projection of the numbered CTE. The ORDER BY ord in that CTE reorders already-assigned (ord, ev_id) pairs but cannot change which row received which nextval() result. Monotonic ev_id-to-ord assignment currently holds because unnest(...) WITH ORDINALITY emits rows in array order — an implementation-level guarantee, not a SQL standard guarantee.
Suggestion: Accept the implicit guarantee and add a comment on the numbered CTE: -- MATERIALIZED + ORDER BY: nextval() is called in ord order before INSERT. Or use row_number() over (order by ord) with a single base nextval() call.

MEDIUM sql/pgque-api/send.sql:96 — Empty-array short-circuit changes error semantics (confidence: 7/10)

send_batch('nonexistent', 'type', array[]::jsonb[]) returns {} silently. The non-empty path raises "queue not found". Callers who probe queue existence with an empty batch will get a false positive.
Suggestion: Document this clearly (see MEDIUM BLOCKING above); optionally move the queue lookup before the cardinality check if consistent error semantics are preferred.

LOW tests/test_api_send.sql — No test for NULL elements within a non-null array (confidence: 7/10)

send_batch('q', 'x', array[null::jsonb]) succeeds and inserts a row with ev_data = NULL. This is silent data loss for callers who pass sparse arrays.
Suggestion: Add a test documenting whether NULL elements are accepted (and stored as NULL) or should be rejected.

LOW docs/reference.md:61 — send_batch(text[]) has no code example (confidence: 7/10)

Every other send* variant has a fenced SQL example; the text[] overload has only a one-line description.
Suggestion: Add: select pgque.send_batch('orders', 'order.created', array['msg-1', 'msg-2']);

LOW sql/pgque-api/send.sql:119 — MATERIALIZED hint lacks explanatory comment (confidence: 6/10)

The hint is correct and load-bearing for nextval() semantics, but future readers may not know why it's there.
Suggestion: Add -- MATERIALIZED: ensures nextval() runs once per row before the INSERT above the numbered CTE.

LOW sql/pgque-api/send.sql — Inherited TOCTOU race between queue metadata read and INSERT (confidence: 5/10)

Between the SELECT FROM pgque.queue and the dynamic INSERT, a table rotation could change the active event table. This race also exists in insert_event_raw — it is pre-existing, not introduced by this PR.
Suggestion: No action needed in this PR; track separately.

---

Summary

Area	Findings	Potential	Filtered
CI	—	—	—
Security	0	0	7
Bugs	0	3	2
Tests	4	2	1
Guidelines	3	2	1
Docs	1	2	1

---

REV-assisted review (AI analysis by postgres-ai/rev)

[NikolayS]

REV Code Review Report

• PR: NikolayS/pgque#159 — fix: make send_batch set-based
• Author: @NikolayS
• Branch: fix/send-batch-set-based
• CI: ⚠️ partial — PG 14–18 + verify passing; client-smoke and claude-review in progress

---

BLOCKING ISSUES (10)

CRITICAL sql/pgque-api/send.sql:125 — Subtransaction in hot path violates design rule #4

begin
    v_table_name := pgque.quote_fqname(pgque.current_event_table(i_queue));
exception when others then
    ...
end;

BEGIN … EXCEPTION WHEN OTHERS … END is a PL/pgSQL savepoint. CLAUDE.md rule #4 explicitly forbids subtransactions in event-insertion hot paths. Subtransactions carry savepoint + UNDO overhead, degrading the throughput this PR claims to improve.
Fix: Remove the subtransaction entirely. The queue-not-found case is already covered by the SELECT … INTO v_seq_name … IF NOT FOUND block that immediately follows. The queue_disable_insert check can be folded into that single SELECT from pgque.queue (the same approach used by insert_event_raw).

---

CRITICAL sql/pgque-api/send.sql:148 — Direct insert into event table bypasses insert_event primitive, violating design rule #3

execute format($sql$
    insert into %s (ev_id, ev_time, ev_owner, ev_retry, ev_type, ev_data, ...)
    select ev_id, $3, null, null, $4, ev_data, ...
$sql$, v_table_name)

Design rule #3: "Modern API functions must reduce cleanly to PgQ primitives." send_batch(text[]) now inserts directly into the event table, bypassing pgque.insert_event and insert_event_raw entirely. Any future validation, hooks, or behavior changes in those primitives are silently skipped.
Fix: Add a pgque.insert_event_bulk primitive to pgque-core that owns the set-based insert, and have send_batch delegate to it. Alternatively, document the intentional bypass with a rationale comment and update the reference.md invariant claim.

---

HIGH docs/reference.md:21 — Documented invariant "All send* functions reduce to pgque.insert_event" is now violated

The Publishing section intro states this as an architectural invariant, which is now false for send_batch. Callers and contributors relying on this guarantee will be misled.
Fix: Update the claim to scope it to single-row send*, or add: "send_batch uses a direct set-based insert for throughput; single-row send* reduce to pgque.insert_event."

---

HIGH sql/pgque-api/send.sql:127 — Exception dispatch on sqlerrm string comparison is fragile

exception when others then
    if sqlerrm = 'Event queue not found' then ...
    elsif sqlerrm = 'Writing to queue disabled' then ...
    else raise; end if;

Any rephrasing of these strings in current_event_table() silently falls through to raise, restoring raw PgQ error messages. WHEN OTHERS also catches transient errors (OOM, lock timeout) that should never be remapped.
Fix: Eliminate the subtransaction (see CRITICAL above) and check queue_disable_insert directly from pgque.queue. If the subtransaction is retained, route on SQLSTATE codes, not sqlerrm.

---

HIGH tests/test_api_send.sql — No end-to-end test: batch-inserted events are never received or acked

Tests 3f and 3g verify storage via direct table reads, bypassing the PgQ snapshot/receive path. A regression in get_batch_events, snapshot isolation, or the numbered JOIN ins logic could silently break consumer delivery while all current tests pass.
Fix: Add a test that calls send_batch, ticks the queue, calls next_batch/receive, and verifies every ev_id appears in the delivered batch with correct ev_type and ev_data.

---

HIGH tests/test_api_send.sql — Disabled queue (write-disabled) with non-empty batch has no test

The error mapping path (elsif sqlerrm = 'Writing to queue disabled') has zero test coverage. A typo in either the source exception message or the mapping guard would silently swallow or misroute the error.
Fix: Add a test that sets queue_disable_insert = true, calls send_batch with a non-empty array, and asserts sqlerrm = 'Insert into queue disallowed'.

---

HIGH tests/test_api_send.sql — NULL elements within a non-null array are untested and silently inserted

The NULL-array guard is tested, but array[null::text, 'foo']::text[] passes the guard and unnest emits a NULL ev_data row that is inserted without complaint. Whether this is acceptable or should be rejected is unspecified.
Fix: Add a test to lock down the behavior. If NULLs should be rejected: if array_position(i_payloads, null) is not null then raise exception 'payloads must not contain null elements'; end if;

---

MEDIUM sql/pgque-api/send.sql:169 — Final numbered JOIN ins can silently return {} if numbered CTE is re-evaluated

select coalesce(array_agg(numbered.ev_id order by numbered.ord), '{}'::bigint[])
  from numbered join ins using (ev_id)

If the planner re-evaluates numbered (calling nextval a second time), the join produces zero matches and COALESCE returns '{}'::bigint[] for a fully-inserted batch — silently losing all returned IDs.
Fix: Collect from ins alone: SELECT coalesce(array_agg(ev_id ORDER BY rn), '{}'::bigint[]) FROM (SELECT ev_id, row_number() OVER () AS rn FROM ins) s

---

MEDIUM tests/test_api_send.sql — Returned ID array monotonicity not asserted

Tests 3f/3g verify payload-to-ID correspondence but never assert that v_ids is strictly increasing. The ordering contract is not directly tested.
Fix: SELECT count(*) FROM generate_subscripts(v_ids, 1) s WHERE s.ord > 1 AND v_ids[s.ord] <= v_ids[s.ord - 1] and assert zero violations.

---

MEDIUM docs/reference.md:53,63 — Disabled-queue exception not documented in either send_batch overload

Both updated descriptions list only empty-array and NULL-array cases. A non-empty batch to a write-disabled queue raises Insert into queue disallowed — undocumented.
Fix: Add to both overload descriptions: "Non-empty arrays to a write-disabled queue raise Insert into queue disallowed."

---

NON-BLOCKING (3)

MEDIUM sql/pgque-api/send.sql:85 — jsonb[] overload allocates an intermediate text[] before delegating

The conversion builds a full text[] array in PL/pgSQL, which the text[] overload immediately unnests — pure overhead.
Suggestion: Fold the cast into the dynamic SQL (u.payload::text as ev_data inside the CTE) to avoid the intermediate allocation.

LOW tests/test_api_send.sql:42 — Small-batch tests don't assert returned IDs are distinct

array_length(v_ids, 1) = 3 passes even if all three values are identical.
Suggestion: Add (select count(distinct x) from unnest(v_ids) x) = 3.

LOW tests/test_api_send.sql:176 — Tests 3f/3g don't verify ev_owner, ev_retry, ev_extra* column defaults

Hard-coded null values for these PgQ-critical columns are not verified; an accidental change would break PgQ internals silently.
Suggestion: Add ev_owner is null AND ev_retry is null AND ev_extra1 is null to the direct table read assertions.

---

POTENTIAL ISSUES (5)

HIGH sql/pgque-api/send.sql:125 — TOCTOU: ticker can rotate event table between current_event_table() and the dynamic INSERT (confidence: 7/10)

The table name is resolved in one statement, the INSERT executes in another. A concurrent rotation could direct the insert to the pre-rotation table.
Suggestion: Derive v_table_name and v_seq_name in a single SELECT from pgque.queue (the pattern used by insert_event_raw).

HIGH sql/pgque-api/send.sql:137 — Redundant double-read of pgque.queue (once via current_event_table(), once for queue_event_seq) (confidence: 6/10)

A concurrent DROP QUEUE between these two reads can produce a misleading error path.
Suggestion: Single SELECT queue_data_pfx, queue_cur_table, queue_event_seq, queue_disable_insert FROM pgque.queue, eliminating the current_event_table() call entirely.

MEDIUM sql/pgque-api/send.sql:154 — nextval($1::regclass) resolves regclass per row (confidence: 7/10)

Pre-casting v_seq_name::regclass to an OID before EXECUTE amortizes the name→OID lookup to once per call.

MEDIUM tests/test_api_send.sql — Single-element batch not explicitly tested (confidence: 8/10)

The cardinality = 0 → set-based transition is tested only at cardinality ≥ 3 and 1000.

LOW tests/test_api_send.sql — Acceptance test us5_batch_load.sql uses per-row insert_event loop, not send_batch (confidence: 6/10)

send_batch is never exercised in the full producer→tick→receive→ack acceptance path.

---

Summary

Area	Findings	Potential	Filtered
CI	0	—	—
Security	1	2	1
Bugs	3	3	1
Tests	4	2	2
Guidelines	2	0	0
Docs	1	1	2

---

REV-assisted review (AI analysis by postgres-ai/rev)

[NikolayS]

REV Code Review Report

• PR: NikolayS/pgque#159 — fix: make send_batch set-based
• Author: @NikolayS
• Branch: fix/send-batch-set-based
• CI: ✅ passing (PG 14–18 + verify) · ⏳ client-smoke, claude-review in progress

---

BLOCKING ISSUES (8)

HIGH blueprints/SPECx.md:1166 — API summary table still says "Loop of insert_event() calls"

| pgque.send_batch(queue, type, payloads[]) | Loop of insert_event() calls | …
Fix: Update the "PgQ primitive underneath" cell to "pgque.insert_event_bulk() (set-based single INSERT)".

HIGH blueprints/SPECx.md:738-769 — §4.1 internal-mapping code block still shows the old FOREACH implementation with the TODO comment this PR resolves

Both send_batch listings (jsonb[] and text[]) show the FOREACH p IN ARRAY … insert_event() loop and the -- TODO: optimize to resolve queue/table once … comment.
Fix: Replace both code blocks with the new insert_event_bulk-delegating implementation, and add insert_event_bulk itself to §4.1 or a new §4.1.1 since reference.md now names it as a primitive.

HIGH tests/test_api_send.sql — pgque.insert_event_bulk has zero direct test coverage

All tests exercise it only through send_batch(). The wrapper's pre-flight guards (null check, cardinality == 0 short-circuit) mean insert_event_bulk is never called with boundary inputs directly.
Fix: Add at least one direct call: pgque.insert_event_bulk('test_send', 'bulk.direct', ARRAY['{}']::text[]) and test the 1-element case and an array containing a NULL element.

HIGH tests/test_api_send.sql — queue_disable_insert = true path in insert_event_bulk is completely untested

Lines 104–108 of send.sql contain the write-guard for disabled queues. This code path exists in the new primitive but has no coverage (grep for disable_insert and session_replication_role in tests/ returns nothing for the bulk path).
Fix: Add a test that sets queue_disable_insert = true, asserts send_batch() raises 'Insert into queue disallowed', and optionally verifies the session_replication_role = 'replica' bypass.

HIGH sql/pgque-api/send.sql:85-140 — insert_event_bulk duplicates insert_event_raw's queue-lookup and disable-check logic with no sync comment; violates CLAUDE.md Key Design Rule 3

The function reimplements the queue-lookup + disable-check + dynamic-INSERT pipeline directly rather than reducing to a PgQ primitive. Any future change to insert_event_raw (new column, new check) must be manually mirrored here.
Fix: Add a header comment acknowledging the deliberate divergence: -- NOTE: does not call insert_event_raw to avoid N nextval round-trips. Keep queue-lookup and disable-insert logic in sync with insert_event_raw.

HIGH sql/pgque-api/send.sql:199-212 — insert_event_bulk grant posture is undocumented and pgque_admin can call it directly, bypassing send_batch's null/cardinality guards

No GRANT EXECUTE is present for insert_event_bulk, but roles.sql has GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA pgque TO pgque_admin, so pgque_admin has direct access without the payloads must not be null or empty-array short-circuit protections.
Fix: Add a comment in the grant block (-- insert_event_bulk: internal primitive, no direct grant) and decide whether pgque_admin should be excluded (add REVOKE EXECUTE ON FUNCTION pgque.insert_event_bulk(text, text, text[]) FROM pgque_admin) or whether direct access is acceptable and should be documented.

MEDIUM tests/test_api_send.sql — single-element batch (cardinality = 1) is not tested

Tests cover 0-element (3d), 3-element (3, 3c), and 1000-element (3f, 3g) arrays. Cardinality-1 is a CTE boundary condition (single nextval call, single-row INSERT, array_agg with one element).
Fix: Add pgque.send_batch('test_send', 'batch.single', ARRAY['{"n":1}'::jsonb]) and assert cardinality(v_ids) = 1 with payload verification.

MEDIUM tests/test_api_send.sql:119-129 — Test 3e checks that send_batch delegates correctly but does not guard insert_event_bulk's own internals

pg_get_functiondef is called only for send_batch. A future regression where insert_event_bulk itself uses a loop would pass Test 3e.
Fix: Extend Test 3e: call pg_get_functiondef('pgque.insert_event_bulk(text, text, text[])'::regprocedure) and assert it contains neither FOREACH nor pgque.insert_event(. Also add a positive assertion: assert v_def ~* 'insert_event_bulk' in the send_batch check to guard against wholesale rewrites.

---

NON-BLOCKING (3)

MEDIUM docs/reference.md:21 — the §4.1 link target will show contradictory code until SPECx.md is updated (follows from BLOCKING #2 above)

Suggestion: Consider updating the link to point to the §4.8 API-summary table or adding an explicit anchor for insert_event_bulk once SPECx.md is updated.

LOW docs/reference.md:324-336 — insert_event_bulk is named as a primitive in the intro paragraph but absent from the "PgQ primitives (advanced)" section

reference.md line 21 names it; the primitives table lists only insert_event (3-arg and 7-arg).
Suggestion: Add an entry for pgque.insert_event_bulk(queue text, type text, payloads text[]) → bigint[] noting it is the internal set-based batch primitive callable by pgque_admin.

MEDIUM sql/pgque-api/send.sql:129 — ev_txid column omission from the INSERT relies silently on the column DEFAULT

ev_txid xid8 NOT NULL DEFAULT pg_current_xact_id() fires correctly, and this matches insert_event_raw. But it is undocumented and will confuse future readers auditing the column list.
Suggestion: Add a brief inline comment: -- ev_txid omitted: DEFAULT pg_current_xact_id() fires automatically.

---

POTENTIAL ISSUES (5)

MEDIUM sql/pgque-api/send.sql:104-109 — NULL elements inside a non-null text[] array are silently inserted as NULL ev_data (confidence: 7/10)

ARRAY['a', NULL, 'b']::text[] would produce a row with ev_data IS NULL. No guard exists in insert_event_bulk. Whether this is intentional is unclear.
Suggestion: Document the behavior or add an element-level NULL guard with a meaningful error.

MEDIUM sql/pgque-api/send.sql:94 — ::regclass cast on queue_event_seq is inconsistent with insert_event_raw and fails earlier on a missing/renamed sequence (confidence: 7/10)

insert_event_raw passes q.queue_event_seq as text directly to nextval(). insert_event_bulk casts it to ::regclass at SELECT time, raising an error before reaching the insert if the sequence is absent.
Suggestion: Either match insert_event_raw's pattern (pass as text, let nextval coerce) or add a comment explaining why early validation is preferable here.

MEDIUM tests/test_api_send.sql:119 — Test 3e's pg_get_functiondef approach is fragile to alternative loop spellings (confidence: 7/10)

\mforeach\M catches the PL/pgSQL keyword correctly (case-insensitive, word-bounded). A FOR … IN … LOOP construct iterating an array would not be caught.
Suggestion: Add a positive ~* 'insert_event_bulk' assertion to make the structural check self-documenting.

LOW sql/pgque-api/send.sql:116-121 — nextval($1) ordering inside the MATERIALIZED CTE is implementation-defined (confidence: 6/10)

ORDER BY ord during CTE materialization drives sequential nextval calls in practice, and the 1000-element tests confirm this. However, PostgreSQL makes no formal guarantee that side-effects inside a CTE body execute in ORDER BY sequence. If ev_id monotonicity matching input position is a strict requirement, document the assumption.

LOW sql/pgque-api/send.sql:131 — COALESCE(array_agg(...), '{}'::bigint[]) is structurally dead code (confidence: 5/10)

The join on ev_id between numbered and ins cannot be empty unless the INSERT fails entirely (which aborts the statement). Since send_batch guards cardinality > 0, numbered is never empty.
Suggestion: Remove the COALESCE or add a comment explaining why it is kept.

---

Summary

Area	Blocking	Non-blocking	Potential	Filtered
CI	✅	—	—	—
Security	0	0	0	4
Bugs	0	0	2	2
Tests	4	0	2	1
Guidelines	2	1	1	3
Docs	2	2	0	1
Total	8	3	5	11

---

REV-assisted review (AI analysis by postgres-ai/rev)

[NikolayS]

Testing evidence for PR #159

Head: 9fe47f29c61ad133bfb9c8f12f9945558edb6929

CI:

• GitHub Actions: green (test PG14/15/16/17/18, verify, client-smoke, claude-review)
• Merge state: clean

Local validation:

• bash build/transform.sh ✅
• Fresh PostgreSQL 18 install from sql/pgque.sql ✅
• Fresh PostgreSQL 18 + tests/test_api_send.sql ✅
• Fresh PostgreSQL 18 + full tests/run_all.sql ✅ (=== ALL TESTS PASSED ===)
• git diff --check ✅

Review:

• REV-style review completed after rebase/docs updates: 0 blocking issues ✅
• Follow-ups addressed:
  • replaced exception when undefined_function then null with explicit to_regprocedure(...) guard
  • documented internal insert_event_bulk grant/superuser behavior
  • documented that send_batch result positions match input positions; numeric ids are not a monotonicity contract

Verdict: ready for maintainer approval/merge. Not merging without explicit approval.

[NikolayS]

Final testing evidence for PR #159

Head: 97b01df1e2a59cfdf9c32a86efe050c8753d26c5

CI:

• GitHub Actions green: test PG14/15/16/17/18, verify, client-smoke, claude-review ✅
• Merge state: clean ✅

Local validation:

• bash build/transform.sh ✅
• Fresh PostgreSQL 18 install from sql/pgque.sql ✅
• Fresh PostgreSQL 18 + tests/test_api_send.sql ✅
• Fresh PostgreSQL 18 + full tests/run_all.sql ✅ (=== ALL TESTS PASSED ===)
• git diff --check ✅

Review:

• REV-style review: 0 blocking issues ✅
• Follow-ups addressed:
  • replaced exception-based grant cleanup with explicit guards
  • documented internal insert_event_bulk grant/superuser behavior
  • documented that send_batch result positions match input positions, not numeric monotonicity
  • preserved SPECx history; PR fix: make send_batch set-based #159 behavior is now documented as dated UPDATE 2026-05-02 notes instead of rewriting old changelog/spec text

Verdict: ready for maintainer approval/merge. Not merging without explicit approval.