test: integration tests for client API against real PostgreSQL - #458
Merged
Conversation
Add tests-integration/client-api, a workspace member that verifies the pgwire client API (PgWireClient, DefaultStartupHandler, simple and extended query handlers, cancellation) against a real PostgreSQL 18 server. run.sh manages the server lifecycle with podman; CI gets a postgres:18 service container job. The suite exposed and fixes several client-side protocol bugs: - ExtendedQueryClient::prepare never sent a Describe message, so ParameterDescription was never returned and param_types was always empty. Prepare now sends Parse + Describe + Sync. - The extended query methods terminated every step with Sync, which ends the extended-query cycle and destroys bound portals (portals do not survive the implicit transaction end). Bind now uses Flush to keep the cycle open; Execute closes it with Sync only on CommandComplete and leaves it open on PortalSuspended; Describe uses Flush for portals and Sync for statements; the one-shot query runs Parse+Bind+Execute+Sync in a single cycle. - Error paths left the trailing ReadyForQuery unread (desynchronizing the next query) or sent a redundant Sync (making the server emit an extra ReadyForQuery with the same effect). Errors now drain to ReadyForQuery, syncing only when the cycle is still open; the same recovery is applied to PgWireClient::simple_query. - SimpleQueryHandler::on_message rejected ParameterStatus messages that servers emit mid-query (e.g. after SET) as unexpected. - Tag::from_str failed on multi-word command tags (CREATE TABLE, DROP TABLE, ...) and read the INSERT tag's oid/rows in the wrong order, disagreeing with Tag-to-CommandComplete encoding.
Servers report parameter changes (e.g. after `SET`) with ParameterStatus messages both during startup and during query execution. The client only cached them during startup, leaving server_parameters() stale for the rest of the session. Add ClientInfo::set_server_parameter (implemented by PgWireClient to update its cache) and route every mid-query ParameterStatus through it: - SimpleQueryHandler::on_message now forwards ParameterStatus to a new on_parameter_status hook, whose default implementation updates the client cache; custom handlers can override it. - The extended query client updates the cache in all of its message loops (prepare/bind/execute/describe/close/query) and in the Sync/finish and error-drain helpers. - PgWireClient::simple_query's post-error drain updates the cache too. Verified by integration tests against a real PostgreSQL 18 server: SET via simple query and via extended query now reflect the new value in client.server_parameters().
Previously startup ParameterStatus messages were only stored when the startup handler collected them itself: DefaultStartupHandler kept a duplicate map that was cloned into ServerInformation at ReadyForQuery and assigned wholesale to the client, and custom handlers that did not track parameters ended up with an empty cache (the assignment could even discard cached entries). Store parameters on the client as they arrive, mirroring the mid-query behavior: - StartupHandler::on_parameter_status gains a default implementation that calls ClientInfo::set_server_parameter, so every handler caches parameters without extra work. - DefaultStartupHandler drops its duplicate parameter map; on_ready_for_query reports the client's cache instead. - PgWireClient::connect merges the handler-returned ServerInformation over the incrementally cached parameters instead of overwriting, so the cache is complete regardless of how the handler builds its return value. The startup integration test now asserts the full GUC_REPORT set (server_version, server_encoding, client_encoding, application_name, standard_conforming_strings, integer_datetimes, DateStyle, TimeZone) is cached, including the application_name echoed back from the Startup message.
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
1 participant
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.
this patch adds integration tests for client-api, and resolves issues found with those tests.