Skip to content

Commit 022a073

Browse files
committed
refactor: extract establish_session, djb2_oid, json_str helpers
Three small dedup passes that came out of the human review: - `startup.rs`: the auth=true and auth=false branches both ran the same post-Trino-client session setup (allocate pid+secret, register cancel, register connection, finish_authentication). Pull the shared tail into `establish_session(client, trino_client, user)`. The one branch difference (logging "client connected" vs "client connected (no auth)") is captured by the `Option<&str>` user parameter. - `catalog/pg_class.rs`: `table_oid` and `namespace_oid` both used djb2 inline. Extract `djb2_oid(parts: &[&str], base: u32) -> u32` and call from both. Doc-comment notes why we use djb2 rather than `std::hash::DefaultHasher` (DefaultHasher's algorithm is allowed to change across Rust releases; OIDs need to be stable across reconnects). - `catalog/mod.rs`: introduce `json_str(values, idx, default)` helper, used from both `pg_class::respond_pg_class` and `pg_attribute::respond_pg_attribute` to keep row-decode boilerplate to one line per column. No behavioural change. All 191 unit tests still pass; clippy clean under `--all-targets -- -D warnings`. License headers migrated to inline SPDX along the way.
1 parent de3d9e0 commit 022a073

3 files changed

Lines changed: 114 additions & 100 deletions

File tree

src/catalog/pg_attribute.rs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
// Copyright 2026 Stackable GmbH
2-
// Licensed under the Open Software License version 3.0 (OSL-3.0).
3-
// See LICENSE file in the project root for full license text.
1+
// SPDX-FileCopyrightText: 2026 Stackable GmbH
2+
// SPDX-License-Identifier: OSL-3.0
43
use std::sync::Arc;
54

65
use pgwire::api::Type;
@@ -9,7 +8,7 @@ use pgwire::error::{PgWireError, PgWireResult};
98
use trino_rust_client::{Client, Row};
109

1110
use super::pg_class::table_oid;
12-
use super::{build_response, text_field};
11+
use super::{build_response, json_str, text_field};
1312
use crate::types::trino_type_to_pg;
1413

1514
/// Query Trino's information_schema.columns and return a pg_attribute-compatible response.
@@ -41,18 +40,21 @@ pub async fn respond_pg_attribute(client: &Arc<Client>) -> PgWireResult<Vec<Resp
4140
let values = trino_row.into_json();
4241
// columns: table_schema (0), table_name (1), column_name (2),
4342
// ordinal_position (3), is_nullable (4), data_type (5)
44-
let schema_name = values.first().and_then(|v| v.as_str()).unwrap_or("public");
45-
let tbl_name = values.get(1).and_then(|v| v.as_str()).unwrap_or("");
46-
let col_name = values.get(2).and_then(|v| v.as_str()).unwrap_or("");
43+
let schema_name = json_str(&values, 0, "public");
44+
let tbl_name = json_str(&values, 1, "");
45+
let col_name = json_str(&values, 2, "");
46+
// Trino reports ordinal_position as a JSON number, but some
47+
// information_schema bridges (e.g. legacy connectors) stringify it.
48+
// Accept either form.
4749
let ordinal = values
4850
.get(3)
4951
.and_then(|v| {
5052
v.as_i64()
5153
.or_else(|| v.as_str().and_then(|s| s.parse().ok()))
5254
})
5355
.unwrap_or(0);
54-
let is_nullable = values.get(4).and_then(|v| v.as_str()).unwrap_or("YES");
55-
let data_type = values.get(5).and_then(|v| v.as_str()).unwrap_or("varchar");
56+
let is_nullable = json_str(&values, 4, "YES");
57+
let data_type = json_str(&values, 5, "varchar");
5658

5759
let attrelid = table_oid(schema_name, tbl_name);
5860
let pg_type = trino_type_to_pg(data_type);

src/catalog/pg_class.rs

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
1-
// Copyright 2026 Stackable GmbH
2-
// Licensed under the Open Software License version 3.0 (OSL-3.0).
3-
// See LICENSE file in the project root for full license text.
1+
// SPDX-FileCopyrightText: 2026 Stackable GmbH
2+
// SPDX-License-Identifier: OSL-3.0
43
use std::sync::Arc;
54

65
use pgwire::api::Type;
76
use pgwire::api::results::Response;
87
use pgwire::error::{PgWireError, PgWireResult};
98
use trino_rust_client::{Client, Row};
109

11-
use super::{build_response, text_field};
10+
use super::{build_response, json_str, text_field};
1211

1312
/// Starting OID for synthetic table entries (above PostgreSQL's system range).
1413
const BASE_TABLE_OID: u32 = 16384;
@@ -19,37 +18,47 @@ const PUBLIC_NAMESPACE_OID: u32 = 2200;
1918
/// Starting OID for synthetic namespace entries (schemas other than "public").
2019
const BASE_NAMESPACE_OID: u32 = 20000;
2120

21+
/// djb2 hash, then map into `[base, i32::MAX)`. Used to derive synthetic
22+
/// OIDs from string names.
23+
///
24+
/// We need a deterministic hash that produces the same OID every time the
25+
/// gateway sees the same name — clients that reconnect must see stable
26+
/// OIDs in pg_class so they can resolve them in pg_attribute. Rust's
27+
/// `std::collections::hash_map::DefaultHasher` is *not* stability-
28+
/// guaranteed across compiler releases (the standard library is allowed
29+
/// to swap the algorithm), so we use the well-known djb2 hash which is
30+
/// fixed and trivial.
31+
fn djb2_oid(parts: &[&str], base: u32) -> u32 {
32+
let mut h: u32 = 5381;
33+
for (i, part) in parts.iter().enumerate() {
34+
if i > 0 {
35+
// Separator byte to avoid collisions between e.g. "abctable"
36+
// and "ab" + "ctable".
37+
h = h.wrapping_mul(33).wrapping_add(0xFF);
38+
}
39+
for b in part.bytes() {
40+
h = h.wrapping_mul(33).wrapping_add(u32::from(b));
41+
}
42+
}
43+
base + (h % (i32::MAX as u32 - base))
44+
}
45+
2246
/// Compute a deterministic OID for a table given its schema and table name.
2347
///
24-
/// Uses the same algorithm as `table_oid`, so pg_attribute can reference the
25-
/// same OID for attrelid.
48+
/// Uses the same hash as `namespace_oid`, so pg_attribute can reference
49+
/// the same OID for `attrelid`.
2650
pub fn table_oid(schema_name: &str, table_name: &str) -> u32 {
27-
// Simple deterministic hash: we want stable OIDs across pg_class and
28-
// pg_attribute calls within the same session. Using a basic hash that
29-
// fits in u32 and is offset from BASE_TABLE_OID.
30-
let mut h: u32 = 5381;
31-
for b in schema_name.bytes() {
32-
h = h.wrapping_mul(33).wrapping_add(u32::from(b));
33-
}
34-
// Separator to avoid collisions between "abctable" and "ab" + "ctable"
35-
h = h.wrapping_mul(33).wrapping_add(0xFF);
36-
for b in table_name.bytes() {
37-
h = h.wrapping_mul(33).wrapping_add(u32::from(b));
38-
}
39-
// Ensure it's at least BASE_TABLE_OID and positive (i32 range)
40-
BASE_TABLE_OID + (h % (i32::MAX as u32 - BASE_TABLE_OID))
51+
djb2_oid(&[schema_name, table_name], BASE_TABLE_OID)
4152
}
4253

43-
/// Compute a deterministic namespace OID for a schema name.
54+
/// Compute a deterministic namespace OID for a schema name. The "public"
55+
/// schema gets the well-known PostgreSQL OID (2200); everything else is
56+
/// hashed.
4457
pub fn namespace_oid(schema_name: &str) -> u32 {
4558
if schema_name == "public" {
4659
return PUBLIC_NAMESPACE_OID;
4760
}
48-
let mut h: u32 = 5381;
49-
for b in schema_name.bytes() {
50-
h = h.wrapping_mul(33).wrapping_add(u32::from(b));
51-
}
52-
BASE_NAMESPACE_OID + (h % (i32::MAX as u32 - BASE_NAMESPACE_OID))
61+
djb2_oid(&[schema_name], BASE_NAMESPACE_OID)
5362
}
5463

5564
/// Map Trino's table_type string to PostgreSQL's relkind char.
@@ -85,12 +94,9 @@ pub async fn respond_pg_class(client: &Arc<Client>) -> PgWireResult<Vec<Response
8594
for trino_row in dataset.into_vec() {
8695
let values = trino_row.into_json();
8796
// columns: table_schema (0), table_name (1), table_type (2)
88-
let schema_name = values.first().and_then(|v| v.as_str()).unwrap_or("public");
89-
let tbl_name = values.get(1).and_then(|v| v.as_str()).unwrap_or("");
90-
let tbl_type = values
91-
.get(2)
92-
.and_then(|v| v.as_str())
93-
.unwrap_or("BASE TABLE");
97+
let schema_name = json_str(&values, 0, "public");
98+
let tbl_name = json_str(&values, 1, "");
99+
let tbl_type = json_str(&values, 2, "BASE TABLE");
94100

95101
let oid = table_oid(schema_name, tbl_name);
96102
let ns_oid = namespace_oid(schema_name);

src/startup.rs

Lines changed: 65 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
// Copyright 2026 Stackable GmbH
2-
// Licensed under the Open Software License version 3.0 (OSL-3.0).
3-
// See LICENSE file in the project root for full license text.
1+
// SPDX-FileCopyrightText: 2026 Stackable GmbH
2+
// SPDX-License-Identifier: OSL-3.0
43
use std::collections::HashMap;
54
use std::fmt::Debug;
65
use std::sync::Arc;
@@ -16,7 +15,6 @@ use pgwire::api::{ClientInfo, PgWireConnectionState};
1615
use pgwire::error::{ErrorInfo, PgWireError, PgWireResult};
1716
use pgwire::messages::startup::{Authentication, SecretKey};
1817
use pgwire::messages::{PgWireBackendMessage, PgWireFrontendMessage};
19-
use rand::Rng;
2018
use trino_rust_client::auth::Auth;
2119

2220
use crate::config::Config;
@@ -32,7 +30,7 @@ static CONNECTION_PID: AtomicI32 = AtomicI32::new(1);
3230
/// authorisation token for somebody else's connection.
3331
fn new_pid_and_secret_key() -> (i32, SecretKey) {
3432
let pid = CONNECTION_PID.fetch_add(1, Ordering::Relaxed);
35-
let secret = rand::thread_rng().r#gen::<i32>();
33+
let secret = rand::random::<i32>();
3634
(pid, SecretKey::I32(secret))
3735
}
3836

@@ -142,31 +140,9 @@ impl StartupHandler for GatewayStartupHandler {
142140
))
143141
.await?;
144142
} else {
145-
// No auth — create Trino client immediately
143+
// No auth — create Trino client immediately and finish.
146144
let trino_client = Arc::new(self.build_trino_client(None, None)?);
147-
let (pid, secret_key) = new_pid_and_secret_key();
148-
client.set_pid_and_secret_key(pid, secret_key.clone());
149-
let active_query_id = session::register_cancel(
150-
pid,
151-
secret_key.clone(),
152-
Arc::clone(&trino_client),
153-
);
154-
let conn_id = format!("{}_{}", client.socket_addr(), pid);
155-
client
156-
.metadata_mut()
157-
.insert(session::CONNECTION_ID_KEY.to_owned(), conn_id.clone());
158-
session::register_connection(
159-
conn_id,
160-
ConnectionState {
161-
trino_client,
162-
config: self.config.clone(),
163-
portals: Default::default(),
164-
active_query_id,
165-
cancel_key: (pid, secret_key),
166-
},
167-
);
168-
finish_authentication(client, &GatewayParameterProvider).await?;
169-
tracing::info!(addr = %client.socket_addr(), "client connected (no auth)");
145+
self.establish_session(client, trino_client, None).await?;
170146
}
171147
}
172148
PgWireFrontendMessage::PasswordMessageFamily(pwd) => {
@@ -177,50 +153,35 @@ impl StartupHandler for GatewayStartupHandler {
177153
let password = pwd.into_password()?;
178154
let user = client.metadata().get("user").cloned().unwrap_or_default();
179155

180-
// Build Trino client with the PG client's credentials
156+
// Build Trino client with the PG client's credentials.
181157
let trino_client =
182158
self.build_trino_client(Some(&user), Some(&password.password))?;
183159

184-
// Validate credentials by running a lightweight query against Trino.
185-
// If Trino rejects the auth, we reject the PG connection immediately
186-
// rather than letting the first real query fail with a confusing error.
160+
// Validate credentials by running a lightweight query against
161+
// Trino. We reject the PG connection here on any failure
162+
// (auth rejection, network error, malformed reply, ...) so
163+
// the client gets a clear failure during startup rather than
164+
// a confusing error on its first real query. The error
165+
// message is logged at the gateway with the underlying
166+
// cause; the client gets the standard `InvalidPassword`
167+
// response — distinguishing auth-failure from other failures
168+
// requires inspecting `trino-rust-client`'s error enum and
169+
// is tracked separately.
187170
if let Err(e) = trino_client
188171
.get::<trino_rust_client::Row>("SELECT 1".to_owned())
189172
.await
190173
{
191-
let msg = e.to_string();
192174
tracing::warn!(
193175
addr = %client.socket_addr(),
194176
user = %user,
195-
"Trino authentication failed: {msg}"
177+
error = %e,
178+
"rejecting PG connection — Trino credential check failed (auth rejection, network error, or unreachable Trino)"
196179
);
197180
return Err(PgWireError::InvalidPassword(user));
198181
}
199182

200-
let trino_client = Arc::new(trino_client);
201-
let (pid, secret_key) = new_pid_and_secret_key();
202-
client.set_pid_and_secret_key(pid, secret_key.clone());
203-
let active_query_id = session::register_cancel(
204-
pid,
205-
secret_key.clone(),
206-
Arc::clone(&trino_client),
207-
);
208-
let conn_id = format!("{}_{}", client.socket_addr(), pid);
209-
client
210-
.metadata_mut()
211-
.insert(session::CONNECTION_ID_KEY.to_owned(), conn_id.clone());
212-
session::register_connection(
213-
conn_id,
214-
ConnectionState {
215-
trino_client,
216-
config: self.config.clone(),
217-
portals: Default::default(),
218-
active_query_id,
219-
cancel_key: (pid, secret_key),
220-
},
221-
);
222-
finish_authentication(client, &GatewayParameterProvider).await?;
223-
tracing::info!(addr = %client.socket_addr(), user = %user, "client connected");
183+
self.establish_session(client, Arc::new(trino_client), Some(user.as_str()))
184+
.await?;
224185
}
225186
_ => {
226187
return Err(PgWireError::ApiError(
@@ -241,6 +202,51 @@ impl GatewayStartupHandler {
241202
self.config.tls_cert.is_some() && self.config.tls_key.is_some()
242203
}
243204

205+
/// Common per-connection setup once the Trino client has been built
206+
/// and (for `--auth=true`) credentials have been verified. Allocates a
207+
/// fresh `(pid, secret_key)`, registers the connection in the cancel
208+
/// registry and the per-connection state map, and finishes the PG
209+
/// startup handshake. `user` is `Some` only on the auth=true path,
210+
/// where it's logged for operator visibility.
211+
async fn establish_session<C>(
212+
&self,
213+
client: &mut C,
214+
trino_client: Arc<trino_rust_client::Client>,
215+
user: Option<&str>,
216+
) -> PgWireResult<()>
217+
where
218+
C: ClientInfo + Sink<PgWireBackendMessage> + Unpin + Send + Sync,
219+
C::Error: Debug,
220+
PgWireError: From<<C as Sink<PgWireBackendMessage>>::Error>,
221+
{
222+
let (pid, secret_key) = new_pid_and_secret_key();
223+
client.set_pid_and_secret_key(pid, secret_key.clone());
224+
let active_query_id =
225+
session::register_cancel(pid, secret_key.clone(), Arc::clone(&trino_client));
226+
let conn_id = format!("{}_{}", client.socket_addr(), pid);
227+
client
228+
.metadata_mut()
229+
.insert(session::CONNECTION_ID_KEY.to_owned(), conn_id.clone());
230+
session::register_connection(
231+
conn_id,
232+
ConnectionState {
233+
trino_client,
234+
config: self.config.clone(),
235+
portals: Default::default(),
236+
active_query_id,
237+
cancel_key: (pid, secret_key),
238+
},
239+
);
240+
finish_authentication(client, &GatewayParameterProvider).await?;
241+
match user {
242+
Some(u) => {
243+
tracing::info!(addr = %client.socket_addr(), user = %u, "client connected")
244+
}
245+
None => tracing::info!(addr = %client.socket_addr(), "client connected (no auth)"),
246+
}
247+
Ok(())
248+
}
249+
244250
/// Build a Trino client, optionally with Basic auth credentials from the PG client.
245251
fn build_trino_client(
246252
&self,

0 commit comments

Comments
 (0)