1.0 release readiness: protocol fixes, org-transfer cleanup, API polish

[quinnj]

In-depth 1.0 readiness pass: wire-protocol and type-handling bug fixes, post-org-transfer cleanup, API polish, and test coverage for all of it. Developed through repeated adversarial self-review — twelve independent review rounds against a live PostgreSQL 16, each fed the previous round's fixes. Several findings below were bugs introduced by an earlier round's fix and caught by the next.

Local: 1180/1180 tests (Docker integration, TLS-certificate fixture, --trim compile check at 74/92 verifier errors). Docs build clean. Full CI matrix green.

Silent data loss / corruption (all pre-existing on main)

• Array elements containing ] were truncated. PostgreSQL only quotes elements containing ", \, {, }, , or whitespace, so ] arrives unquoted — and was treated as a terminator. ["a]b", "x[1]", "plain"] round-tripped back as ["a"], silently dropping two elements. File paths, x[1]-style names and embedded JSON all hit this.
• Arrays with a lower bound ≠ 1 lost every element. PostgreSQL prefixes those with explicit dimensions ([0:2]={a,b,c}); the [ was taken for an array-open bracket, so array_fill(..., ARRAY[0]), arr[0:2] slices and array_prepend results returned the dimension string (["0:2"]) or threw.
• tsrange/tstzrange were 100% undecodable. Range bounds containing whitespace are quoted, and the quotes were passed straight to the element parser, so every timestamp range threw Month: 2530 out of range. The type registry advertised these types.
• Pooled connections leaked transactions. A with_connection block that threw mid-transaction returned the connection with that transaction open; the next borrower's BEGIN became a SAVEPOINT and their commit only decremented a counter, so their writes were silently discarded while their code reported success. Connections are now reset on release, using both the client flag and the server's own ReadyForQuery status (so a raw execute(conn, "BEGIN") is caught too).
• "char" zero values crashed catalog queries. SELECT attidentity FROM pg_attribute threw BoundsError.
• Non-ISO DateStyle broke all date parsing with an error pointing nowhere near the cause. The session now aligns DateStyle/IntervalStyle when the server reports something the text parsers can't read — via SET, not startup options, since poolers commonly reject the latter.

Protocol correctness

• copy_from hung forever when the COPY statement errored before CopyInResponse; COPY misuse via execute/cursor/wrong-direction now aborts cleanly instead of deadlocking (with a fresh Sync after CopyFail, since the pre-sent one is ignored in copy-in mode).
• A NOTIFY delivered while the same connection ran queries desynced the stream and destroyed the connection — waitfor had no fallback branch and left the message body on the wire.
• wait_for_notification never worked over TLS (the deadline arrives wrapped in a TLSError), and could leave an expired read deadline set, permanently poisoning the connection.
• Server-declared message lengths are bounded at PostgreSQL's own 1 GiB maximum — reachable pre-authentication, where a 5-byte header could commit ~2 GB.
• All unsafe_string(pointer(...)) parsing is GC-preserved and bounds-checked against the buffer actually received (read returns a short buffer at EOF).
• IPv6 hosts were rejected at connect time; failed connection attempts leaked a file descriptor each.
• Transaction state survived a failed COMMIT, permanently blocking reconnect and making the next cursor skip its BEGIN.

Security

• Password material is redacted from debug logs — including the SCRAM client proof, which is an offline brute-force oracle and is the default auth method on modern PostgreSQL.
• cancel_query! sends the cancel key over TLS when the connection uses TLS, and refuses to send it in the clear under require/verify-full (throwing rather than failing silently).
• An unrecognized connection parameter now errors: ssl_mode=verify-full (a typo) previously left sslmode unset and silently fell back to an unauthenticated connection. Real libpq keywords this driver doesn't implement are accepted and ignored, so provider-issued URIs keep working.
• Error.severity uses the non-localized field, so severity checks don't depend on server locale.

API surface

• Breaking (pre-1.0): the accepted-but-ignored binary kwarg is removed from DBInterface.execute.
• ConnectionParams.debug/.reconnect were silently ignored; sslservername and style are now available on every connect and pool path.
• PostgresInterfaceError <: Exception; consistent taxonomy (server errors are Postgres.Error with SQLSTATE, client-side failures are PostgresInterfaceError).
• public declarations for the supported surface on Julia 1.11+, and docstrings throughout.

Org transfer / metadata

deploydocs and badges point at JuliaDatabases; LICENSE named Example.jl; README/docs showed a removed set_query_logger! API and $1 examples that are a syntax error when copy-pasted. Dead code removed, Parsers compat tightened, unused Logging dependency dropped.

🤖 Generated with Claude Code

[quinnj]

[P1] Keep reported server parameters current after SET

The getter only reflects startup values because the execution paths discard ParameterStatus (S) messages. I reproduced this against PostgreSQL 16: get_server_parameter(conn, "application_name") returned ""; after DBInterface.execute(conn, "SET application_name = 'review_changed'"), SHOW application_name returned review_changed but this getter still returned "". PostgreSQL sends ParameterStatus for reported GUC changes during normal query execution. Please parse those messages into conn.server_parameters in both extended and simple-query paths, and add a regression test. A public 1.0 API must not return stale server state.

[quinnj]

c1ae7ba makes this stale cache observable immediately after connect. I created a role with DateStyle = SQL, DMY and IntervalStyle = iso_8601. align_session_formats! correctly changes the live session, and date/interval results parse. However, it runs SET through the simple-query reader, which discards the resulting ParameterStatus messages. The new connection therefore reports get_server_parameter(..., "DateStyle") == "SQL, DMY" and IntervalStyle == "iso_8601" while current_setting is ISO, MDY / postgres.

Please route all S messages through one connection-aware updater, including these post-auth alignment queries. The new regression should assert both current_setting and the public getters against the non-default database.

[quinnj]

[P1] Do not return an open transaction to the pool

pool_isvalid checks only the socket, so release puts a connection with an active or failed transaction back into circulation. I reproduced this with a limit-1 pool: borrower A began a transaction, inserted 42, and released; borrower B received the same connection, in_transaction was still true, and it could read A’s uncommitted row. It could also commit or roll back A’s work. On release, fully roll back a transaction before reuse, or close/drop the connection if cleanup fails. Do not rely only on the current manual flag: a borrower can also issue raw BEGIN through DBInterface.execute, while the driver currently discards the transaction-status byte in ReadyForQuery. Track that server status or reset the session defensively. Add regression tests for both helper-started and SQL-started transactions. This cross-borrower state leak blocks a 1.0 release.

[quinnj]

A second transaction-status failure causes a false successful commit. At exact head e8451c0, I ran Postgres.transaction, inserted one row, caught a SELECT 1/0 error inside the callback, and returned :reported_success. PostgreSQL marked the transaction failed, so its later COMMIT command completed as ROLLBACK. The helper returned :reported_success, Postgres.in_transaction(conn) became false, and the table contained zero rows.

This is silent data loss: the public transaction helper reports success although nothing committed. Please track the ReadyForQuery status byte and/or validate the transaction-control CommandComplete tag. commit must not report success when the server reports ROLLBACK. Add the caught-inner-error case for Postgres.transaction, DBInterface.transaction, @transaction, and manual start_transaction/commit.

[quinnj]

Retested pool release at exact head 007ceaa. The new cleanup fixes the helper-started case: a connection released after Postgres.start_transaction is rolled back, the next borrower sees in_transaction == false, and the abandoned row is absent.

The raw-SQL case from the original finding remains. I acquired the limit-1 pool connection, ran BEGIN and an insert through DBInterface.execute, then released it. The next acquire returned the same object, local in_transaction was false, and the uncommitted row was visible. reset_pooled_connection! returns early because the driver still discards the ReadyForQuery transaction status. Please keep this open until release is safe for both helper-started and SQL-started transactions.

[quinnj]

Retested on eab05bd. The new ReadyForQuery state does not fix this case because both a successful COMMIT and PostgreSQL rewriting COMMIT to ROLLBACK end with status I.

All three helpers still return :body_returned as success after the callback catches SELECT 1/0; the inserted row count is then 0. Please validate the simple-query CommandComplete tag for transaction control. A top-level commit must receive COMMIT, not ROLLBACK. Keep the Ready status for pool cleanup, but it cannot prove commit success by itself.

[quinnj]

[P1] Start a transaction for the advertised Statement cursor overload

The cursor(stmt, ...) overload does not start or require a transaction. Outside an explicit transaction, PostgreSQL ends the implicit transaction at the first Sync and drops the suspended portal. I reproduced the documented overload with SELECT generate_series(1, 5) and fetchsize=2: it returns rows 1 and 2, then fails with SQLSTATE 34000, portal ... does not exist. Please make this overload use the same owned-transaction setup and failure cleanup as cursor(conn, ...) (or reject it unless a transaction is already active), and add a multi-batch regression test for the Statement form.

[quinnj]

[P2] Enforce cache policy when a removed Statement is reused

An evicted Statement remains cached=true. checkstmt re-prepares it and inserts it into conn.statements without running LRU eviction. I reproduced this with statement_cache_maxsize=1: prepare SELECT 1, prepare SELECT 2 (cache size 1), then execute the retained first Statement; the cache grows to 2. The same path can repopulate the cache after clear_statement_cache! or when the maximum is 0. Repeating this with retained handles can grow the cache without the documented bound. Please route reinsertion through the active cache policy, or mark removed statements uncached, and add retained-handle regression cases for eviction and disabled caching.

[quinnj]

There is a second retained-handle failure when the SQL key is repopulated by a different Statement. With cache size 1, I prepared SELECT 1 as old, evicted it with SELECT 2, then prepared SELECT 1 again as a new object. Executing old sees the SQL key and skips re-prepare, but its server name was closed; PostgreSQL returns SQLSTATE 26000, prepared statement does not exist. The validity check must require conn.statements[stmt.sql] === stmt, not only haskey.

[quinnj]

A second exact-head case shows that stale Statement handles also break the configured cache bound.

With statement_cache_maxsize=1, I prepare A and then B, which evicts A. Executing the still-live A handle makes checkstmt reprepare it and insert it directly into conn.statements without eviction:

AFTER_EVICTION: size=1 keys=[B]
STALE_A_RESULT: 1
AFTER_STALE_REUSE: size=2 max=1 keys=[A, B]

It also defeats disabling the cache:

set_statement_cache_maxsize!(conn, 0)
AFTER_DISABLE: size=0
DBInterface.execute(old_b)
AFTER_REUSE_DISABLED: size=1 max=0

Please make stale-handle reuse follow the active cache policy. It must not insert when the max is zero, and it must evict before insertion when the cache is full. Tests should keep old handles across LRU eviction, clear_statement_cache!, and set_statement_cache_maxsize!(..., 0).

[quinnj]

A third handle-lifetime failure is independent caller aliasing. At exact head e8451c0, two DBInterface.prepare(conn, "SELECT 1") calls return the same mutable object (s1 === s2). Closing s2 makes DBInterface.execute(s1) throw statement has been closed.

The standard DBInterface function form makes this happen without an explicit second handle: hold s3 = prepare(conn, "SELECT 2"), then call DBInterface.execute(f, conn, "SELECT 2", nothing). DBInterface prepares internally and closes that handle in finally; because the cache returns s3 itself, the call succeeds but leaves the independently held s3 closed.

Please separate cache ownership from caller handle ownership, or add correct lease/reference tracking. Independent prepare calls and generic function-form execution must not close each other. Add both exact regressions alongside the eviction/stale-handle cases.

[quinnj]

A fourth cache-disabled path leaks server statements through the standard DBInterface API. With statement_cache_maxsize=0, I called connection-form DBInterface.executemany(conn, sql, params) five times. The generic DBInterface method calls this driver prepare method, but it never receives or closes the uncached handle. pg_prepared_statements grew from 0 to 5 while get_cached_statements(conn) stayed empty.

Please override the connection-form bulk path or otherwise guarantee that an uncached internally prepared statement is closed in finally. Add repeated executemany(conn, ...) and executemultiple(conn, ...) cases with cache size zero, checking the live server catalog, alongside the stale-handle and alias-lifetime regressions.

[quinnj]

The public cache contract is also inverted. The manual says Postgres.jl caches prepared statements internally and exposes statement_cache_maxsize, but three repeated DBInterface.execute(conn, "SELECT 42") calls leave get_cached_statements(conn) at size 0. Connection-form execute always uses the unnamed statement and bypasses the LRU. The cache is used only by explicit prepare calls, which is what creates the independent-handle aliasing and close-lifetime defects in this thread. Please settle the 1.0 ownership design: use an internal cache for connection-form execution if that is the documented feature, while explicit prepare returns independently closeable handles, or rewrite the docs and option names to describe exact behavior. Add repeated direct-execute cache hit/eviction tests.

[quinnj]

[P2] Apply connection-level debug to normal prepare and execute calls

conn.debug is not combined with the per-call debug flag in DBInterface.prepare, either DBInterface.execute overload, or their describe/exec calls. I verified this with a TestLogger: a connection created with debug=true produced zero wire-log records for DBInterface.execute(conn, "SELECT 1"), while the same call with debug=true explicitly produced 19. This also means the newly honored ConnectionParams.debug field does not affect the main query path. Please use debug || conn.debug consistently and add a log-capture regression test.

[quinnj]

[P1] Redact the password when ConnectionParams is displayed

The default struct display prints the password verbatim. For example, repr(parse_dsn("postgresql://alice:s3cr3t@localhost/db")) currently includes "s3cr3t". This makes a public configuration value leak credentials when it is the last REPL expression, logged, or included in an error report, despite the wire-debug redaction in this PR. Please define redacted show methods for ConnectionParams and add a test that the password never appears in either compact or text/plain output.

[quinnj]

[P1] Make escape_literal safe for every supported string mode

Documenting this assumption does not make the public escaping helper safe. I reproduced a predicate injection on PostgreSQL 16 after SET standard_conforming_strings=off: an input containing a backslash, quote, and OR true -- escapes the first generated quote, and a query whose original WHERE condition is false returns a row. Please emit an escape-string literal with both backslashes and quotes handled, or make escaping connection-aware, so the result is safe independent of this session GUC. Add the off-mode regression test. Parameters remain preferred, but an API named escape_literal must not have a server-supported injection mode.

[quinnj]

[P2] Reject every extra statement, not only a second COPY FROM

The single-statement guard detects only another CopyInResponse. Other statements are still executed and silently drained. I reproduced copy_from(conn, "COPY t FROM STDIN; COPY t TO STDOUT", "7\\n") returning success after discarding the second copy’s output. I also reproduced copy_to(conn, "COPY t TO STDOUT; COPY (SELECT 8) TO STDOUT") returning the ambiguous concatenation "7\\n8\\n". Track command/statement boundaries and reject any response sequence after the one expected COPY operation, while still draining safely. Add mixed-direction and two-copy-out cases to the regression suite.

[quinnj]

[P1] Make closing a pool terminal and handle checked-out connections

DBInterface.close!(pool) only closes connections currently in the idle values list. It records no closed state. I reproduced this by acquiring the sole connection, closing the pool, then releasing and acquiring again: the checked-out connection remained open, the closed pool accepted it, and acquire returned the same live connection. Please add pool lifecycle state: reject new acquires after close, close idle connections immediately, and close checked-out connections when they are later released. Add this checked-out-close regression test.

[quinnj]

[P2] Do not promise arbitrary julia_type conversion without a parser

Only Symbol gets a parser. For any other custom julia_type, the registry advertises that type but parse_value falls through and returns the wire String. I verified this directly with a wrapper type: type_info(...).julia_type was the wrapper while the parsed value type was String. Either restrict and document this keyword to the actually supported choices, or accept/build an explicit parser for custom enum types. Add a test that the declared row schema and actual value type agree.

[quinnj]

This also affects the generic register_type! contract. Its new doc says that omitting parser returns String, but TypeInfo still records the caller supplied julia_type, so Tables.schema advertises that type while row values are strings. Please make the stored schema type match the parser result, or require a parser for non-String registrations. Cover both register_enum! and register_type! with Tables.schema plus actual-value assertions.

[quinnj]

6323566 makes the schema truthful only after a non-NULL value is parsed. The contract is still inconsistent for empty and all-NULL results. With register_type!(conn, 23, Wrap) and no parser, a nonempty result now has schema String, an empty result has schema Wrap, and an all-NULL result has schema Union{Missing, Wrap} even though no Wrap value can ever be produced. Please fix the registry metadata at registration time instead of inferring the parser result from observed rows.

[quinnj]

[P1] Control or honor DateStyle before parsing fixed-layout dates

The date/time parsers assume ISO output, but the connection neither requests ISO nor parses according to the server-reported DateStyle. This can be a role or database default, not only a user change. I reproduced it with SET DateStyle = 'SQL, DMY'; selecting DATE '2026-08-06' then throws Month: 2552 out of range because the wire value is 06/08/2026. Please establish the required text format during startup (and cover other parser assumptions such as IntervalStyle), or dispatch parsing from current server parameters. Add a connection whose initial role/database settings are non-default, not only a post-connect SET test.

[quinnj]

Additional live cases show that ISO alone is not enough. SELECT make_date(10000,1,2) fails with Month 2530 out of range. SELECT make_date(-1,1,2) silently returns 0001-01-02 AD even though PostgreSQL sent a BC date. The valid date value infinity also throws a generic parser error. Please define the 1.0 support contract for PostgreSQL date/timestamp ranges and special values, then parse supported values without silent era changes. Add these cases with the DateStyle tests.

[quinnj]

More live temporal failures confirm this needs a full format contract. With IntervalStyle=sql_standard, a valid interval fails while parsing +1-2; with IntervalStyle=postgres_verbose, it fails on @. Valid TIME 24:00:00 throws because Julia Time rejects hour 24. A historical timestamptz whose server text ends in -04:56:02 is silently converted as if the offset were -04:56, producing a value two seconds early. Please include IntervalStyle, the PostgreSQL 24:00 boundary, and offset seconds in the temporal support decision and regression matrix.

[quinnj]

There is also silent subsecond loss in the default temporal mappings. Live TIME 12:34:56.123456 returned Time(12, 34, 56, 123), even though Julia Time can hold finer precision. TIMESTAMP 2026-01-02 12:34:56.123456 returned a millisecond DateTime. Please preserve all representable time precision, and make the timestamp/timestamptz precision limit explicit or use a lossless representation. Add microsecond round trips so the 1.0 behavior is deliberate rather than silent.

[quinnj]

One IntervalStyle case is silent data corruption, not only a parse failure. With IntervalStyle=iso_8601, PostgreSQL sent P1Y2M3DT4H5M6S for a nonzero interval. parse_interval found no recognized periods and returned Dates.Millisecond(0). Please ensure unsupported formats fail closed if they are not parsed, and test that no valid nonzero interval can become zero.

[quinnj]

Behavior is fixed at 0da51f2 for initial role/database defaults. I created a login role with DateStyle = SQL, DMY and IntervalStyle = iso_8601, then connected through Postgres.jl. The startup options overrode both to ISO, MDY and postgres; a date and a compound interval parsed correctly.

Please strengthen the committed regression before resolving this. It currently asserts those two settings against an ordinary PostgreSQL image whose defaults are already ISO, MDY and postgres, so the test passes even if the new startup options are deleted. Configure a non-default role/database in the test as the original finding requested, then assert both status values and decoded data. Also document that changing these required formats after connection is unsupported or detect it: SET DateStyle TO SQL, DMY still makes the next valid date fail in the parser.

[quinnj]

Exact-head follow-up at eab05bd: the initial-default case is fixed, but a valid later SET still breaks both the decoder and the public state API. After SET DateStyle = SQL, DMY, get_server_parameter(conn, "DateStyle") still reports "ISO, MDY", while SELECT DATE "2026-08-06" throws Month: 2552 out of range. After SET IntervalStyle = sql_standard, the getter still reports "postgres", while a normal mixed interval throws invalid base 10 digit in "+1-2". Please either maintain the enforced formats after every ParameterStatus change or parse according to the live value. The getter must report the server value. Add post-connect SET regressions for both settings, not only non-default initial role/database settings.

[quinnj]

[P1] Pin client_encoding to UTF8 for Julia strings

The startup message leaves client_encoding at the server, role, or database default, while every query and result is encoded/decoded as Julia UTF-8. I reproduced the data error after setting client_encoding=LATIN1: the string é appears to round-trip, but PostgreSQL reports length($1::text) == 2 because the two UTF-8 bytes were interpreted as two LATIN1 characters. A non-UTF8 role default causes this from the first query. Please request UTF8 in the StartupMessage and verify the returned ParameterStatus before normal traffic. Add a non-ASCII parameter/result test from a connection whose initial client encoding is non-UTF8.

[quinnj]

[P1] Apply connect_timeout to the full handshake

connect_timeout currently bounds only the TCP dial and TLS handshake. All pre-TLS, startup, and authentication reads can still block forever after a peer accepts the socket. I reproduced this with a local server that accepts and never answers: connect_timeout=1 was still blocked after a two-second timedwait, and returned only after I closed the accepted socket. Please install one connection-establishment deadline across SSL negotiation, StartupMessage, and authentication, then clear it when the connection becomes ready. Add stalled plain and TLS-negotiation server tests.

[quinnj]

[P1] Treat a short discard as EOF instead of success

The skipbytes! helper immediately above ignores the count returned by readbytes! and subtracts the requested size. Reseau documents that this call can stop short at EOF, so truncated message bodies can be reported as fully consumed. A final expected message can therefore make an operation return success with a broken stream. Please require readbytes! to return the requested count (otherwise throw EOFError so the caller closes the socket), and add short-body tests for the helper and a final protocol message.

[quinnj]

[P1] Keep the caller timeout after the first byte

Clearing the deadline here makes timeout ineffective once any byte arrives. A peer can send only the message-type byte, or a partial header/body, and wait_for_notification(timeout=...) will then block forever in the following reads. Keep the first-byte polling behavior, but after that byte set an absolute deadline for the caller’s remaining timeout while reading the rest. If that deadline expires mid-message, close the desynchronized socket and throw; only a timeout before the first byte can safely return nothing or poll again. Add a trickle test that sends one byte and stalls past the requested timeout.

[quinnj]

This commit does not resolve the blocker. It now documents that the caller timeout ends when a message starts, so a peer that sends one type byte can still block the call forever. There is also a direct contradiction in the new TLS note: timeout === nothing still installs a 100 ms read deadline on every poll, so the claimed blocking wait is exposed to the same split-record path. If TLS reads cannot resume after a transport deadline, the no-timeout path must use a true blocking read, and the finite-timeout path must not treat a deadline as safe unless it knows no TLS-record bytes were consumed.

[quinnj]

[P1] Preserve asynchronous messages instead of silently dropping them

This fixes stream alignment but loses valid notifications and notices. I reproduced LISTEN, then NOTIFY from a second connection, then DBInterface.prepare on the listener: the prepare succeeds now, but wait_for_notification(timeout=0.2) returns nothing because this branch discarded the event. Custom notification/notice callbacks are also bypassed. Thread connection/style context through these setup waits and queue notifications for wait_for_notification while invoking the documented callbacks. Extend the regression test to assert delivery, not only that the next query survives.

[quinnj]

[P1] Scope every information_schema join to the requested schema

The query matches key_column_usage only by table and column names. The following joins also omit catalog and schema keys. I reproduced this with schemas review_a and review_b. Each schema had parent(id) and child(id, parent_id REFERENCES parent). Postgres.describe(conn, "child"; schema="review_a") returned 12 rows instead of 2. The rows included constraints from both schemas.

Please join on table_catalog/table_schema, constraint_catalog/constraint_schema, and the relevant ordinal-position keys. Add a regression with duplicate table and constraint names in two schemas. Composite foreign keys also need position-aware joins so each local column maps to the correct referenced column.

[quinnj]

The query also needs a one-row-per-column invariant. In one schema, a column with both UNIQUE and FOREIGN KEY constraints already produces duplicate describe rows because the join emits one row per constraint. Keep c.ordinal_position, aggregate or use scoped EXISTS/lateral lookups, and ORDER BY ordinal_position. Add a column with multiple constraints plus a composite foreign key to the regression.

[quinnj]

[P2] Use a machine-detectable MIT license file before 1.0

GitHub currently classifies this repository as Other with SPDX id NOASSERTION. This file wraps the license text as a Markdown blockquote and adds a custom lead sentence, so automated consumers do not recognize the standard MIT text.

Please use the canonical MIT license text as plain text, preferably in LICENSE, with only the copyright line customized. Verify that GitHub reports MIT after merge, or validate the file with a license detector before merge.

[quinnj]

[P1] Preserve the transaction body error when cleanup also fails

All three transaction helpers replace the primary failure with a rollback failure.

At exact head fa9bad1:

Postgres.transaction(conn) do conn
    close(conn.socket)
    error("primary failure")
end

The result is not ErrorException("primary failure"). It is:

PostgresInterfaceError: postgres connection has been closed or disconnected;
reconnect disabled during transaction

I reproduced the same replacement through DBInterface.transaction and Postgres.@transaction. A real backend or network failure during the body has the same shape: the query error explains the incident, then rollback(conn) fails and hides it.

Please make rollback best-effort during unwinding. Preserve the body exception as the primary error. If cleanup also fails, attach or report that secondary failure without replacing the primary cause. When the transport is already dead, clear the local transaction bookkeeping because the server-side transaction ended with the session. Add the same regression test for all three helpers.

Retested at exact head 2cf22bf. The dead-socket body case still loses ErrorException("primary failure") in all three helpers; each now reports only PostgresInterfaceError: postgres connection has been closed or disconnected.

This commit also exposes the same error-precedence bug on the normal commit path. With a UNIQUE ... DEFERRABLE INITIALLY DEFERRED constraint, each helper inserts duplicate rows successfully and receives SQLSTATE 23505 only from COMMIT. commit clears the transaction state in finally, then the helper catch block calls rollback; that throws PostgresInterfaceError: no transaction in progress and replaces the real commit error. The helper must distinguish a body failure from a commit failure, and it must never replace either primary error with cleanup output.

Verified fixed at eab05bd. All three dead-socket body cases now rethrow ErrorException("primary failure") and clear local transaction state. All three deferred-constraint cases preserve Postgres.Error with SQLSTATE 23505 and finish with no local transaction open.

[quinnj]

[P1] Close an exhausted named portal too

done means the Execute reached CommandComplete; it does not mean PostgreSQL dropped the named portal. This guard therefore makes DBInterface.close!(cursor) a no-op for a fully consumed cursor inside a caller-owned transaction.

I verified this at fa9bad1 with PostgreSQL 16: start a transaction, build cursor(stmt; fetchsize=2), consume all five rows, and query pg_cursors by cursor.portal. The portal count is 1 after exhaustion and remains 1 after DBInterface.close!(cursor). It disappears only when the transaction ends. Repeating completed cursors in a long transaction accumulates server-side portals even though callers close each cursor.

Please track portal closure separately from row completion and send Close(P) once for every named portal that is still registered, including exhausted ones. Add a live pg_cursors regression test that remains inside the transaction and observes the portal disappear immediately after close.

[quinnj]

Retested at exact head 2cf22bf. The named portal is still present in pg_cursors after full exhaustion and remains present after DBInterface.close!(cursor). I matched pg_cursors.name against the exact UUID in cursor.portal, so the inspection query portal is excluded. The new double-close guard is useful but does not address this resource leak.

[quinnj]

[P1] Make the documented first run work after the stated install step

Pkg.add("Postgres") followed by the quick-start import fails in a clean environment with ArgumentError: Package DBInterface not found in current path. DBInterface, Tables, and StructUtils are transitive dependencies, so users cannot import them directly unless they also add them to their environment. The manual has the same problem with using Postgres, Tables, and it closes conn before later snippets reuse it.

Please make the default quick start runnable after only Pkg.add("Postgres"). Use the DBInterface binding that Postgres intentionally exports and avoid Tables in the minimal first example. For examples that require Tables or StructUtils, state that users must add those optional integration packages. Please keep the package export surface narrow; exporting more dependencies is not the fix.

[quinnj]

[P1] Make duplicate result labels compatible with Tables.jl

PostgreSQL permits duplicate output labels, including the ordinary unlabeled query SELECT 1, 2. At e8451c0, both SELECT 1 AS x, 2 AS x and SELECT 1, 2 produce duplicate names. Tables.rowtable(result) then throws duplicate field name in NamedTuple, while result[1][:x] silently returns only the second value.

Please normalize result labels to stable unique symbols before constructing Result, ResultRow.lookup, and cursor rows. Preserve positional access. Add regressions for explicit duplicate aliases and repeated ?column? labels, including Tables.rowtable and cursor materialization. The schema, row names, and symbol lookup must use the same normalized names.

[quinnj]

[P1] Do not leave @transaction open on early control flow

The macro has only a catch cleanup path. A return, break, or other non-throwing control transfer from expr bypasses both commit and rollback.

At exact head e8451c0, I called a function whose @transaction block inserted one row and then executed return :early_return. The function returned that value, Postgres.in_transaction(conn) remained true, and the row was still visible as uncommitted work on the same session. A later manual rollback removed it. Returning this connection to a pool would expose the open transaction to another borrower.

Please make every exit path finish the transaction, and evaluate the conn expression only once. Add early return, loop break, thrown-error, and normal-completion regressions. Define whether nonlocal successful exits commit or roll back, but never leave the transaction open.

[quinnj]

Review follow-up at 007ceaa — transaction error precedence is only partly fixed

The commit-failure half is fixed. I retested a deferred unique violation through Postgres.transaction, DBInterface.transaction, and @transaction. All three now preserve Postgres.Error with SQLSTATE 23505, clear in_transaction, and store no rows.

The body-error half remains. In each helper, I closed the session socket and then threw ErrorException("primary failure") inside the body. All three return only PostgresInterfaceError: postgres connection has been closed or disconnected. The primary body exception is still lost, although local transaction state is now cleared. The catch path must run rollback as best-effort cleanup without letting its failure replace the active exception.

[quinnj]

Review follow-up at 0da51f2 — internal "char" is only partly fixed

The valid scalar zero value now reads as Char(0), and SQL NULL remains missing. This fixes the original scalar read exception.

The requested lossless boundary is not complete. A live ARRAY[::"char", Z::"char", NULL] result still returns the raw string {"",Z,NULL} because OID 1002 is not registered. Binding the new Char(0) representation back to $1::"char" sends a NUL byte and fails with SQLSTATE 22021 (invalid byte sequence for encoding "UTF8": 0x00); binding Z works. Please add the array OID mapping and serialize Char(0) as the empty text representation so the chosen scalar type round-trips. Keep one-byte, zero, NULL, array, and bound-parameter cases in the live regression.

[quinnj]

[P1] Do not pass a trim check that did not compile or run

On exact head c1ae7ba with Julia 1.12.6, JuliaC 0.3.8, and released StructUtils 2.8.3, the direct command used here ends with Trim verify finished with 46 errors and ERROR: Failed to compile ...postgres_trim_queries.jl; it creates no runnable product. This test nevertheless passes because 46 is below the budget of 92, and lines 211-214 skip the executable check whenever any verifier error exists.

The failures are also not all one StructUtils construction error. The verifier reports Base64/SCRAM paths (errors 1-8 and 31-38), typed StructUtils sinks, and Dates/Base formatting paths. Thus the current green job cannot establish the PR description trim-compile claim and would allow many new errors before it failed.

Please make the supported contract explicit. If trim-safe compilation is a 1.0 claim, require zero verifier errors, a successful compiler exit, and execution of the produced binary against PostgreSQL. If it is not a 1.0 support claim, mark this as an expected-failure or unsupported lane and remove the statement that the trim-compile check passes. An error budget must not count a failed compile as a passing integration test.

[quinnj]

[P1] Preserve PostgreSQL microseconds for time and interval

Exact-head live repro on PostgreSQL 16:

SELECT
    make_time(12,34,56.123456) AS t,
    ARRAY[make_time(12,34,56.123456)] AS ta,
    make_interval(secs => 0.123456) AS i;

The driver returns Time(12, 34, 56, 123), Any[Time(12, 34, 56, 123)], and Millisecond(123). The expected Time is Time(12, 34, 56, 123, 456). PostgreSQL has six-digit fractional precision, and Julia Time and Microsecond can represent it. The current _pg_hms_at stops after three digits, so pg_parse_time, time arrays, and parse_interval_time silently discard 456 microseconds.

DateTime itself has millisecond precision, so timestamp truncation can be a documented type limitation. The same reason does not apply to time or interval. Please parse all six PostgreSQL fractional digits for these types and add scalar, array, and interval live regressions.

The fixed-position date parser also silently changes eras. SELECT DATE '0001-01-01 BC', TIMESTAMP '0001-01-01 02:03:04 BC' returns text with a BC suffix, but the driver ignores that suffix and returns Julia year 1 for both values. PostgreSQL 1 BC maps to astronomical year 0, not AD year 1. Years with more than four digits fail with an unrelated month-range error because the parser assumes the first hyphen is at byte 5. Please either map the full PostgreSQL date domain correctly or reject unsupported eras/ranges clearly. Do not return a different date.

There is a second silent timestamp error for real historical zone offsets. After SET TIME ZONE 'Europe/Paris', PostgreSQL renders the UTC instant 1890-01-01 00:00:00+00 as 1890-01-01 00:09:21+00:09:21. The driver returns DateTime("1890-01-01T00:00:21"), which is 21 seconds late, because tzoffset_seconds parses hours and minutes but ignores offset seconds. Please include seconds in the offset parser and cover a historical IANA-zone case.

[quinnj]

[P1] Do not reject valid three-dimensional built-in arrays

On exact head c1ae7ba, PostgreSQL 16 returns ARRAY[[[1::integer]]] correctly as Any[Any[Int32[1]]]. The same shape for date[] and jsonb[] throws PostgresInterfaceError("arrays nested deeper than two dimensions are not supported on the untyped parse path"):

SELECT ARRAY[[[make_date(2024,1,2)]]];
SELECT ARRAY[[[to_jsonb(1)]]];

These are valid standard PostgreSQL values. The limit also affects the other built-in arrays that use parse_array_by_oid, such as time, timestamp, timestamptz, interval, numeric, bytea, and UUID arrays. The connection remains usable after the conversion error, but the value cannot be read.

Please make all registered built-in array parsers handle every PostgreSQL array dimension. If trim compilation cannot support that implementation, document and enforce the reduced support contract explicitly instead of calling valid built-in values exotic only in a source comment. Add a live three-dimensional regression for at least one direct parser and one OID-based parser.

[quinnj]

[P1] Decode doubled quotes in composite fields

PostgreSQL emits a quote inside a composite field by doubling it. This live value has four fields:

CREATE TYPE pg_temp.codex_comp AS (a text, b text, c text, d text);
SELECT ROW('a"b', E'c\\d', 'x,y', '')::pg_temp.codex_comp::text;

The server text is ("a""b","c\\\\d","x,y",""). On exact head, parse_composite_fields returns five values: ["a", "b", "c\\d", "x,y", ""]. register_composite! therefore throws its length-mismatch error for this valid row.

The quoted-field loop treats every " as the closing delimiter. Please recognize "" as one literal quote, while retaining the backslash handling, and add a live registered-composite round trip with quote, backslash, comma, empty string, and SQL NULL fields.

[quinnj]

[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]

[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]

[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]

[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]

[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]

[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]

[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]

[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]

[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]

[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]

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]

[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]

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.