fix: experimental SQL bugs + CI coverage#288
Draft
NikolayS wants to merge 3 commits into
Draft
Conversation
pgque.throughput() aligned buckets with an integer division by 60, so any bucket size under 60 seconds raised division-by-zero, and 60-120 s sizes that are not minute multiples (e.g. 90 seconds) silently collapsed to minute boundaries. Compute buckets in epoch seconds instead, so any positive interval works, and reject non-positive bucket sizes with a clear error. Tests added first (red), then the fix (green). https://claude.ai/code/session_01KAaEGkQZmey1D1xCsVGmqv
pgque.create_queue(text, jsonb) wrote 'max_retries' straight into pgque.queue, bypassing the >= 0 validation in pgque.set_queue_config(). A negative value made every nack dead-letter on first attempt. Route all options through set_queue_config(), which already accepts max_retries. Test added first (red), then the fix (green). https://claude.ai/code/session_01KAaEGkQZmey1D1xCsVGmqv
sql/experimental/*.sql and tests/run_experimental.sql were not exercised by any workflow, which is how the throughput and create_queue bugs shipped. Install the experimental files into a separate database and run the suite across the full PG matrix. https://claude.ai/code/session_01KAaEGkQZmey1D1xCsVGmqv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bugs
A1 (medium) —
pgque.throughput()division by zero for sub-minute buckets.sql/experimental/observability.sqlaligned buckets with% (extract(epoch from i_bucket_size)::int / 60). Integer division makes the divisor 0 for any bucket size under 60 seconds, soselect * from pgque.throughput('q', '1 hour', '30 seconds')aborted withdivision by zerowhenever ticks existed in the window. Sizes between 60 s and 120 s that are not minute multiples (e.g.'90 seconds'→% 1) silently collapsed to minute boundaries.A2 (low) —
pgque.create_queue(text, jsonb)accepted negativemax_retries.sql/experimental/config_api.sqlspecial-cased'max_retries'with a directupdate pgque.queue, bypassing the>= 0validation inpgque.set_queue_config(). With'{"max_retries": -1}', thecoalesce(ev_retry,0) >= v_max_retriescheck inpgque.nack()is always true, so every event dead-letters on first nack. History check: the special case landed in the same commit (8a54a7e) that introduced the validatingset_queue_config()override, which already acceptsmax_retries— the special case was redundant, never a workaround.D2 (low) — experimental SQL had zero CI coverage.
tests/run_experimental.sqlwas referenced by no workflow, which is exactly how A1/A2 shipped.Fixes
to_timestamp(floor(extract(epoch from tick_time) / extract(epoch from i_bucket_size)) * ...)— so any positive interval works; rejecti_bucket_size <= interval '0'(or null) with a clear exception.pgque.set_queue_config(), dropping the special case, so negative values are rejected with the canonical error (max_retries must be >= 0).Run experimental testsstep in thetestjob of.github/workflows/ci.yml(full PG 14–18 matrix): installssql/pgque.sqlplus the threesql/experimental/*.sqlfiles into a separatepgque_experimentaldatabase (so the later idempotency/uninstall steps keep testing the default install) and runstests/run_experimental.sql.Red/green TDD: tests added first and confirmed failing, then the fixes. New tests:
tests/test_observability.sql: throughput with'30 seconds'bucket (was: division by zero), exact 90-second bucketing with synthetic ticks (2 buckets, 100/150 events), non-positive bucket size raises.tests/test_experimental_config_api.sql:create_queue('q', '{"max_retries": -1}')raises the canonical error and the queue is not created.Verification
Fresh scratch DB, PostgreSQL 16:
Result (tail):
Pre-fix run of the same suite failed with
ERROR: division by zero(test 9) andERROR: create_queue should reject negative max_retries(config test), confirming red before green.Main suite unaffected — fresh DB:
Result:
=== ALL TESTS PASSED ===(exit 0).sql/experimental/*.sqlis not embedded in the generatedsql/pgque.sql(verified: nothroughput/jsonbcreate_queuein it), so no rebuild viabuild/transform.shwas needed.Addresses findings A1, A2, D2 of #283
https://claude.ai/code/session_01KAaEGkQZmey1D1xCsVGmqv
Generated by Claude Code