feat: empty statement implementation for server-api - #461
Merged
Conversation
… PostgreSQL An empty query string (no statement: "", ";", ";;", whitespace-only) sent with Parse used to be dispatched to the user's QueryParser, which typically fails with a syntax error. Real PostgreSQL accepts it: Parse succeeds, the statement name is remembered as empty, and later Bind/Describe/Execute on that name follow empty-query semantics. Do we need to store the empty statement server-side? Yes, but only as a name marker, not a statement: Parse of an empty query must replace any statement previously stored under the same name, and Bind on that name must succeed and shadow portals of the same name. There is no parsed value to store, so instead of changing PortalStore/StoredStatement/Portal to carry an empty variant (a breaking change for every implementor), the default extended-query handlers track empty statement and portal names in a private per-connection registry kept in SessionExtensions: - on_parse: empty queries never reach QueryParser; the name is marked empty (replacing any stored statement) - on_bind: binding an empty statement succeeds with zero parameters (binding parameters is rejected with 08P01, like PostgreSQL) and shadows portals of the same name - on_execute: an empty portal answers EmptyQueryResponse and never calls do_query; the portal stays valid across repeated Execute - on_describe: an empty statement describes as ParameterDescription (no parameters) + NoData; an empty portal as NoData - on_close/on_sync: drop empty statements/portals like real ones Behavior verified message-for-message against PostgreSQL 18.4 driven over the wire (raw socket probe), and covered by unit tests with a mock client asserting the exact message sequences.
After review: there are almost no direct PortalStore implementors or
callers outside the crate (only examples/cursor.rs), so representing the
empty variant in the store costs far less breakage than any change to
StoredStatement/Portal (which every do_query implementation touches
through portal.statement.statement) — and it is the better home for the
state anyway.
PortalStore now models three states per name (missing / empty / real):
- StatementEntry { Empty, Statement(Arc<StoredStatement<S>>) } returned
by get_statement, with as_statement()/is_empty() helpers
- PortalEntry { Empty, Portal(Arc<Portal<S>>) } returned by get_portal
- new put_empty_statement/put_empty_portal store empty markers; like
every put_*, they replace whatever was stored under the name, so
rm_*/clear_portals remove empty entries along with regular ones
MemPortalStore stores the entries in its maps directly. The private
EmptyStatementRegistry (SessionExtensions) from the previous commit is
removed: replacement/removal/clearing now fall out of normal store
semantics instead of a second bookkeeping structure that could drift
(e.g. clear_portals not clearing markers).
Wire behavior is unchanged: verified message-for-message against
PostgreSQL 18 again with the raw-socket probe (Parse/Bind/Describe/
Execute/Sync of empty queries, statement and portal shadowing, 08P01
on bound parameters, Close/Sync cleanup).
Keep only brief comments where they add non-obvious information; drop narration of self-explanatory code.
QueryParser::parse_sql now returns Option<Self::Statement> and StoredStatement::parse returns Option<StoredStatement<S>>: None denotes an empty query, stored as an empty statement and executed to EmptyQueryResponse. The syntactic empty-query check (semicolons and whitespace only) moved into StoredStatement::parse, so on_parse has a single branch and custom on_parse overrides calling it get empty-query handling for free.
Replace StatementEntry/PortalEntry with a single Entry<T>: Empty marker or Value(Arc<T>), where T is StoredStatement<S> or Portal<S>. Clone is implemented manually so the payload type does not need to be Clone.
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.
Fixes #304
This is my idea about #453 . To minimize the breaking change impact, we updated
PortalStoreAPI to accept a new variant calledEmpty. The framework layer should handle empty query automatically.