Skip to content

Add trino_client crate: typed Settings, parameterized SQL via EXECUTE IMMEDIATE#1192

Merged
bbalser merged 9 commits into
mainfrom
bbalser/pedantic-babbage-46b8fb
May 19, 2026
Merged

Add trino_client crate: typed Settings, parameterized SQL via EXECUTE IMMEDIATE#1192
bbalser merged 9 commits into
mainfrom
bbalser/pedantic-babbage-46b8fb

Conversation

@bbalser
Copy link
Copy Markdown
Collaborator

@bbalser bbalser commented May 18, 2026

New workspace crate at trino_client/ that wraps trino-rust-client with:

  • Settings: typed, serde-deserializable config — host, port, user, catalog, schema, secure, ca_cert_path, insecure_skip_tls_verify, plus auth: Option<AuthSettings> with Basic { username, password } and Jwt { token } variants.
  • Statement: parameterized SQL builder — Statement::new(sql).bind(name, value).render() -> Result<String>. Named placeholders (:id, :name) render to Trino's EXECUTE IMMEDIATE 'sql with ?' USING v1, v2, ... form. Scanner is quote- and comment-aware (skips : inside '...', "...", -- ..., /* ... */).
  • Param enum with From impls for: i32, i64, u32, u64, f32, f64, bool, &str, String, Vec<u8>, &[u8], chrono::NaiveDate, chrono::NaiveDateTime, chrono::DateTime<Utc>, and a blanket Option<T: Into<Param>> that renders None as bare NULL. Bytes render as Trino VARBINARY hex literals (X'deadbeef'). u64 > i64::MAX errors at render time.
  • SqlStatement / SqlQuery traits: a struct implements SqlStatement (returns a Statement); add SqlQuery: SqlStatement with type Row for typed row-returning queries. Client::execute(&q) and Client::get_all(&q) dispatch through these.
  • Client: owns the underlying trino_rust_client::Client. Exposes from_settings, from_inner, inner, trait-based execute/get_all, plus raw execute_raw / get_all_raw for ad-hoc SQL (and for DDL, since Trino's EXECUTE IMMEDIATE rejects DDL).
  • Re-exports trino_rust_client so downstream callers using #[derive(Trino)] and error::Error::EmptyData only need one dep.

Migration of existing callers (helium_iceberg, mobile_verifier, mobile_packet_verifier) is intentionally out of scope for this PR — most of those sites interpolate identifiers ({NAMESPACE}.{TABLE_NAME}) which the parameterized API can't bind, so migration is a separate, mechanical follow-up.

Test plan

54 tests pass (44 unit + 10 integration):

  • cargo build -p trino_client --all-targets
  • cargo clippy -p trino_client --all-targets -- -D warnings
  • Unit tests cover: literal rendering for every Param variant, quote-/comment-aware scanner edge cases ('', "", : inside strings/identifiers/comments), missing-param / no-param / NaN / Inf / u64-overflow errors, Option blanket, Settings serde deserialization for basic auth, JWT auth, TLS options.
  • Integration tests against local Trino (localhost:8080, user admin): scalar round-trip, date round-trip, naive-/UTC-timestamp equality vs TIMESTAMP literals, apostrophe escaping round-trip, SqlStatement::execute path, SqlQuery::get_all path with typed Row, Option<None>IS NULL, Option<Some> → value, u64BIGINT, Vec<u8>X'...' VARBINARY.

@bbalser bbalser requested review from macpie and michaeldjeffrey May 18, 2026 11:43
Comment on lines +11 to +12
schema: Option<String>,
secure: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having scheme and secure seems a bit redundant.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is schema not scheme

Comment thread trino_client/src/builder.rs Outdated
catalog: Option<String>,
schema: Option<String>,
secure: bool,
ca_cert_path: Option<PathBuf>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use, or are we planning on using ca_certs?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we do, removed 91b745d

@bbalser bbalser merged commit 0e3dade into main May 19, 2026
30 checks passed
@bbalser bbalser deleted the bbalser/pedantic-babbage-46b8fb branch May 19, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants