Repository navigation
Proposals from the rollout (fetch_one/fetch_scalar/execute, ComplexHelper cache, readonly_transaction, ...) - #52
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…_most() statement_timeout Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…er connection Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ostgresJsonResponse Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…, cache=False semantics, order_by ambiguity) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…t/configure/check to the psycopg pool Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…middleware notes Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #51 (not
Closes: the lazy default pool,where_and,paginateandfetch_all(read_only=True)are proposed below, not implemented)Fixes #53
Version 0.14.2 -> 0.15.0 (additive; behaviour notes under "Decisions").
Implemented
fetch_one,fetch_scalar,executeinpgdevkit/db/fetch.py, exported frompgdevkit.db. They share_acquire/_render/_statement_timeoutand the cancel/abort machinery withfetch_allthrough one internal_run(only what is read from the cursor differs), same keywords (con=/pool=/cancel=/statement_timeout=,model=/row_mapper=onfetch_one), t-string overloads withoutparams.executereturns the rowcount; on a pooled connection it commits when the call returns, on a givenconnothing is committed.ComplexHelpercache (pluscomplex_helper="auto"on everypg_*helper, from the OneSales comment: a default helper per call, which the cache makes cheap; opt-in, so existing reads stay plainSELECT *): catalog lookups, column types and "registered on this connection" state are cached per connection (WeakKeyDictionary, so it dies with the connection and pooled connections keep theirs). A helper created per call now costs 1 query (the SELECT) instead of ~5 after the first use of a connection; asserted in tests by counting statements.helper.clear_cache()andComplexHelper(con, cache=False)for schema changes. Also fixes_get_complex_typere-fetching (and re-registering) enums on every call.pg_retrieve_many:where=(t-string, orpsycopg.sqlcomposable +params=),order_by=(column /(column, "asc"|"desc")/ list / composable),limit=. Backwards compatible. Smallest design that fits; the select-list builder was not made public (where/order_by/limitcover the MDMApp case; say if you want it exported too).set_default_pool()also accepts a plainCallable[[], AsyncContextManager[AsyncConnection]](PoolLike).readonly_transaction(con):set_read_only(True), acon.transaction()block, and afinallythat always putsread_onlyback (to its previous value, i.e.Nonenormally) (skipped for a closed/broken connection so it cannot mask the original error). Tested on a 1-connection pool, with errors, a refused write, and an autocommit connection.statement_timeout=at_most(seconds): opt-in "never loosen" (min(current, requested), current 0 = unlimited), computed server-side frompg_settings.at_mostreturns afloatsubclass, so no signature changed; works forfetch_*,executeandPostgresJsonResponse. Plain values keep replacing the setting in both directions; documented.%in sqlglot expressions: withparams is not None,_renderdoubles every literal%(string literals, quoted identifiers,%modulo) but not the%(name)splaceholders (rendered as unique tokens first, then restored), on a copy of the tree. Also applied toPostgresJsonResponse. Docstring/README/SKILL no longer document it as a limitation.PgPool(OneSales):min_size,timeout,configure,checkpass through to the psycopg pool (min_sizenow defaults tomin(4, max_size), somax_size < 4no longer raises). PgPool._dsn doesn't quote password/dsn_params values (spaces, backslashes break the connection) #53: the conninfo is built withmake_conninfo, so passwords /dsn_paramswith spaces, backslashes, quotes work; round-trip tests incl. Azure tokens.{"error"}vs{"detail"}(plus a small opt-inerror_key = "detail"class attribute onPostgresJsonResponse),NaN/Infinityas strings, theBaseHTTPMiddlewaretruncation caveat,+00:00vsZ,statement_timeoutsemantics. README andskills/pgdevkit/SKILL.mdupdated for everything above.Decisions for a human
readonly_transactionis not a sandbox (found in review, documented in the docstring/README/SKILL): SQL without bound params uses psycopg's simple protocol, soCOMMIT; INSERT ...leaves the read-only transaction (TEMP tables stay writable,SETsticks). Untrusted SQL needs a SELECT-only DB role in addition. A stricter helper (e.g. rejecting multi-statement text) could be built on top; not done here. This also argues againstfetch_all(read_only=True)(OneSales): it would suggest more safety than it gives, so it is proposed only (usereadonly_transaction).PgPool.open()is already idempotent. Proposal:PgPool(..., lazy=True)whoseconnection()returns an async context manager that doesawait self.open()(guarded by anasyncio.Lock) before borrowing, plus the existing explicitclose()at shutdown. Open points: who callsclose()if nobody opened at startup, and whether a failed first open (bad credentials, DB down) is retried on the next use or cached.where_and(conditions)/paginate(query, limit, offset, order_by=)(OneSales): not done. They are optional andpg_retrieve_many(where=, order_by=, limit=)already covers the table-bound case; for free-form queries a t-string with{x:q}fragments does the job. Happy to add if wanted, but I'd want the target API agreed first (the returned fragment type:psycopg.sql.ComposedvsTemplate).ComplexHelpercache default on, as the issue asks. Risk (review finding): DDL rolled back after it was cached (e.g. a test fixture that creates types in a rolled-back transaction on a reused connection) leaves stale OIDs (cache lookup failed for type ...); documented,helper.clear_cache()/cache=Falsehandle it. Making it opt-in would avoid the hazard but defeat the per-call-helper use case; your call. An explicitcomplex_fields=shortcut was not added.LIKE '50%%'in a sqlglot expression together withparams(the old documented workaround) would now send50%%to Postgres (harmless for LIKE wildcards, but different for=/to_charformats); CCMT2'sFixedPostgresdialect, which already doubles%, would double twice. Plain strings andpsycopg.sqlcomposables are untouched.readonly_transactionrestores the connection's previousread_onlyvalue instead of hard-resetting toNone(identical in the normal case; keeps a replica connection'sTrue).exp.Placeholder()/%s) placeholders in sqlglot expressions remain rejected by the existing name check; unchanged.Test notes
Live-Postgres tests were run against a private throwaway cluster (they skip without a container runtime):
tests/db/test_fetch.py,test_fetch_statement_timeout.py,test_transaction.py,test_complex_types.py,test_crud.py,tests/test_pg_json_response.py,tests/test_pg_json_statement_timeout.py.uv run ty check pgdevkitis clean;import pgdevkit.dbstill does not load sqlglot.🤖 Generated with Claude Code