Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Prepare Postgres.jl for the 1.0 release - #5

Merged
quinnj merged 23 commits into
mainfrom
release-1.0-polish
Aug 7, 2026
Merged

Prepare Postgres.jl for the 1.0 release#5
quinnj merged 23 commits into
mainfrom
release-1.0-polish

Conversation

@quinnj

@quinnjquinnj commented Aug 6, 2026

Copy link
Copy Markdown
Member

This closes the correctness, protocol, security, compatibility, and packaging blockers found during the 1.0 review.

What changed

  • Makes connection-form extended queries atomic through transaction-mode PgBouncer. Independent prepared-statement handles now have clear cache ownership and cleanup.
  • Fixes transaction state, savepoint, commit-tag, cursor, and pooled-connection correctness failures.
  • Adds verified server TLS, CA-directory, client-certificate mTLS, and TLS cancellation coverage. Authentication and protocol parsing now fail closed on malformed input.
  • Keeps server parameters current, forces UTF8 startup, rejects unsafe ignored DSN requirements, redacts connection passwords, and clears bound parameter strings.
  • Fixes date, interval, range, array, composite, enum, numeric, byte, and PostgreSQL "char" decoding and round trips across PostgreSQL 14 through 18.
  • Adds an exact dependency-floor CI job, a PostgreSQL 14-18 CI matrix, a canonical MIT LICENSE, runnable first-install examples, and an explicit 1.0 support policy.
  • Removes the misleading trim-compilation gate. JuliaC --trim is explicitly outside the 1.0 support boundary.
  • Adds the development-assistance disclosure requested by current General registry guidance.

Exact-head validation

Candidate: 70486441be2be067c3001f68eab2efcbabc6b276

  • Julia 1.12 and PostgreSQL 16: 1,398 / 1,398 tests passed, including Aqua, verify-full TLS, CA-directory TLS, required client-certificate mTLS, cancellation, and protocol regressions.
  • Julia 1.10 with every declared dependency floor: 1,398 / 1,398 tests passed.
  • Transaction-mode PgBouncer competing-client reproduction: 200 interleaved queries, 0 failures.
  • PostgreSQL 14, 15, 17, and 18 compatibility suites passed locally; exact-head CI repeats that matrix.
  • Documentation build passed doctests, cross-references, and document checks.
  • A clean environment with only Pkg.add("Postgres") passed the README quick start.

Post-1.0 support expansion is tracked in #6.

Co-authored by Claude Code
Co-authored by Codex

quinnjand others added 5 commits August 5, 2026 23:38
Protocol/correctness fixes:
- copy_from no longer hangs forever when the COPY statement errors before
CopyInResponse (e.g. missing table): ReadyForQuery now terminates the
wait loop and the server error is surfaced with the connection usable
- copy_from/copy_to now reject non-COPY and wrong-direction statements
cleanly (CopyFail abort) instead of deadlocking or silently succeeding
- COPY statements via DBInterface.execute or Postgres.cursor now abort the
server-side copy (CopyFail + fresh Sync, since the pre-copy Sync is
ignored in copy-in mode) and throw a clear error pointing at
copy_from/copy_to, keeping the connection usable
- IPv6 host literals are bracketed when forming the transport address
([::1]:5432); unix socket paths get a clear unsupported error
- GC.@preserve added around unsafe_string(pointer(buf)) parsing loops
(error/notice/notification/parameter-status/describe/command-tag)
- password material (cleartext and md5 hash) is redacted from debug logs
- PostgresInterfaceError now subtypes Exception
- parse_numeric throws a clear error for NaN/Infinity numeric values
- cursor(conn, sql) rolls back its own transaction if cursor setup fails
API surface polish:
- ConnectionParams.debug/reconnect are honored by connect (kwargs still
override); sslservername plumbed through ConnectionParams, keyword DSNs,
URIs, and all ConnectionPool constructors; style kwarg available on the
DSN/params connect and pool paths
- removed the accepted-but-ignored `binary` kwarg from execute
- public declarations for the supported API surface (Julia 1.11+)
- docstrings for the public API (connection, pool, transactions, COPY,
LISTEN/NOTIFY, type registry, statement cache, cursor, errors, types)
Org-transfer/metadata cleanup:
- deploydocs points at JuliaDatabases/Postgres.jl
- LICENSE names Postgres.jl (was template text naming Example.jl)
- README: CI/docs/codecov badges, raw"..." on $1 examples so they are
copy-pasteable, style-based query logging (set_query_logger! was removed
pre-1.0 but docs still showed it), sslservername documented
- docs: same fixes, plus reference blocks for the newly documented types
- Parsers compat tightened to "2" (0.3/1 predate APIs in use); unused
Logging dependency dropped; dead code removed (ERROR_CODE,
DATETIME_OPTIONS, parse_to_julia_array, commented-out escape/lastrowid)
Tests: COPY misuse/error-path coverage, ConnectionParams debug/reconnect,
sslservername parsing, IPv6/unix-socket address formation, numeric special
values, updated export-surface test for public names.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- add the GC.@preserve missed in update_server_parameters! (same
unsafe_string(pointer) pattern fixed elsewhere in this PR)
- consistent exception taxonomy: COPY misuse (non-COPY, wrong-direction,
via execute/cursor) now throws PostgresInterfaceError everywhere instead
of Error in some paths and PostgresInterfaceError in others; the Error
docstring now documents the residual protocol-level client uses
- copy-out error precedence: a genuine mid-stream server error (e.g.
COPY (SELECT 1/0) TO STDOUT) is surfaced instead of being masked by the
misuse error in the execute and cursor paths
- copy_in's post-data drain aborts a second CopyInResponse (multi-statement
query strings) with CopyFail instead of deadlocking
- README: drop inaccurate "pure-Julia" (TLS uses OpenSSL_jll via Reseau)
- tests for all of the above
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A failing data source in copy_from now aborts the copy with CopyFail and
drains to ReadyForQuery, so the connection stays usable and the source's
error propagates. A failing dest IO (or socket failure) mid copy-out
closes the socket instead of leaving a desynced connection that still
looks valid to pool_isvalid — matching the mid-stream bail behavior of
the execute and cursor paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- redact SASL/SCRAM messages from the debug log: the earlier redaction
covered only cleartext and md5, leaving the client proof (and thus an
offline brute-force oracle for the password) in the log for SCRAM-SHA-256,
the default auth method on modern PostgreSQL
- bound all message-field parsing by the buffer actually received. Reseau's
read returns a short buffer at EOF without throwing, so loops bounded on
the server-declared length, and the unbounded unsafe_string(pointer(buf))
NUL scans, could read past the allocation and surface heap bytes as
column names, command tags, or error text. Adds cstring_at and uses it in
the error/notice/notification/command-tag parsers; describeprepared and
the DataRow value loop now bounds-check every offset before reading.
- send CancelRequest over TLS when the connection being cancelled uses TLS
(the cancel key is a credential valid for that backend's lifetime), and
refuse to send it in the clear under sslmode=require/verify-full
- bound the numeric exponent so bogus server text can't drive an enormous
BigInt scaling
- reject embedded NULs in escape_identifier/escape_literal, and document
that escape_literal assumes standard_conforming_strings=on
- document that sslservername is also the name verified against the
certificate under verify-full (not merely an SNI override), that require
encrypts without authenticating the server, and that query_logger receives
bound parameter values
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- cancel_query! no longer fails silently. cancel_request's refusal to send
the key in cleartext now throws instead of returning false into a
discarded return value, and cancel_query! throws when the request could
not be delivered at all — previously a refused or failed cancel left the
caller believing a runaway query had been cancelled.
- the cancel connection requires TLS when the connection being cancelled
actually negotiated TLS, not merely when sslmode said so: under the
default "prefer" the main session can be on TLS while the cancel
connection silently fell back to cleartext.
- bound the server-declared message length in readheader (and in the
pre-TLS, pre-auth SSLRequest error path, which bypasses it) to
PostgreSQL's own 1 GiB protocol maximum. Reseau's read allocates the
requested size up front, so a 5-byte header claiming ~2 GB committed that
much memory before a single body byte arrived — reachable by an on-path
attacker before authentication.
- parse_numeric reports an out-of-range exponent consistently instead of
letting an oversized one surface as OverflowError
- document that debug logging redacts authentication messages but not bind
parameter values
Adds coverage that cancellation works over TLS against the SSL fixture
server, and that the cleartext refusal throws.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Two bugs in the previous commit, plus adjacent hardening:
- the prefer->require TLS upgrade for the cancel connection was inside
`if trylock(conn.lock)`, so it was skipped in cancel_query!'s primary use
case: another task holds the lock running the very query being cancelled.
A default-sslmode connection that had negotiated TLS would still let the
cancel key fall back to cleartext — exactly the downgrade the upgrade was
added to prevent. The socket check needs no lock (host/pid/skey are read
unlocked too), so it now happens before the trylock.
- the new message-length check threw API.Error, which describeprepared
treats as "the stream is clean, at ReadyForQuery" — so a bogus length
left a desynchronized socket open and reusable by the pool. It now closes
the socket before throwing, like the other protocol-corruption paths.
- DataRow's column count is signed on the wire: a negative passed the
upper-bound check and produced an unfilled row (UndefRefError downstream)
instead of a clean protocol error. Also bounds against typeIds, not just
names.
- the remaining ParameterStatus field loop is bounded by the buffer length
rather than the server-declared length, matching the others.
- the three COPY mid-stream catches surface an already-received server
error instead of the raw IO error, matching the execute path.
Adds DataRow malformed-message unit tests, and makes the TLS cancel test
use the default sslmode so it covers the upgrade path with the lock held.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/API.jl Outdated
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/execute.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/connection_string.jl
Comment threadsrc/Postgres.jl Outdated
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/array_parsing.jl
- waitfor's message loop had no fallback branch, so any message type it
wasn't waiting for had its header read and its body left on the wire —
the body was then read as the next message header. Reproducible against a
real server: LISTEN on a connection, have another connection NOTIFY, then
run a query on the listener; the NotificationResponse arrives interleaved
with the query's messages and destroys the connection. Now discards the
body like every other read loop. Covered by a regression test (verified by
mutation: removing the fix fails that test).
- wait_for_notification's read deadline now covers only the first byte of a
message. Previously it covered the body too, so a message straddling the
100ms poll boundary left the stream parked mid-message, and the swallowed
DeadlineExceededError meant the loop resumed reading body bytes as a
header. It also no longer runs _clear_read_deadline! on a socket that the
message-length check just closed, which replaced the protocol diagnostic
with a Reseau-internal NetClosingError.
- DataRow's column count must equal the described column count, not merely
not exceed it: a short count left the caller's row partly unfilled.
- extract the cancel TLS-upgrade decision into cancel_sslmode and test it
directly — the end-to-end test could not pin it (mutation-verified: the
SSL fixture always offers TLS, so removing the upgrade changed nothing).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/API.jl
Comment threadsrc/Postgres.jl
Comment threadsrc/api/types.jl Outdated
Comment threadLICENSE.md Outdated
- align_session_formats! treated an unreported IntervalStyle as already
correct while treating an unreported DateStyle as needing a fix. A pooler
that doesn't forward IntervalStyle (pgbouncer before 1.21, or without it in
track_extra_parameters) against a server set to sql_standard therefore left
every interval decoding to zero, silently — in exactly the deployment the
ParameterStatus approach was chosen to support. Both now correct an absent
value, and the recorded server parameters are updated to match.
- the pool reset only saw transactions opened through start_transaction, so a
raw DBInterface.execute(conn, "BEGIN") sailed through: the next borrower
inherited the open transaction and its commit committed the previous
borrower's abandoned writes. The connection now also tracks the server's own
ReadyForQuery transaction status, which sees transactions however they were
opened, and the pool consults both.
- empty sslmode is no longer treated as unset. libpq rejects it, and the
previous commit's "empty means unset" reasoning is wrong here specifically:
an unexpanded ${PGSSLMODE} intended as verify-full would have silently
become the unauthenticated default. It fails loudly again.
- a failing rollback in the transaction wrappers no longer replaces the error
that caused it, which was still losing the SQLSTATE when the body failed
because the session died.
- @transaction binds its connection expression once: `@transaction
acquire(pool) ...` previously acquired a different connection for the
BEGIN, the COMMIT and the ROLLBACK, and leaked pool permits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make register_enum! honor or restrict julia_type

The public signature accepts any julia_type::Type, and its docstring says enum values are returned as that type. Only Symbol gets a parser. Every other requested type is registered with no parser and the wire value remains a String.

Live exact-head repro:

@enum CodexMood sad ok happy
Postgres.register_enum!(conn, "codex_mood"; schema=temp_schema, julia_type=CodexMood)
row =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT 'ok'::pg_temp.codex_mood AS mood")))

row.mood is "ok"::String, and the reported schema widens to Union{CodexMood,String}. Please either convert to the requested type, add a documented parser argument, or restrict julia_type to the identity String and supported Symbol cases. This contract should be settled before the 1.0 API freezes.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Prevent callback queries from interleaving the active protocol stream

Notice and notification callbacks run synchronously before the active query reaches ReadyForQuery. The connection lock does not protect this boundary because it is reentrant for the current task. A documented custom callback can therefore query the same connection and interleave two protocol exchanges.

I reproduced this on exact head with a style whose first notice_callback runs DBInterface.execute(style.conn, "SELECT 99 AS nested"), then executed DROP TABLE IF EXISTS to produce a notice. The callback fired, the nested query consumed messages belonging to the outer query and failed with unexpected message type 'Z' ... protocol state is corrupted, the outer query failed with NetClosingError, and the connection was closed.

Please defer user callbacks until the current exchange is back at a safe message boundary, or reject same-connection reentry before any bytes are written while preserving the outer stream. Apply the rule to notice and notification callbacks in execute, cursor, COPY, setup waits, and notification waiting. Add a callback that attempts a same-connection query and prove it cannot corrupt or hang the active connection. Document whether callback reentry is supported.

The guard must cover all user code invoked during result consumption, not only style hooks. A registered type-parser closure or custom StructUtils lift can also capture the connection and reenter while Exec is still draining rows.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define the array-type support boundary before 1.0

The manual says that arrays map to Julia arrays, but the default registry covers only selected array OIDs. Exact-head live results for common built-in types are raw String values:

ARRAY[1::oid,2::oid] -> "{1,2}" :: String
ARRAY['x'::name,'y'::name] -> "{x,y}" :: String
ARRAY[5::bit(3),2::bit(3)] -> "{101,010}" :: String
ARRAY[5::bit(3)::varbit] -> "{101}" :: String
ARRAY[int4range(1,3)] -> "{\"[1,3)\"}" :: String

Internal "char"[] has the same gap. Please register the intended built-in array OIDs, including range-array OIDs, or narrow the public docs to an exact supported list and explain how users register the rest. Add live schema/value tests for every array mapping that the 1.0 docs promise.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Accept ordinary Julia matrices as PostgreSQL array parameters

Parameter encoding only dispatches on AbstractVector. A normal Julia Matrix therefore falls through to string(x) instead of PostgreSQL array syntax.

Exact-head live repro:

matrix =reshape(Int32[1, 2, 3, 4], 2, 2)
DBInterface.execute(conn, raw"SELECT $1::int4[]", (matrix,))

The driver sends Int32[1 3; 2 4], and PostgreSQL rejects it with SQLSTATE 22P02 because the value does not start with {. The equivalent nested vector [[1,2],[3,4]] succeeds, so this is a dispatch gap rather than a server limitation.

The manual states that arrays map to Julia arrays. Please encode rectangular AbstractArray inputs with their dimensions preserved, or document the accepted parameter representation precisely. Add a live round trip that checks array_ndims, array_dims, and values for a Matrix.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the public PostgresRange value bindable, or define it as read-only

The driver returns standard range columns as its public PostgresRange{T} type, but the generic parameter encoder sends the default Julia struct display.

Exact-head live round trip:

r =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT int4range(1, 3, '[)') AS r"))).r
DBInterface.execute(conn, raw"SELECT $1::int4range", (r,))

The bound text is Postgres.API.PostgresRange{Int32}(1, 3, true, false, false). PostgreSQL rejects it with SQLSTATE 22P02 as a malformed range literal.

Please serialize PostgresRange in PostgreSQL range syntax, including quoted and escaped bounds, empty ranges, and unbounded endpoints. If returned mapping types are intentionally read-only, state that parameter boundary in the 1.0 manual instead. A live read-then-bind round trip should cover a numeric range and a quoted text/custom range.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not turn a logger failure into an apparent query failure after commit

The success-side query_logger call is inside the same try as query execution. If the logger throws, the catch treats that callback error as a query failure, calls the logger again with success=false, and rethrows. The database operation has already completed.

I reproduced this on exact head with a custom style whose logger throws only for a successful INSERT. DBInterface.execute threw ErrorException("logger failed after success"), but a subsequent query found the inserted row. The recorded calls for the same SQL were success=true and then success=false.

This can make application retry logic duplicate a committed write. It affects direct and prepared execute plus both COPY wrappers. Please isolate logger failures from database outcome reporting. Either contain and report callback errors separately, or define an explicit callback-failure policy that cannot label a completed query as failed. Add a write regression that throws in the success logger and proves the caller cannot confuse the callback failure with a server/transaction failure.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not let a cursor commit a transaction started with raw SQL

eab05bd now tracks the server ReadyForQuery transaction status for pool cleanup, but the public in_transaction and cursor ownership logic still use only the helper-managed flag.

Live exact-head sequence:

  1. Execute raw BEGIN and insert a row.
  2. Postgres.in_transaction(conn) reports false, while the new server-status field is true.
  3. An observer connection cannot see the row.
  4. Open Postgres.cursor(conn, ...; fetchsize=1), consume it, and close it.
  5. The cursor sends another BEGIN, receives there is already a transaction in progress, marks the transaction as cursor-owned, and sends COMMIT on close.
  6. The observer now sees the row, although the caller never committed its raw transaction.

Postgres.commit(conn) also rejects a raw transaction as no transaction in progress, despite the in_transaction docstring promising the actual connection state.

Please make ownership distinct from server transaction state. in_transaction must reflect ReadyForQuery, and a cursor must never claim or commit a transaction that was already active. Add an observer-connection regression for raw BEGIN plus cursor close, and cover raw BEGIN with commit, rollback, and nested helper behavior.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P1] Do not allow a pre-authentication peer to request a 1 GiB allocation

The new MAX_MESSAGE_LEN check uses PostgreSQL's 1 GiB valid-message ceiling as the safety ceiling. That does not protect this client from memory exhaustion. In the pre-TLS SSLRequest ErrorResponse path, an unauthenticated peer can send only the type byte and a length of 1 GiB. The code accepts it and immediately calls read(socket, len), which allocates the declared body before the peer sends it or proves its identity.

This is reachable even when the caller requested verify-full, because the peer controls the plaintext SSL negotiation response before certificate verification. A 1 GiB cap still permits a one-connection process-killing allocation.

Please use a small, message-specific cap for pre-authentication and diagnostic/control messages. For potentially large authenticated result messages, use a deliberate configurable limit or bounded/streamed handling. Add a fake-server test that declares a large SSLRequest error body but sends no body, and prove the client rejects the header without allocating in proportion to it. I did not execute the full-size allocation because that would risk the review host.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Define parameter encoding for the JSON value type that queries return

json and jsonb results are returned as JSON.LazyValue, but the generic parameter encoder applies string(x). That string is a human display, not JSON.

Exact-head live read-then-bind:

j =only(Tables.rowtable(DBInterface.execute(conn,
"SELECT '{\"a\":1}'::jsonb AS j"))).j
DBInterface.execute(conn, raw"SELECT $1::jsonb", (j,))

The driver sends text beginning LazyObject{String} with 1 entry:. PostgreSQL rejects it with SQLSTATE 22P02 and Token "LazyObject" is invalid.

Please encode the public JSON result type as JSON, or document it as read-only and provide the exact supported JSON parameter forms. This and PostgresRange show that the catch-all string(x) fallback does not provide a safe general parameter contract. Add live JSON/JSONB read-then-bind regressions before the 1.0 API is fixed.

@quinnj

quinnj commented Aug 6, 2026

Copy link
Copy Markdown
MemberAuthor

[P1] Do not send statement_timeout through startup options

The format-alignment change correctly avoids startup options because transaction poolers commonly reject it. The documented statement_timeout connection option still uses options=-c statement_timeout=... in writestartupmessage, so the same compatibility failure remains.

Exact-head live test through PgBouncer 1.25.2 in transaction mode:

DBInterface.connect(Postgres.Connection,
"127.0.0.1", "postgres", "postgres";
port=56432, dbname="postgres", sslmode="disable",
statement_timeout=1234)

Connection setup fails with FATAL SQLSTATE 08P01: unsupported startup parameter in options: statement_timeout.

A post-authentication session SET alone is not a safe fix for transaction pooling. I connected two clients without the startup option. Client A set 1111ms; client B immediately observed 1111ms. Client B then set 2222ms; client A immediately observed 2222ms. Their local getters still reported A=1111 and B=2222. PgBouncer did not track this setting, so it leaked across logical clients and the API state became false.

Please define a pooler-safe contract. If connection-level enforcement is promised through transaction pooling, apply it at a boundary that follows each backend assignment, such as a transaction-local/query boundary, and verify it cannot leak. Otherwise detect and document the unsupported combination instead of reporting a value the server is not enforcing. Preserve the configured behavior across reconnects. Test both startup and set_statement_timeout! with two competing clients through a stock transaction-mode PgBouncer that does not allowlist options.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Temporal follow-up: handle PostgreSQL's valid 24:00:00 time value

The earlier precision finding is not the only valid time value outside the current Julia mapping. PostgreSQL accepts and emits TIME '24:00:00'. Exact head throws ArgumentError("Hour: 24 out of range (0:23)") for both the scalar and time[] forms because Dates.Time cannot represent hour 24.

Please include this in the explicit temporal support boundary. Use a lossless representation if time is promised across PostgreSQL's full domain, or throw a clear driver error that names the unsupported value and type. Do not expose an incidental Julia constructor error. Add scalar and array live tests. timetz is also currently returned as raw String, so the 1.0 type table should state that boundary explicitly.

@quinnj

Copy link
Copy Markdown
MemberAuthor

[P2] Make the documented exception taxonomy true for built-in conversions

The PR description and Postgres.Error docstring say server failures are Postgres.Error and other client-side failures use PostgresInterfaceError. Built-in result conversion currently leaks implementation exceptions.

Exact-head examples include:

  • valid PostgreSQL TIME '24:00:00' and time[] values throw ArgumentError from Dates.Time;
  • a non-ISO date reaches ArgumentError from Dates.Date;
  • stale or mismatched integer metadata reaches Parsers.Error;
  • malformed built-in array/bytea tokens can expose ArgumentError.

Please define the 1.0 exception boundary precisely. Wrap driver-owned decoding and protocol validation failures in PostgresInterfaceError with the PostgreSQL type/OID and value context, while preserving Postgres.Error and deliberately allowing user parser/style exceptions to remain identifiable. Then test the public exception types. Otherwise narrow the taxonomy claim and docstrings before release.

@quinnj

Copy link
Copy Markdown
MemberAuthor

Review checkpoint — eab05bd is NOT READY for 1.0

The hosted matrix is green, but exact-head adversarial tests still reproduce release blockers. Fix order:

  1. P0 data isolation: transaction-mode PgBouncer can return another logical client's prepared query result. The exact-head competing-client test is wrong in 100/200 iterations.
  2. Transaction correctness: a failed transaction can report a successful commit; raw BEGIN is absent from the public state and a cursor can commit it; transaction/cursor scopes admit unrelated tasks; @transaction leaks on nonlocal control flow; nested savepoints are not released.
  3. Protocol and security: asynchronous messages are discarded; notification timeouts can hang or break TLS records; connection timeout does not cover setup/authentication; short discards and truncated descriptions can pass; NUL values can inject C-string fields; an unauthenticated peer can request a 1 GiB allocation; requirement-bearing DSN options are ignored; UTF-8 is not enforced.
  4. Pool and TLS behavior: pool close is not terminal, dead peers are reused, statement_timeout is rejected by stock PgBouncer and leaks between logical clients, and the live mTLS / TLS 1.2 IP verification cases remain blocked in the resolved Reseau path.
  5. Data/API contracts: multi-bit values lose data; valid temporal values lose precision or change meaning; OID-based 3D arrays fail; common array mappings are raw strings; composite quotes split fields; enum julia_type is not honored; returned JSON/range values and Julia matrices do not bind; callback reentry corrupts the stream; logger failure can make a committed write appear failed.
  6. 1.0 validation: the trim job passes a failed compile and runs no binary; declared dependency floors are too low; Aqua has Postgres/StructUtils ambiguities and missing compat entries; the quick start is not runnable after its stated install; supported PostgreSQL/type boundaries and the license artifact are not release-ready.

Each item has an exact reproduction and requested regression in its issue or inline thread. I will retest claimed fixes against those original cases. I will mark the review READY/CLEAN only after the exact head clears the P0/P1 set, the P2 API boundaries are implemented or stated precisely, the direct trim claim is truthful, full local/live validation passes, and exact-head CI is green.

Round-12 review findings, all verified against a live server:
- register_range! was advertised-but-broken for element types outside the
six builtins: registration succeeded, then every value threw. The parser
now binds the element type discovered at registration time.
- The DateStyle correction set 'ISO, MDY' unconditionally, silently
reinterpreting ambiguous input literals like '01/02/2020' for a
DMY-configured database (and hard-erroring on '13/02/2020'). Only the
output-format half is corrected now; the configured field order is kept.
- statement_timeout moved from the startup `options` parameter to a
post-connect SET: pgbouncer rejects unknown startup options outright, so
sending it there failed the whole connection.
- parse_interval silently returned a zero interval for text it didn't
understand (sql_standard, iso_8601, postgres_verbose renderings). Only a
genuine "00:00:00" decodes to zero; anything unrecognized now throws,
naming the IntervalStyle requirement.
- BC dates decoded silently as AD -- a different year numbering with no
year zero. They now throw, including the timestamptz rendering where
" BC" follows the zone offset, out of sight of the datetime parser.
- Years beyond 9999 misparsed into "Month: NNNN out of range" errors; the
year field is scanned rather than assumed 4 digits wide.
- "char" values for high-bit bytes arrive as backslash-octal escapes
("\377") and decoded to '\\'; they now decode to the byte value, on the
OID path, the typed-struct lift, and the array element path alike.
"char"[] (oid 1002) was unregistered entirely and returned raw literals.
- libpq keywords that are accepted-but-ignored now warn when set to a
value that requests a security or connection-selection behavior
(channel_binding=require, sslcrl, sslcrldir, requiressl,
target_session_attrs, options); silently dropping them left callers
believing a protection was in place.
- A raw-SQL transaction's tracked server status survived reconnect,
triggering a spurious ROLLBACK warning on the fresh session.
- Documented the session-format requirement (ISO / postgres), the values
with no Julia representation (infinity, BC, numeric NaN), and that a
'\0' read from a "char" column cannot be bound back as text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@quinnj

Copy link
Copy Markdown
MemberAuthor

Exact-head 826babf PgBouncer retest is still a P0 failure. Against PgBouncer 1.25.2 in transaction mode with one backend and max_prepared_statements=200, two concurrent Postgres.jl connections ran 100 simple queries each; 100/200 results used the other client row or metadata. Examples include client A receiving 222 for SELECT 111 AS from_a, and client B receiving the from_a column/result for SELECT 222 AS from_b.

The new post-connect statement_timeout SET also leaks across pooled clients. Connection A was created with statement_timeout=731; connection B was created without a timeout. Both then read server statement_timeout=731ms, while the local getters reported A=731, B=nothing. Moving the setting out of StartupMessage avoids PgBouncer startup rejection, but a session SET is not owned by a logical client in transaction pooling.

Please keep Parse/Bind/Execute ownership atomic across the pooler and either implement a safe session-setting strategy or reject transaction-pooling use clearly before any query. This head remains NOT READY.

quinnjand others added 2 commits August 6, 2026 09:23
Round-13 review findings, all verified against a live server:
- Driver transaction helpers consulted only the client-side flag, so over
a transaction opened with raw SQL (execute(conn, "BEGIN")) they issued a
BEGIN the server ignored and their commit committed the caller's
transaction -- the caller's own ROLLBACK then silently no-oped.
start_transaction now nests inside a raw transaction with a savepoint
(tracked by owns_base_transaction); the outermost driver commit releases
that savepoint and rollback rolls back to it, leaving the caller's
transaction open and under their control. cursor treats the
server-reported transaction as "already in one", as its docstring says.
- Range bounds ending in a multibyte character threw StringIndexError
(byte-index slicing): every textrange('α','ω')-style value was
undecodable after a successful registration.
- Timezone offsets carrying a seconds field ("+05:21:10", the rendering of
LMT-era timestamps in named zones) silently lost the seconds.
- The variable-width year scan accepted fewer than 4 year digits, so
"03-04-2020" -- Postgres-style output after a mid-session SET DateStyle
-- silently decoded as year 3 where the old fixed-width parser threw.
The year is now bounded to 4..9 digits and both separators are checked,
which also closes an Int-overflow and short-input BoundsErrors on
adversarial input.
- An unreported DateStyle (a pooler not forwarding ParameterStatus) was
corrected to 'ISO, MDY', force-flipping DMY sessions; setting just 'ISO'
preserves the server-side field order we can't see.
- server_in_transaction went stale on every failed statement: error paths
skipped the status copy, so a failed COMMIT left it true and drew a
spurious ROLLBACK ("no transaction in progress") on the next pool
release. The status is recorded in a finally on the prepared path and
published through a Ref on the simple-query path.
- postgres's valid time '24:00:00' threw a raw ArgumentError mid-decode;
it now reports PostgresInterfaceError like other unrepresentable values.
- register_enum!/register_composite!/register_range! register the type's
array OID alongside it, so mood[]/composite[]/range[] values decode
instead of returning the raw literal string. register_range! documents
that element types must be registered before ranges over them.
- The reconnect test now triggers checkconn directly: the previous version
passed even without the fix because the follow-up statement refreshed
the flag from its own ReadyForQuery.
Mutation-tested: the raw-transaction savepoint base, the cursor ownership
check, and the checkconn reset each fail their new tests when reverted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix transaction and statement ownership, make unnamed execution safe through transaction-mode PgBouncer, harden protocol and TLS handling, enforce the supported type and DSN contracts, and add the release validation matrix and support policy.
@quinnjquinnj mentioned this pull request Aug 6, 2026
10 tasks
@quinnjquinnj changed the title 1.0 release readiness: protocol fixes, org-transfer cleanup, API polishPrepare Postgres.jl for the 1.0 releaseAug 6, 2026
@quinnjquinnj closed this Aug 6, 2026
@quinnjquinnj reopened this Aug 6, 2026
Copy the generated pg_hba.conf into container-owned storage so Linux bind-mount permissions cannot block TLS startup.
Require Reseau 1.3.5 for the renewed precompile certificate and keep the exact lower-bound job aligned.
@quinnj

Copy link
Copy Markdown
MemberAuthor

READY/CLEAN at 49823c316d118d3f9eb6166cda1127b130d631c5.

  • Exact-head GitHub Actions: 15/15 jobs green
  • Julia 1.12 local validation: 1,398/1,398 checks passed
  • Exact Julia 1.10 dependency floors: 1,398/1,398 checks passed
  • Review threads: 40/40 resolved
  • GitHub state: MERGEABLE/CLEAN

The deferred post-1.0 hardening remains tracked in #6. This head is ready to squash-merge for v1.0.0.

@quinnj
quinnj merged commit 79b8209 into mainAug 7, 2026
17 checks passed
@quinnj

Copy link
Copy Markdown
MemberAuthor

@JuliaRegistrator register

@JuliaRegistrator

Copy link
Copy Markdown

Comments on pull requests will not trigger Registrator, as it is disabled. Please try commenting on a commit or issue.

Sign up for freeto 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.

2 participants

@quinnj@JuliaRegistrator