Skip to content

feat: add native Trino driver#795

Open
cerebrixos wants to merge 1 commit intoAltimateAI:mainfrom
cerebrixos:codex/add-trino-driver
Open

feat: add native Trino driver#795
cerebrixos wants to merge 1 commit intoAltimateAI:mainfrom
cerebrixos:codex/add-trino-driver

Conversation

@cerebrixos
Copy link
Copy Markdown

@cerebrixos cerebrixos commented May 7, 2026

Summary

  • Add a native Trino driver using trino-client.
  • Wire Trino into connection normalization, registry loading, dbt profile parsing, Docker discovery, environment discovery, SQL explain, and warehouse add guidance.
  • Document Trino configuration, authentication, installation, and support matrix entries.

Details

The previous dbt profile mapping treated trino as PostgreSQL-compatible. This adds a first-class Trino connector instead, with support for:

  • HTTP and HTTPS server URLs
  • catalog and schema defaults
  • Basic authentication
  • bearer token authentication
  • extra headers, session properties, and extra credentials
  • schema/table/column introspection through catalog-scoped information_schema
  • SELECT limit injection with truncation detection

Tests

  • cd packages/drivers && bun test test/trino-unit.test.ts
  • cd packages/opencode && bun test test/altimate/driver-normalize.test.ts test/altimate/connections.test.ts test/tool/project-scan.test.ts test/altimate/tools/sql-explain.test.ts
  • cd packages/opencode && bun run typecheck

Note: bunx tsc -p packages/drivers/tsconfig.json --noEmit still fails on the existing bun:sqlite type resolution issue in packages/drivers/src/sqlite.ts, unrelated to this Trino change.


Summary by cubic

Adds a native Trino driver and wires it into discovery, dbt profiles, SQL explain, and docs. This delivers first-class Trino support with catalog/schema introspection and HTTP(S) auth.

  • New Features

    • Native Trino connector using trino-client with HTTP/HTTPS, Basic and Bearer auth, extra headers/session/credentials, and catalog/schema defaults.
    • Introspection via catalog-scoped information_schema; SELECT limit injection with truncation detection.
    • Integration: config normalization aliases (e.g., server → connection_string, database/dbname → catalog, token → access_token); dbt trino now maps to the native driver; Docker discovery recognizes Trino images; env var detection for TRINO_* and DATABASE_URL (trino/presto); EXPLAIN/EXPLAIN ANALYZE supported; query history disabled for Trino by design.
    • Docs: added Trino configuration/auth examples, support matrix updates, and warehouse discovery notes; site now lists 13 supported warehouses including Trino.
  • Dependencies

    • Added trino-client (optional peer dependency) to @altimateai/drivers.

Written for commit 590a5d4. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Trino as a supported data warehouse with full connectivity and query execution.
    • Enabled automatic Trino detection via environment variables and Docker containers.
    • Integrated dbt Trino profiles for seamless project configuration.
  • Documentation

    • Updated warehouse configuration, driver support, and tools documentation to include Trino setup and authentication details.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds comprehensive Trino warehouse support to Altimate Code. The implementation includes a native TypeScript driver using trino-client, configuration discovery mechanisms (dbt profiles, Docker containers, environment variables), integration with finops and warehouse tooling, and full user documentation.

Changes

Trino Warehouse Driver Integration

Layer / File(s) Summary
Core Driver Implementation
packages/drivers/src/trino.ts
Implements Trino connector: connect() sets up server/catalog/schema/headers/auth, execute() injects LIMIT and streams paginated results, listSchemas() queries catalogs, listTables() and describeTable() query information_schema, close() cleans up.
Driver Registration & Exports
packages/drivers/src/index.ts, packages/opencode/src/altimate/native/connections/registry.ts
Exports connectTrino function; registers trino type in driver map; validates password as string; adds dynamic import for @altimateai/drivers/trino in connector factory.
Config Alias Normalization
packages/drivers/src/normalize.ts
Defines TRINO_ALIASES mapping (dbname→catalog, database→catalog, token→access_token, server→connection_string) and registers in DRIVER_ALIASES for config normalization.
dbt Profile Discovery
packages/opencode/src/altimate/native/connections/dbt-profiles.ts
Maps dbt trino adapter to native trino type; adds resolveEnvVars() to expand dbt Jinja {{ env_var(...) }} expressions; implements multi-location profile path resolution (explicit, DBT_PROFILES_DIR, project-local, ~/.dbt/profiles.yml).
Docker & Environment Discovery
packages/opencode/src/altimate/native/connections/docker-discovery.ts, packages/opencode/src/altimate/tools/project-scan.ts
Detects Trino via Docker image patterns, TRINO_* environment variables, and DATABASE_URL schemes (trino, trino+http, trino+https, presto); sets default port 8080 and user trino.
Feature Integration
packages/opencode/src/altimate/native/connections/register.ts, packages/opencode/src/altimate/native/finops/query-history.ts, packages/opencode/src/altimate/tools/warehouse-add.ts
Adds ExplainPlan interface; wires Trino to use EXPLAIN ANALYZE; marks Trino with no durable query history (live state only); extends warehouse-add post-connection suggestions; updates error message to include mongodb.
Package Dependencies
packages/drivers/package.json, packages/opencode/script/publish.ts
Adds trino-client@^0.2.8 as optional dependency and registers >=0.2 in peer dependencies for publishing.
Tests
packages/drivers/test/trino-unit.test.ts, packages/opencode/test/altimate/connections.test.ts, packages/opencode/test/altimate/driver-normalize.test.ts, packages/opencode/test/altimate/tools/sql-explain.test.ts, packages/opencode/test/tool/project-scan.test.ts
Unit tests for driver config/headers/BasicAuth, LIMIT injection/truncation, table/column introspection; tests for dbt adapter mapping, alias normalization, EXPLAIN dialect behavior, environment variable and URL detection.
Documentation
README.md, docs/docs/configure/warehouses.md, docs/docs/drivers.md, docs/docs/data-engineering/tools/warehouse-tools.md, docs/docs/getting-started/index.md
Adds Trino to supported warehouses (13 total); documents connection config fields, authentication methods, default port, and protocol; documents environment variable detection and Docker auto-discovery; updates marketing materials.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

  • AltimateAI/altimate-code#739: Both PRs modify the finops query-history implementation (buildHistoryQuery); this PR adds Trino-specific handling (returns null).
  • AltimateAI/altimate-code#574: Both PRs follow the same warehouse driver integration pattern, modifying drivers/index, normalize.ts, registry, discovery, and publish script.
  • AltimateAI/altimate-code#705: Both PRs extend warehouse driver support by updating driver registry, alias maps, and connection discovery mechanisms in parallel ways.

Suggested labels

contributor

Poem

🐰 A rabbit hops through Trino's door,
Where catalogs and schemas soar,
With LIMIT clauses, safe and sound,
This warehouse driver's homeward-bound! 🏡

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is missing the required 'PINEAPPLE' identifier at the very top, as mandated by the repository template for all AI-generated contributions. Add the word 'PINEAPPLE' at the very beginning of the PR description before any other content, as required by the repository template for AI-generated contributions.
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add native Trino driver' clearly and concisely describes the main change—implementing a first-class Trino database driver.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 23 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/docs/data-engineering/tools/warehouse-tools.md">

<violation number="1" location="docs/docs/data-engineering/tools/warehouse-tools.md:57">
P3: Docker discovery is documented inconsistently across sections; the summary table drops ClickHouse/MongoDB while the command docs still list them.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@@ -54,7 +54,7 @@ env_bigquery | bigquery | GOOGLE_APPLICATION_CREDENTIALS
| **dbt project** | Walks up directories for `dbt_project.yml`, reads name/profile |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Docker discovery is documented inconsistently across sections; the summary table drops ClickHouse/MongoDB while the command docs still list them.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/docs/data-engineering/tools/warehouse-tools.md, line 57:

<comment>Docker discovery is documented inconsistently across sections; the summary table drops ClickHouse/MongoDB while the command docs still list them.</comment>

<file context>
@@ -54,7 +54,7 @@ env_bigquery  | bigquery | GOOGLE_APPLICATION_CREDENTIALS
 | **dbt manifest** | Parses `target/manifest.json` for model/source/test counts |
 | **dbt profiles** | Searches for `profiles.yml`: `DBT_PROFILES_DIR` env var → project root → `<home>/.dbt/profiles.yml` |
-| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL containers |
+| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL/Trino containers |
 | **Existing connections** | Bridge call to list already-configured warehouses |
 | **Environment variables** | Scans `process.env` for warehouse signals (see table below) |
</file context>
Suggested change
| **dbt project** | Walks up directories for `dbt_project.yml`, reads name/profile |
| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL/Trino/ClickHouse/MongoDB containers |

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/src/altimate/native/connections/dbt-profiles.ts (1)

56-64: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

env_var regex only matches single-quoted args; dbt also accepts double quotes.

Both {{ env_var('DBT_USER') }} and {{ env_var("DBT_USER") }} are valid in dbt's Jinja, and double-quoted forms are very common in real profiles. The current regex only matches single quotes, so any profile using double quotes will fall through unresolved (and the literal {{ env_var("…") }} ends up as the field value).

🛠️ Suggested fix — accept either quote style for both name and default
 function resolveEnvVars(value: unknown): unknown {
   if (typeof value !== "string") return value
   return value.replace(
-    /\{\{\s*env_var\s*\(\s*'([^']+)'\s*(?:,\s*'([^']*)'\s*)?\)\s*\}\}/g,
-    (_match, envName: string, defaultValue?: string) => {
-      return process.env[envName] ?? defaultValue ?? ""
+    /\{\{\s*env_var\s*\(\s*(['"])([^'"]+)\1\s*(?:,\s*(['"])([^'"]*)\3\s*)?\)\s*\}\}/g,
+    (_match, _q1, envName: string, _q2, defaultValue?: string) => {
+      return process.env[envName] ?? defaultValue ?? ""
     },
   )
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/altimate/native/connections/dbt-profiles.ts` around
lines 56 - 64, The resolveEnvVars function's regex only matches single-quoted
args so double-quoted env_var forms are left unresolved; update the regex in
resolveEnvVars to accept either single or double quotes (use a capture for the
opening quote like (['"]) and a backreference \1 for the closing quote) for both
the env name and the optional default, then use the captured groups for envName
and defaultValue in the replacement so both {{ env_var('NAME') }}, {{
env_var("NAME") }}, and defaults with either quote style are handled.
🧹 Nitpick comments (2)
packages/opencode/test/tool/project-scan.test.ts (1)

567-602: 💤 Low value

LGTM — Trino env-var and DATABASE_URL detection tests look correct.

The TRINO_HOST test covers field mapping including catalog, schema, and password masking. The DATABASE_URL scheme loop correctly asserts all four Trino/Presto URL schemes map to type === "trino".

One minor note: the for...of scheme loop at line 591 combines four scheme assertions in a single test(). If one scheme fails, Bun's error output will report the test name but not automatically identify the failing scheme. Consider splitting into separate test cases or adding scheme to the expect failure message if you ever find the test brittle in CI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/tool/project-scan.test.ts` around lines 567 - 602, The
test "detects Trino via DATABASE_URL with trino schemes" embeds a for...of loop
over schemes which hides which scheme fails; update the test to run each scheme
as its own subtest by using test.each or describe.each so each scheme is
reported individually, e.g., replace the for...of loop with test.each([...]) or
describe.each([...]) and inside call clearWarehouseEnvVars(), set
process.env.DATABASE_URL, await detectEnvVars(), and assert trino fields
(including scheme in the test title) to get per-scheme failure diagnostics.
packages/drivers/src/trino.ts (1)

180-186: 💤 Low value

Static-analysis SQL-injection hint is a false positive — but worth a short comment.

OpenGrep flags the template-literal concatenation in listSchemas/listTables/describeTable. Catalog goes through quoteIdent (doubles ") and schema/table go through escapeStringLiteral (doubles '), which are the correct primitives for Trino — information_schema queries can't accept catalog as a bind parameter, and Trino's standard string-literal escape is ''. Consider adding a short inline comment explaining why bind parameters aren't used here, so future readers (and the linter) don't try to "fix" this:

+    // information_schema queries are catalog-qualified at the FROM clause,
+    // which Trino cannot bind. We escape via quoteIdent / escapeStringLiteral.
     const result = await connector.execute(
       `SELECT table_name, table_type
        FROM ${quoteIdent(catalog)}.information_schema.tables
        WHERE table_schema = '${escapeStringLiteral(schema)}'
        ORDER BY table_name`,
       10000,
     )

Also applies to: 196-202, 219-226

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/drivers/src/trino.ts` around lines 180 - 186, Add a short inline
comment in the SQL-building sites (functions listSchemas, listTables,
describeTable) explaining that the template-literal concatenation is intentional
and safe because identifiers are sanitized with quoteIdent (which doubles ") and
values use escapeStringLiteral (which doubles '), and that Trino does not
support using bind parameters for catalog/schema/table identifiers in
information_schema queries; this will silence static-analysis false positives
and guide future readers not to replace these with parameterized binds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/docs/configure/warehouses.md`:
- Around line 518-519: The Docker auto-discovery list in the table row that
currently reads "Docker containers | Finds running PostgreSQL, MySQL, SQL
Server, Trino, and ClickHouse containers" is missing MongoDB and MariaDB; update
that table row to include "MongoDB" and "MariaDB" so it matches the documented
discovery behavior elsewhere in the PR (look for the table row text "Docker
containers | Finds running PostgreSQL, MySQL, SQL Server, Trino, and ClickHouse
containers" to locate the exact spot to edit).

In `@docs/docs/data-engineering/tools/warehouse-tools.md`:
- Line 57: The Docker DBs detection-method table row ("| **Docker DBs** | Bridge
call to discover running PostgreSQL/MySQL/MSSQL/Trino containers |") is
inconsistent with the warehouse_discover section; update that table row to list
the same databases (add ClickHouse, MongoDB, and MariaDB alongside
PostgreSQL/MySQL/MSSQL/Trino) so the documentation matches the
warehouse_discover section and ensures parity between the two descriptions.

In `@docs/docs/drivers.md`:
- Around line 126-133: Add a blank line between the "### Trino" heading and the
following Markdown table so the table does not immediately follow the heading
(this will satisfy markdownlint MD058); locate the "### Trino" heading and the
table block listing auth methods and insert one empty line between them.

In `@packages/drivers/src/trino.ts`:
- Around line 82-91: The current try-catch around the dynamic import and the
export validation masks export-shape errors as "not installed"; split the logic
so the import("trino-client") is inside its own try-catch and only that catch
throws "Trino driver not installed...", then perform the validation of
Trino/BasicAuth/Trino.create outside that catch and, if validation fails, throw
a distinct error like "Trino.create export not found in trino-client" (or
include the actual module shape) so missing/renamed exports are reported
accurately; reference the Trino and BasicAuth bindings and the
import("trino-client") call when updating the control flow.

In `@packages/opencode/src/altimate/native/connections/registry.ts`:
- Around line 133-134: When constructing/normalizing connection data for display
in warehouse.list, ensure Trino connections use catalog as a fallback for
database: detect the registry entry for "trino" (the trino key in registry.ts)
and if a connection's config has no config.database but has config.catalog, set
config.database = config.catalog before rendering/storing the connection object;
this ensures warehouse.list shows a meaningful Database value for Trino.

---

Outside diff comments:
In `@packages/opencode/src/altimate/native/connections/dbt-profiles.ts`:
- Around line 56-64: The resolveEnvVars function's regex only matches
single-quoted args so double-quoted env_var forms are left unresolved; update
the regex in resolveEnvVars to accept either single or double quotes (use a
capture for the opening quote like (['"]) and a backreference \1 for the closing
quote) for both the env name and the optional default, then use the captured
groups for envName and defaultValue in the replacement so both {{
env_var('NAME') }}, {{ env_var("NAME") }}, and defaults with either quote style
are handled.

---

Nitpick comments:
In `@packages/drivers/src/trino.ts`:
- Around line 180-186: Add a short inline comment in the SQL-building sites
(functions listSchemas, listTables, describeTable) explaining that the
template-literal concatenation is intentional and safe because identifiers are
sanitized with quoteIdent (which doubles ") and values use escapeStringLiteral
(which doubles '), and that Trino does not support using bind parameters for
catalog/schema/table identifiers in information_schema queries; this will
silence static-analysis false positives and guide future readers not to replace
these with parameterized binds.

In `@packages/opencode/test/tool/project-scan.test.ts`:
- Around line 567-602: The test "detects Trino via DATABASE_URL with trino
schemes" embeds a for...of loop over schemes which hides which scheme fails;
update the test to run each scheme as its own subtest by using test.each or
describe.each so each scheme is reported individually, e.g., replace the
for...of loop with test.each([...]) or describe.each([...]) and inside call
clearWarehouseEnvVars(), set process.env.DATABASE_URL, await detectEnvVars(),
and assert trino fields (including scheme in the test title) to get per-scheme
failure diagnostics.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a24a4e41-d302-4fb1-b333-74272c0e5444

📥 Commits

Reviewing files that changed from the base of the PR and between c859b57 and 590a5d4.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (22)
  • README.md
  • docs/docs/configure/warehouses.md
  • docs/docs/data-engineering/tools/warehouse-tools.md
  • docs/docs/drivers.md
  • docs/docs/getting-started/index.md
  • packages/drivers/package.json
  • packages/drivers/src/index.ts
  • packages/drivers/src/normalize.ts
  • packages/drivers/src/trino.ts
  • packages/drivers/test/trino-unit.test.ts
  • packages/opencode/script/publish.ts
  • packages/opencode/src/altimate/native/connections/dbt-profiles.ts
  • packages/opencode/src/altimate/native/connections/docker-discovery.ts
  • packages/opencode/src/altimate/native/connections/register.ts
  • packages/opencode/src/altimate/native/connections/registry.ts
  • packages/opencode/src/altimate/native/finops/query-history.ts
  • packages/opencode/src/altimate/tools/project-scan.ts
  • packages/opencode/src/altimate/tools/warehouse-add.ts
  • packages/opencode/test/altimate/connections.test.ts
  • packages/opencode/test/altimate/driver-normalize.test.ts
  • packages/opencode/test/altimate/tools/sql-explain.test.ts
  • packages/opencode/test/tool/project-scan.test.ts

Comment on lines +518 to 519
| Docker containers | Finds running PostgreSQL, MySQL, SQL Server, Trino, and ClickHouse containers |
| Environment variables | Scans for `SNOWFLAKE_ACCOUNT`, `PGHOST`, `DATABRICKS_HOST`, etc. |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Auto-discovery Docker list is incomplete for current behavior/docs.

This line omits MongoDB (and MariaDB), which are documented as discoverable elsewhere in this PR.

Suggested patch
-| Docker containers | Finds running PostgreSQL, MySQL, SQL Server, Trino, and ClickHouse containers |
+| Docker containers | Finds running PostgreSQL, MySQL, MariaDB, SQL Server, Trino, ClickHouse, and MongoDB containers |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| Docker containers | Finds running PostgreSQL, MySQL, SQL Server, Trino, and ClickHouse containers |
| Environment variables | Scans for `SNOWFLAKE_ACCOUNT`, `PGHOST`, `DATABRICKS_HOST`, etc. |
| Docker containers | Finds running PostgreSQL, MySQL, MariaDB, SQL Server, Trino, ClickHouse, and MongoDB containers |
| Environment variables | Scans for `SNOWFLAKE_ACCOUNT`, `PGHOST`, `DATABRICKS_HOST`, etc. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/docs/configure/warehouses.md` around lines 518 - 519, The Docker
auto-discovery list in the table row that currently reads "Docker containers |
Finds running PostgreSQL, MySQL, SQL Server, Trino, and ClickHouse containers"
is missing MongoDB and MariaDB; update that table row to include "MongoDB" and
"MariaDB" so it matches the documented discovery behavior elsewhere in the PR
(look for the table row text "Docker containers | Finds running PostgreSQL,
MySQL, SQL Server, Trino, and ClickHouse containers" to locate the exact spot to
edit).

| **dbt manifest** | Parses `target/manifest.json` for model/source/test counts |
| **dbt profiles** | Searches for `profiles.yml`: `DBT_PROFILES_DIR` env var → project root → `<home>/.dbt/profiles.yml` |
| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL containers |
| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL/Trino containers |
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Detection-method table is now inconsistent with the warehouse_discover section.

This row omits ClickHouse/MongoDB (and MariaDB) even though the same page documents them as discovered containers.

Suggested wording update
-| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL/Trino containers |
+| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MariaDB/MSSQL/Trino/ClickHouse/MongoDB containers |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MSSQL/Trino containers |
| **Docker DBs** | Bridge call to discover running PostgreSQL/MySQL/MariaDB/MSSQL/Trino/ClickHouse/MongoDB containers |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/docs/data-engineering/tools/warehouse-tools.md` at line 57, The Docker
DBs detection-method table row ("| **Docker DBs** | Bridge call to discover
running PostgreSQL/MySQL/MSSQL/Trino containers |") is inconsistent with the
warehouse_discover section; update that table row to list the same databases
(add ClickHouse, MongoDB, and MariaDB alongside PostgreSQL/MySQL/MSSQL/Trino) so
the documentation matches the warehouse_discover section and ensures parity
between the two descriptions.

Comment thread docs/docs/drivers.md
Comment on lines +126 to +133
### Trino
| Method | Config Fields |
|--------|--------------|
| No auth | `host`, `port`, `catalog`, `schema`, `user` |
| Basic | `host`, `port`, `catalog`, `schema`, `user`, `password` |
| Bearer token | `host`, `port`, `catalog`, `schema`, `access_token` |
| Connection String | `connection_string: "https://trino.example.com:8443"` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a blank line before the Trino auth table to satisfy markdownlint (MD058).

The table starts immediately under the heading, which triggers the configured lint warning.

Suggested patch
 ### Trino
+
 | Method | Config Fields |
 |--------|--------------|
 | No auth | `host`, `port`, `catalog`, `schema`, `user` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 127-127: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/docs/drivers.md` around lines 126 - 133, Add a blank line between the
"### Trino" heading and the following Markdown table so the table does not
immediately follow the heading (this will satisfy markdownlint MD058); locate
the "### Trino" heading and the table block listing auth methods and insert one
empty line between them.

Comment on lines +82 to +91
try {
const mod = await import("trino-client")
Trino = mod.Trino ?? mod.default?.Trino ?? mod.default
BasicAuth = mod.BasicAuth ?? mod.default?.BasicAuth
if (!Trino?.create) {
throw new Error("Trino.create export not found in trino-client")
}
} catch {
throw new Error("Trino driver not installed. Run: npm install trino-client")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Misleading error when trino-client is installed but missing the expected exports.

The try wraps both the dynamic import("trino-client") and the if (!Trino?.create) throw new Error("Trino.create export not found …") validation. The inner throw is caught by the same catch on line 89 and rewritten to "Trino driver not installed. Run: npm install trino-client". If a user has trino-client installed but its shape changes (e.g., a major-version bump that re-exports differently), they'll be told to run a npm install that won't fix anything.

🛠️ Suggested fix — only treat actual import failure as "not installed"
 export async function connect(config: ConnectionConfig): Promise<Connector> {
   let Trino: any
   let BasicAuth: any
+  let mod: any
   try {
-    const mod = await import("trino-client")
-    Trino = mod.Trino ?? mod.default?.Trino ?? mod.default
-    BasicAuth = mod.BasicAuth ?? mod.default?.BasicAuth
-    if (!Trino?.create) {
-      throw new Error("Trino.create export not found in trino-client")
-    }
+    mod = await import("trino-client")
   } catch {
     throw new Error("Trino driver not installed. Run: npm install trino-client")
   }
+  Trino = mod.Trino ?? mod.default?.Trino ?? mod.default
+  BasicAuth = mod.BasicAuth ?? mod.default?.BasicAuth
+  if (!Trino?.create) {
+    throw new Error("Trino.create export not found in trino-client — check the installed package version")
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/drivers/src/trino.ts` around lines 82 - 91, The current try-catch
around the dynamic import and the export validation masks export-shape errors as
"not installed"; split the logic so the import("trino-client") is inside its own
try-catch and only that catch throws "Trino driver not installed...", then
perform the validation of Trino/BasicAuth/Trino.create outside that catch and,
if validation fails, throw a distinct error like "Trino.create export not found
in trino-client" (or include the actual module shape) so missing/renamed exports
are reported accurately; reference the Trino and BasicAuth bindings and the
import("trino-client") call when updating the control flow.

Comment on lines +133 to 134
trino: "@altimateai/drivers/trino",
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Trino entries may show blank “Database” in warehouse.list.

Trino uses catalog as canonical, but list output reads only config.database. Add a Trino fallback so discovered/saved Trino connections render meaningful context.

Suggested patch
   for (const [name, config] of configs) {
+    const type = typeof config.type === "string" ? config.type.toLowerCase() : ""
     warehouses.push({
       name,
-      type: config.type,
-      database: config.database as string | undefined,
+      type: config.type,
+      database:
+        type === "trino"
+          ? ((config.catalog as string | undefined) ?? (config.database as string | undefined))
+          : (config.database as string | undefined),
     })
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/altimate/native/connections/registry.ts` around lines
133 - 134, When constructing/normalizing connection data for display in
warehouse.list, ensure Trino connections use catalog as a fallback for database:
detect the registry entry for "trino" (the trino key in registry.ts) and if a
connection's config has no config.database but has config.catalog, set
config.database = config.catalog before rendering/storing the connection object;
this ensures warehouse.list shows a meaningful Database value for Trino.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants