Skip to content

Commit 0c674ef

Browse files
committed
build: bump pgwire to 0.39 and migrate to SessionExtensions
pgwire 0.39.0 added typed per-connection state via `SessionExtensions`, which was the upstream change `TODO.md` was waiting for. Migrate `ConnectionState` storage off the global `DashMap` keyed by `{addr}_{pid}` and onto `client.session_extensions()`. Mechanical changes: - `session.rs`: drop the `CONNECTIONS` global, the `_conn_id` metadata key, `register_connection`, `get_connection`, and `remove_connections_for_addr`. Cancel registry stays — a `CancelRequest` arrives on a *different* TCP connection that doesn't have access to the originating connection's `SessionExtensions`, so a process-wide map is still required. Replaced its `dashmap::DashMap` with `Mutex<HashMap>` since the registry is small and only touched on connect/disconnect/cancel. - `cancel.rs`: `lookup_cancel` now returns the cloned `(client, active_query_id)` pair directly, so the registry mutex is released before any await and the previous "snapshot then drop the ref guard" dance is gone. - `startup.rs::establish_session`: replaces `register_connection` with `client.session_extensions().insert(ConnectionState { ... })`. No more `_conn_id` formatting or metadata insertion. - `query_simple.rs` / `query_extended.rs`: replace `session::get_connection(&conn_id)` with `client.session_extensions().get::<ConnectionState>()`. The returned `Arc<ConnectionState>` is `Send + Sync + 'static`, so we drop the per-handler `Arc::clone` dance and the `drop(conn_state)` before-await workaround. - `main.rs`: drop `ConnectionCleanup` (the per-task drop guard that called `remove_connections_for_addr`). Cleanup happens via `Drop for ConnectionState` when pgwire tears down the connection's `SessionExtensions`. - `Cargo.toml`: drop `dashmap` direct dependency (still pulled in transitively by trino-rust-client). - `SimpleQueryHandler` no longer requires `ClientPortalStore` in 0.39 — drop the trait bound. Also bumps tokio (1.52.0 -> 1.52.1) and rustls-pki-types (1.14.0 -> 1.14.1) to their latest patch revisions. `cargo build --all-targets`, `cargo test --lib` (191 passed), `cargo test --bin` (5 passed), `cargo clippy --all-targets -- -D warnings`, `cargo fmt --check`, and `reuse lint` (59/59) all pass post-migration. Net: -125 lines (244 deletions, 119 insertions).
1 parent cece3ed commit 0c674ef

8 files changed

Lines changed: 119 additions & 244 deletions

File tree

Cargo.lock

Lines changed: 6 additions & 33 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ repository = "https://github.com/stackabletech/postgresql-trino-gateway"
1010
keywords = ["postgresql", "trino", "gateway", "powerbi"]
1111

1212
[dependencies]
13-
pgwire = "0.38"
13+
pgwire = "0.39"
1414
tokio = { version = "1", features = ["full"] }
1515
async-trait = "0.1"
1616
futures = "0.3"
@@ -21,7 +21,6 @@ anyhow = "1"
2121
serde_json = "1"
2222
trino-rust-client = "0.9"
2323
async-stream = "0.3"
24-
dashmap = "6"
2524
sqlparser = { version = "0.61", features = ["visitor"] }
2625
rustls-pemfile = "2"
2726
rustls-pki-types = "1"

src/cancel.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,26 +26,22 @@ pub struct GatewayCancelHandler;
2626
#[async_trait]
2727
impl CancelHandler for GatewayCancelHandler {
2828
async fn on_cancel_request(&self, req: CancelRequest) {
29-
let entry = match session::lookup_cancel(req.pid, &req.secret_key) {
30-
Some(e) => e,
31-
None => {
32-
tracing::debug!(
33-
pid = req.pid,
34-
"CancelRequest for unknown (pid, secret_key) — ignored"
35-
);
36-
return;
37-
}
29+
let Some((trino_client, active_query_id)) =
30+
session::lookup_cancel(req.pid, &req.secret_key)
31+
else {
32+
tracing::debug!(
33+
pid = req.pid,
34+
"CancelRequest for unknown (pid, secret_key) — ignored"
35+
);
36+
return;
3837
};
3938

40-
// Snapshot the query id and trino-client handle, then drop the
41-
// DashMap ref guard before the async cancel call — guards aren't
42-
// Send and can't be held across awaits.
43-
let trino_client = std::sync::Arc::clone(&entry.trino_client);
44-
let query_id = match entry.active_query_id.lock() {
39+
// `lookup_cancel` already cloned out the bits we need; the registry
40+
// mutex was released inside it, so we're free to await below.
41+
let query_id = match active_query_id.lock() {
4542
Ok(g) => g.clone(),
4643
Err(p) => p.into_inner().clone(),
4744
};
48-
drop(entry);
4945

5046
let Some(qid) = query_id else {
5147
tracing::debug!(

src/main.rs

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// SPDX-FileCopyrightText: 2026 Stackable GmbH
22
// SPDX-License-Identifier: OSL-3.0
3-
use std::net::SocketAddr;
43
use std::sync::Arc;
54
use std::time::Duration;
65

@@ -15,7 +14,6 @@ use postgresql_trino_gateway::handler::GatewayHandlerFactory;
1514
use postgresql_trino_gateway::policy;
1615
use postgresql_trino_gateway::query_extended::GatewayExtendedQueryHandler;
1716
use postgresql_trino_gateway::query_simple::GatewayQueryHandler;
18-
use postgresql_trino_gateway::session;
1917
use postgresql_trino_gateway::startup::GatewayStartupHandler;
2018
use postgresql_trino_gateway::tls;
2119

@@ -31,16 +29,6 @@ const SHUTDOWN_DRAIN_TIMEOUT_ENV: &str = "GATEWAY_SHUTDOWN_DRAIN_TIMEOUT_SECS";
3129
/// unwinding before letting the process exit.
3230
const ABORT_UNWIND_GRACE: Duration = Duration::from_secs(1);
3331

34-
/// Drop guard that removes per-connection state when the connection task
35-
/// ends, including on panic or cancellation. Defined at module scope so the
36-
/// accept loop body stays readable.
37-
struct ConnectionCleanup(SocketAddr);
38-
impl Drop for ConnectionCleanup {
39-
fn drop(&mut self) {
40-
session::remove_connections_for_addr(self.0);
41-
}
42-
}
43-
4432
#[tokio::main]
4533
async fn main() -> anyhow::Result<()> {
4634
tracing_subscriber::fmt::init();
@@ -153,9 +141,8 @@ async fn main() -> anyhow::Result<()> {
153141
let tls = tls_acceptor.clone();
154142
tasks.spawn(async move {
155143
let _permit = permit; // released on task exit
156-
let _guard = ConnectionCleanup(peer_addr);
157144
if let Err(e) = pgwire::tokio::process_socket(socket, tls, factory).await {
158-
tracing::error!(error = %e, "connection error");
145+
tracing::error!(addr = %peer_addr, error = %e, "connection error");
159146
}
160147
});
161148
}

src/query_extended.rs

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use pgwire::error::{PgWireError, PgWireResult};
1515
use pgwire::messages::PgWireBackendMessage;
1616

1717
use crate::query_pipeline::process_query;
18-
use crate::session::{self, CachedPortalResponse, MAX_CACHED_PORTALS, PortalCache};
18+
use crate::session::{CachedPortalResponse, ConnectionState, MAX_CACHED_PORTALS, PortalCache};
1919

2020
/// Handles the extended query protocol (Parse/Bind/Describe/Execute).
2121
///
@@ -86,15 +86,11 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
8686
PgWireError: From<<C as Sink<PgWireBackendMessage>>::Error>,
8787
{
8888
let query = &portal.statement.statement;
89+
tracing::debug!(query, "Extended query execute");
8990

90-
let conn_id = client
91-
.metadata()
92-
.get(session::CONNECTION_ID_KEY)
93-
.ok_or_else(|| PgWireError::ApiError("No connection ID in metadata".into()))?
94-
.clone();
95-
tracing::debug!(conn_id = %conn_id, query, "Extended query execute");
96-
97-
let conn_state = session::get_connection(&conn_id)
91+
let conn_state = client
92+
.session_extensions()
93+
.get::<ConnectionState>()
9894
.ok_or_else(|| PgWireError::ApiError("Connection state not found".into()))?;
9995

10096
// Take any response stashed by do_describe_portal. If the cached
@@ -104,39 +100,36 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
104100
let cached_entry = take_cached(&conn_state.portals, &portal.name)?;
105101
if let Some(entry) = cached_entry {
106102
if entry.query == *query {
107-
tracing::trace!(conn_id = %conn_id, portal = %portal.name, "Extended query execute: served from describe cache");
103+
tracing::trace!(portal = %portal.name, "Extended query execute: served from describe cache");
108104
return Ok(entry.response);
109105
}
110106
tracing::debug!(
111-
conn_id = %conn_id,
112107
portal = %portal.name,
113108
"Discarding stale describe-cache entry — portal was re-bound"
114109
);
115110
}
116111

117-
let trino_client = Arc::clone(&conn_state.trino_client);
118-
let config = Arc::clone(&conn_state.config);
119-
let active_query_id = Arc::clone(&conn_state.active_query_id);
120-
drop(conn_state);
121-
122-
let responses =
123-
process_query(query, &trino_client, &config, Some(&active_query_id)).await?;
112+
let responses = process_query(
113+
query,
114+
&conn_state.trino_client,
115+
&conn_state.config,
116+
Some(&conn_state.active_query_id),
117+
)
118+
.await?;
124119
let response = responses
125120
.into_iter()
126121
.next()
127122
.ok_or_else(|| PgWireError::ApiError("Empty pipeline response".into()))?;
128123
match &response {
129124
Response::Query(qr) => {
130125
tracing::trace!(
131-
conn_id = %conn_id,
132126
portal = %portal.name,
133127
columns = qr.row_schema.len(),
134128
"Extended query execute: query response (no cache)"
135129
);
136130
}
137131
Response::Execution(_tag) => {
138132
tracing::trace!(
139-
conn_id = %conn_id,
140133
portal = %portal.name,
141134
"Extended query execute: execution response (no cache)"
142135
);
@@ -185,25 +178,20 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
185178
// Run the pipeline once, return the schema, and stash the response so
186179
// do_query can serve Execute without re-running.
187180
let query = &portal.statement.statement;
181+
tracing::debug!(query, "Extended query describe portal");
188182

189-
let conn_id = client
190-
.metadata()
191-
.get(session::CONNECTION_ID_KEY)
192-
.ok_or_else(|| PgWireError::ApiError("No connection ID in metadata".into()))?
193-
.clone();
194-
tracing::debug!(conn_id = %conn_id, query, "Extended query describe portal");
195-
let conn_state = session::get_connection(&conn_id)
183+
let conn_state = client
184+
.session_extensions()
185+
.get::<ConnectionState>()
196186
.ok_or_else(|| PgWireError::ApiError("Connection state not found".into()))?;
197-
let trino_client = Arc::clone(&conn_state.trino_client);
198-
let config = Arc::clone(&conn_state.config);
199-
// Clone the Arcs so we can use them after the .await below;
200-
// conn_state is a DashMap ref-guard that can't be held across awaits.
201-
let portals = Arc::clone(&conn_state.portals);
202-
let active_query_id = Arc::clone(&conn_state.active_query_id);
203-
drop(conn_state);
204187

205-
let responses =
206-
process_query(query, &trino_client, &config, Some(&active_query_id)).await?;
188+
let responses = process_query(
189+
query,
190+
&conn_state.trino_client,
191+
&conn_state.config,
192+
Some(&conn_state.active_query_id),
193+
)
194+
.await?;
207195
let response = responses
208196
.into_iter()
209197
.next()
@@ -228,7 +216,7 @@ impl ExtendedQueryHandler for GatewayExtendedQueryHandler {
228216
// Trino's `DELETE /v1/statement/{queryId}` so abandoned queries
229217
// release Trino resources promptly.
230218
insert_cached(
231-
&portals,
219+
&conn_state.portals,
232220
portal.name.clone(),
233221
CachedPortalResponse {
234222
query: query.clone(),

src/query_simple.rs

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,17 @@
11
// SPDX-FileCopyrightText: 2026 Stackable GmbH
22
// SPDX-License-Identifier: OSL-3.0
33
use std::fmt::Debug;
4-
use std::sync::Arc;
54

65
use async_trait::async_trait;
76
use futures::Sink;
7+
use pgwire::api::ClientInfo;
8+
use pgwire::api::query::SimpleQueryHandler;
89
use pgwire::api::results::Response;
9-
use pgwire::api::{ClientInfo, ClientPortalStore};
1010
use pgwire::error::{PgWireError, PgWireResult};
1111
use pgwire::messages::PgWireBackendMessage;
1212

13-
use pgwire::api::query::SimpleQueryHandler;
14-
1513
use crate::query_pipeline::process_query;
16-
use crate::session;
14+
use crate::session::ConnectionState;
1715

1816
#[derive(Debug)]
1917
pub struct GatewayQueryHandler;
@@ -22,37 +20,29 @@ pub struct GatewayQueryHandler;
2220
impl SimpleQueryHandler for GatewayQueryHandler {
2321
async fn do_query<C>(&self, client: &mut C, query: &str) -> PgWireResult<Vec<Response>>
2422
where
25-
C: ClientInfo + ClientPortalStore + Sink<PgWireBackendMessage> + Unpin + Send + Sync,
23+
C: ClientInfo + Sink<PgWireBackendMessage> + Unpin + Send + Sync,
2624
C::Error: Debug,
2725
PgWireError: From<<C as Sink<PgWireBackendMessage>>::Error>,
2826
{
2927
tracing::debug!(query, "Simple query received");
3028

31-
let conn_id = client
32-
.metadata()
33-
.get(session::CONNECTION_ID_KEY)
34-
.ok_or_else(|| PgWireError::ApiError("No connection ID in metadata".into()))?
35-
.clone();
36-
let conn_state = session::get_connection(&conn_id)
29+
let conn_state = client
30+
.session_extensions()
31+
.get::<ConnectionState>()
3732
.ok_or_else(|| PgWireError::ApiError("Connection state not found".into()))?;
38-
let trino_client = Arc::clone(&conn_state.trino_client);
39-
let config = Arc::clone(&conn_state.config);
40-
let active_query_id = Arc::clone(&conn_state.active_query_id);
41-
// `conn_state` is a `dashmap::Ref` guard holding a read-lock on its
42-
// shard; the guard is not `Send`, so it cannot be held across the
43-
// `process_query(...).await` below. Drop it explicitly after we've
44-
// cloned out the Arcs we need; otherwise the borrow-checker rejects
45-
// the await.
46-
drop(conn_state);
4733

48-
let result = process_query(query, &trino_client, &config, Some(&active_query_id)).await;
34+
let result = process_query(
35+
query,
36+
&conn_state.trino_client,
37+
&conn_state.config,
38+
Some(&conn_state.active_query_id),
39+
)
40+
.await;
4941
match &result {
50-
Ok(responses) => tracing::trace!(
51-
conn_id = %conn_id,
52-
response_count = responses.len(),
53-
"Simple query processed"
54-
),
55-
Err(e) => tracing::debug!(conn_id = %conn_id, error = %e, "Simple query failed"),
42+
Ok(responses) => {
43+
tracing::trace!(response_count = responses.len(), "Simple query processed")
44+
}
45+
Err(e) => tracing::debug!(error = %e, "Simple query failed"),
5646
}
5747
result
5848
}

0 commit comments

Comments
 (0)