From a1cddb5b727e963acc55f5faf6f5018eb22f05ee Mon Sep 17 00:00:00 2001 From: "lcj.licunjie" <1067532060@qq.com> Date: Wed, 9 Sep 2026 12:31:01 +0800 Subject: [PATCH 1/2] fix(trino): pin a session schema for unqualified statements --- docs/API_DOCS.md | 1 + docs/providers/trino.md | 15 ++- e2e/connection-management.spec.ts | 17 +++ src/components/ConnectionModal.tsx | 21 +++ src/hooks/use-connection-form.ts | 8 ++ src/hooks/use-connection-payload.ts | 1 + src/lib/agent/context-snapshot.ts | 1 + src/lib/db-ui-config.ts | 3 +- .../db/providers/sql/trino/http-transport.ts | 7 +- src/lib/db/providers/sql/trino/index.ts | 2 +- src/lib/db/providers/sql/trino/introspect.ts | 6 +- src/lib/seed/connection-filter.ts | 1 + src/lib/seed/types.ts | 1 + src/lib/storage/connection-secrets.ts | 1 + src/lib/types.ts | 2 + tests/components/ConnectionModal.test.tsx | 14 ++ tests/hooks/use-connection-form.test.ts | 120 ++++++++++++++++++ tests/integration/db/trino-provider.test.ts | 25 +++- .../unit/agent-connection-eligibility.test.ts | 6 + tests/unit/db/trino/http-transport.test.ts | 9 +- tests/unit/db/trino/introspect.test.ts | 2 +- tests/unit/lib/agent/context-snapshot.test.ts | 1 + tests/unit/lib/db-ui-config.test.ts | 2 +- .../lib/storage/connection-secrets.test.ts | 1 + tests/unit/seed/connection-filter.test.ts | 8 ++ tests/unit/seed/types.test.ts | 33 +++++ 26 files changed, 291 insertions(+), 17 deletions(-) diff --git a/docs/API_DOCS.md b/docs/API_DOCS.md index a8c33d57c..951e71b5e 100644 --- a/docs/API_DOCS.md +++ b/docs/API_DOCS.md @@ -1282,6 +1282,7 @@ interface DatabaseConnection { user?: string; // Username password?: string; // Password database?: string; // Database name (Couchbase: the bucket; Druid: unused, it has one catalog; Trino: the CATALOG; Cassandra: the KEYSPACE) + schema?: string; // Trino: session schema for unqualified table names connectionString?: string; // Full connection string (alternative; Druid has no URI form, host + port only; Cassandra has none either, no URI carries localDataCenter) createdAt: Date; // Creation timestamp color?: string; // UI accent for this connection diff --git a/docs/providers/trino.md b/docs/providers/trino.md index ac772bd90..8334e071f 100644 --- a/docs/providers/trino.md +++ b/docs/providers/trino.md @@ -475,12 +475,23 @@ quote character in its grammar at all. | `host` | **Yes** | The coordinator. The only validated requirement | | `port` | No | Defaults to `8080`, for both `http://` and `https://` | | `database` | No | **The catalog** to pin ([§3.2](#32-the-connections-database-field-pins-one-catalog)). Without it the editor works and the tree does not | +| `schema` | No | Session schema for unqualified table names, sent as `X-Trino-Schema` | | `username` | No | Sent as `X-Trino-User`. Defaults to `libredb`; never omitted ([§3.6](#36-a-password-is-a-tls-only-credential)) | | `password` | No | `Authorization: Basic`, **and only over TLS** ([§4.3](#43-tls-and-the-password-rule)) | | `ssl` | No | Selects `https://` | -There is no field for a session schema, which is why every generated name is qualified -`schema.table`: Trino resolves a bare name only when the session has a schema. +The connection form exposes **Catalog Name** and **Schema Name**. Set both to run +unqualified statements in the editor and Create Table: for example, Catalog `memory` +and Schema `default` let `SELECT * FROM widgets` and `CREATE TABLE t (id INTEGER NOT NULL)` +resolve inside `memory.default`. `SHOW SCHEMAS` lists the catalog's available schemas. +No schema is guessed: an omitted or empty value sends no schema header, so existing +connections keep their behavior until edited. Without a session schema, qualify table names. + +Per-statement `TrinoQueryOptions.schema` overrides the connection default; an explicit empty +string omits the header for that statement. Headers still use the dialect's prefix. The schema +tree continues to list the whole catalog and qualify names as `schema.table`, so selecting a +table outside the session schema still targets that table. Fully qualified queries can still +reach other catalogs. The session schema is also supported in seeded connections. ### 4.2 There is no connection string — yet diff --git a/e2e/connection-management.spec.ts b/e2e/connection-management.spec.ts index c0c15bd5d..bd2adbc59 100644 --- a/e2e/connection-management.spec.ts +++ b/e2e/connection-management.spec.ts @@ -46,6 +46,23 @@ test.describe("Connection Management", () => { await expect(page.locator('input[value="localhost"]').first()).toBeVisible(); }); + test("Trino sends the catalog and session schema entered in the form", async ({ page }) => { + await page.route("**/api/db/test-connection", (route) => route.fulfill({ json: { success: true, latency: 1 } })); + const sidebarButtons = page.locator("text=LibreDB Studio").locator("..").locator("..").locator("button"); + await sidebarButtons.last().click(); + const dialog = page.getByRole("dialog"); + await dialog.getByRole("button", { name: "Trino", exact: true }).click(); + await dialog.getByLabel("Catalog Name").fill("memory"); + await dialog.getByLabel("Schema Name").fill("default"); + const request = page.waitForRequest("**/api/db/test-connection"); + await dialog.getByRole("button", { name: "Test Connection", exact: true }).click(); + expect((await request).postDataJSON()).toMatchObject({ + type: "trino", + database: "memory", + schema: "default", + }); + }); + test("connection modal can be closed", async ({ page }) => { const sidebarButtons = page.locator("text=LibreDB Studio").locator("..").locator("..").locator("button"); await sidebarButtons.last().click(); diff --git a/src/components/ConnectionModal.tsx b/src/components/ConnectionModal.tsx index 16c981287..906a9bfe6 100644 --- a/src/components/ConnectionModal.tsx +++ b/src/components/ConnectionModal.tsx @@ -89,6 +89,8 @@ export function ConnectionModal({ setPassword, database, setDatabase, + schema, + setSchema, connectionString, setConnectionString, mongoConnectionMode, @@ -553,6 +555,25 @@ export function ConnectionModal({ )} + {takesConnectionField(type, "schema") && ( +
+ + setSchema(e.target.value)} + placeholder="default" + className="h-10 bg-panel border-hairline focus:border-brand-tint/50 transition-all text-xs font-mono" + /> +

+ Used for unqualified table names in queries and Create Table. Leave empty to qualify names + yourself. Run SHOW SCHEMAS to list the catalog's schemas. +

+
+ )} + {/* In the open rather than behind the Advanced accordion for the reason Cassandra's field below is: the deployment that needs it is diff --git a/src/hooks/use-connection-form.ts b/src/hooks/use-connection-form.ts index 9a1db5e47..070511d61 100644 --- a/src/hooks/use-connection-form.ts +++ b/src/hooks/use-connection-form.ts @@ -46,6 +46,7 @@ const FIELD_OWNERSHIP: Record = { user: "edited", password: "edited", database: "edited", + schema: "edited", connectionString: "edited", createdAt: "edited", color: "edited", @@ -133,6 +134,7 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon const [user, setUser] = useState(""); const [password, setPassword] = useState(""); const [database, setDatabase] = useState(""); + const [schema, setSchema] = useState(""); const [isTesting, setIsTesting] = useState(false); const [connectionString, setConnectionString] = useState(""); const [mongoConnectionMode, setMongoConnectionMode] = useState<"host" | "connectionString">("host"); @@ -198,6 +200,7 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon setUser(editConnection.user || ""); setPassword(editConnection.password || ""); setDatabase(editConnection.database || ""); + setSchema(editConnection.schema || ""); setConnectionString(editConnection.connectionString || ""); setEnvironment(editConnection.environment || "local"); if (editConnection.connectionString) { @@ -273,6 +276,7 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon setUser(""); setPassword(""); setDatabase(""); + setSchema(""); setConnectionString(""); setMongoConnectionMode("host"); setType("postgres"); @@ -341,6 +345,7 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon ...(addressedFields.has("user") ? { user } : {}), ...(addressedFields.has("password") ? { password } : {}), ...(addressedFields.has("database") ? { database } : {}), + ...(addressedFields.has("schema") && schema ? { schema } : {}), createdAt: editConnection?.createdAt || new Date(), environment, color: ENVIRONMENT_COLORS[environment], @@ -381,6 +386,7 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon user, password, database, + schema, environment, mongoConnectionMode, connectionString, @@ -623,7 +629,9 @@ export function useConnectionForm({ isOpen, onConnect, editConnection, onTestCon password, setPassword, database, + schema, setDatabase, + setSchema, connectionString, setConnectionString, mongoConnectionMode, diff --git a/src/hooks/use-connection-payload.ts b/src/hooks/use-connection-payload.ts index 59f719bb9..e48fb0d99 100644 --- a/src/hooks/use-connection-payload.ts +++ b/src/hooks/use-connection-payload.ts @@ -94,6 +94,7 @@ const CONNECTION_RELEVANCE: Record = { user: "resolution", password: "resolution", database: "resolution", + schema: "resolution", connectionString: "resolution", serviceName: "resolution", instanceName: "resolution", diff --git a/src/lib/agent/context-snapshot.ts b/src/lib/agent/context-snapshot.ts index ddcb34071..d650e5503 100644 --- a/src/lib/agent/context-snapshot.ts +++ b/src/lib/agent/context-snapshot.ts @@ -820,6 +820,7 @@ export function connectionIdentity(connection: DatabaseConnection): string { connection.host ?? "", connection.port ?? "", connection.database ?? "", + connection.schema ?? "", connection.connectionString ?? "", connection.serviceName ?? "", connection.instanceName ?? "", diff --git a/src/lib/db-ui-config.ts b/src/lib/db-ui-config.ts index c42e1e223..f0bd08b6c 100644 --- a/src/lib/db-ui-config.ts +++ b/src/lib/db-ui-config.ts @@ -35,6 +35,7 @@ export interface DatabaseUIConfig { | "user" | "password" | "database" + | "schema" | "connectionString" | "serviceName" | "instanceName" @@ -255,7 +256,7 @@ export const DB_UI_CONFIG: Record = { // `SHOW CATALOGS` answers jmx, memory, system, tpcds, tpch) and a connection pins // one, the way a PostgreSQL connection pins a database. The form labels it // "Catalog" rather than "Database" - see ConnectionModal.tsx. - connectionFields: ["host", "port", "user", "password", "database"], + connectionFields: ["host", "port", "user", "password", "database", "schema"], }, cassandra: { icon: CassandraIcon, diff --git a/src/lib/db/providers/sql/trino/http-transport.ts b/src/lib/db/providers/sql/trino/http-transport.ts index 1d8a71844..064e9d4ff 100644 --- a/src/lib/db/providers/sql/trino/http-transport.ts +++ b/src/lib/db/providers/sql/trino/http-transport.ts @@ -690,6 +690,7 @@ export class TrinoHttpTransport implements TrinoTransport { private readonly origin: string; private readonly user: string; private readonly catalog: string | undefined; + private readonly schema: string | undefined; private readonly authorization: string | undefined; constructor(dialect: TrinoDialect, config: DatabaseConnection) { @@ -706,6 +707,7 @@ export class TrinoHttpTransport implements TrinoTransport { // boundary: a fully qualified statement still reaches any catalog the session // can see. this.catalog = config.database; + this.schema = config.schema; if (config.password === undefined || config.password === "") { this.authorization = undefined; @@ -880,14 +882,13 @@ export class TrinoHttpTransport implements TrinoTransport { */ private submitHeaders(options: TrinoQueryOptions): Record { const catalog = options.catalog ?? this.catalog; + const schema = options.schema ?? this.schema; return { ...this.sessionHeaders(), "content-type": SQL_CONTENT_TYPE, [this.header(HEADER_SUFFIXES.TIME_ZONE)]: CLIENT_TIME_ZONE, ...(catalog === undefined || catalog === "" ? {} : { [this.header(HEADER_SUFFIXES.CATALOG)]: catalog }), - ...(options.schema === undefined || options.schema === "" - ? {} - : { [this.header(HEADER_SUFFIXES.SCHEMA)]: options.schema }), + ...(schema === undefined || schema === "" ? {} : { [this.header(HEADER_SUFFIXES.SCHEMA)]: schema }), }; } diff --git a/src/lib/db/providers/sql/trino/index.ts b/src/lib/db/providers/sql/trino/index.ts index 2ac6d7c77..6276b777b 100644 --- a/src/lib/db/providers/sql/trino/index.ts +++ b/src/lib/db/providers/sql/trino/index.ts @@ -458,7 +458,7 @@ export class TrinoProvider extends SQLBaseProvider { const catalog = this.config.database; if (catalog === undefined || catalog === "") { throw new DatabaseConfigError( - `This connection pins no ${this.dialect.displayName} catalog, so there is no schema to list. Set the catalog on the connection, or qualify every name in full.`, + `This connection pins no ${this.dialect.displayName} catalog, so there is no schema to list. Set the catalog on the connection to list its tables. Set a session schema as well to use unqualified table names in queries and Create Table.`, this.type, ); } diff --git a/src/lib/db/providers/sql/trino/introspect.ts b/src/lib/db/providers/sql/trino/introspect.ts index eb124c55e..a19ce2a86 100644 --- a/src/lib/db/providers/sql/trino/introspect.ts +++ b/src/lib/db/providers/sql/trino/introspect.ts @@ -549,9 +549,9 @@ function readTableAddresses(rows: TrinoRow[]): TableAddress[] { * the cluster has configured being reachable. * * A table's display name is therefore `schema.table`, always qualified: Trino - * resolves an unqualified name only when the SESSION has a schema, and this - * transport pins a catalog and no schema (there is no connection field for one), - * so a bare name in a generated statement would not resolve at all. + * lists tables across every schema in the catalog, regardless of the optional + * session schema pinned on the connection. Qualifying names keeps tree-generated + * statements targeting the selected table even when it is outside that schema. */ export async function getSchema(runner: TrinoQueryRunner, catalog: string): Promise { const [tableRows, columnRows] = await Promise.all([ diff --git a/src/lib/seed/connection-filter.ts b/src/lib/seed/connection-filter.ts index 816554684..a312c0781 100644 --- a/src/lib/seed/connection-filter.ts +++ b/src/lib/seed/connection-filter.ts @@ -41,6 +41,7 @@ export function filterByRoles(connections: SeedConnection[], userRoles: string[] // MongoDB's auth database. Dropping it here would list a seeded connection that // authenticates against the wrong database and reports a credentials error. authSource: conn.authSource, + schema: conn.schema, createdAt: new Date(), managed: conn.managed ?? true, roles: conn.roles, diff --git a/src/lib/seed/types.ts b/src/lib/seed/types.ts index d0b1a5706..30124af0f 100644 --- a/src/lib/seed/types.ts +++ b/src/lib/seed/types.ts @@ -84,6 +84,7 @@ export const SeedConnectionSchema = z.object({ // deployment). Optional because the driver falls back to the database being opened, // which is right only when the two are the same. authSource: z.string().optional(), + schema: z.string().optional(), }); export const SeedConfigSchema = z diff --git a/src/lib/storage/connection-secrets.ts b/src/lib/storage/connection-secrets.ts index 2d249b6ed..62910d0d9 100644 --- a/src/lib/storage/connection-secrets.ts +++ b/src/lib/storage/connection-secrets.ts @@ -42,6 +42,7 @@ export const CONNECTION_FIELDS: Record = { // A DATABASE NAME (`admin`), not a credential. The password that authenticates // against it is the secret, and it is classified above. authSource: "public", + schema: "public", managed: "public", seedId: "public", agentUser: "public", diff --git a/src/lib/types.ts b/src/lib/types.ts index dd4859299..a81f4509c 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -150,6 +150,8 @@ export interface DatabaseConnection { user?: string; password?: string; database?: string; + /** Trino: the session schema used to resolve unqualified table names. */ + schema?: string; connectionString?: string; createdAt: Date; color?: string; diff --git a/tests/components/ConnectionModal.test.tsx b/tests/components/ConnectionModal.test.tsx index 36b73af22..4e6f64b92 100644 --- a/tests/components/ConnectionModal.test.tsx +++ b/tests/components/ConnectionModal.test.tsx @@ -179,6 +179,8 @@ function getDefaultForm() { setInstanceName: mockSetInstanceName, localDataCenter: "", setLocalDataCenter: mockSetLocalDataCenter, + schema: "", + setSchema: mock(() => {}), authSource: "", setAuthSource: mockSetAuthSource, showSSH: false, @@ -234,6 +236,7 @@ mock.module("@/hooks/use-connection-form", () => ({ // passes or fails against THIS list, not against src/lib/db-ui-config.ts. The real table is // the authority and tests/unit/lib/db-ui-config.test.ts derives it from the providers. const MOCK_CONNECTION_FIELDS: Record = { + trino: ["host", "port", "user", "password", "database", "schema"], sqlite: ["database"], libredb: ["database"], duckdb: ["database"], @@ -840,6 +843,16 @@ describe("ConnectionModal", () => { expect(queryByText(/The Trino catalog to open/)).not.toBeNull(); }); + test("Trino exposes an editable session schema beside the catalog", () => { + const setSchema = mock(() => {}); + mockFormOverrides = { type: "trino", schema: "tiny", setSchema }; + const { getByLabelText } = render(React.createElement(ConnectionModal, createDefaultProps())); + const input = getByLabelText("Schema Name") as HTMLInputElement; + expect(input.value).toBe("tiny"); + fireEvent.change(input, { target: { value: "default" } }); + expect(setSchema).toHaveBeenCalledWith("default"); + }); + test("Trino warns that a password needs TLS, before the connection can 401 on it", () => { // Measured on 476 with authentication DISABLED: `Authorization: Basic` over plain // HTTP is answered 401, "Password not allowed for insecure authentication". So @@ -857,6 +870,7 @@ describe("ConnectionModal", () => { const { queryByText } = render(React.createElement(ConnectionModal, props)); expect(queryByText("Catalog Name")).toBeNull(); + expect(queryByText("Schema Name")).toBeNull(); expect(queryByText(/refuses a password over plain HTTP/)).toBeNull(); }); diff --git a/tests/hooks/use-connection-form.test.ts b/tests/hooks/use-connection-form.test.ts index a3a2ebf02..278c67560 100644 --- a/tests/hooks/use-connection-form.test.ts +++ b/tests/hooks/use-connection-form.test.ts @@ -24,6 +24,7 @@ const DEFAULT_PORTS: Record = { // list that omitted `user` while the provider authenticated with it. The real table is the // authority; tests/unit/lib/db-ui-config.test.ts derives it from the provider sources. const MOCK_CONNECTION_FIELDS: Record = { + trino: ["host", "port", "user", "password", "database", "schema"], sqlite: ["database"], libredb: ["database"], duckdb: ["database"], @@ -1641,6 +1642,125 @@ describe("useConnectionForm", () => { expect(result.current.authSource).toBe(""); }); + test("buildConnection includes the Trino schema", async () => { + const fetchMock = mockGlobalFetch({ + "/api/db/test-connection": { ok: true, json: { success: true, latency: 20 } }, + }); + + const { result } = renderHook(() => useConnectionForm(defaultProps)); + + act(() => { + result.current.setType("trino"); + result.current.setSchema("default"); + }); + + await act(async () => { + await result.current.handleTestConnection(); + }); + + const testCall = fetchMock.mock.calls.find( + (call) => typeof call[0] === "string" && call[0].includes("/api/db/test-connection"), + ); + const body = JSON.parse(testCall![1]!.body as string); + expect(body.schema).toBe("default"); + }); + + test.each(["default", ""])("saving an edited Trino connection writes schema %s", async (schema) => { + mockGlobalFetch({ + "/api/db/test-connection": { ok: true, json: { success: true } }, + }); + const onConnect = mock<(connection: DatabaseConnection) => void>(() => {}); + const editConnection: DatabaseConnection = { + id: "trino-1", + name: "Trino", + type: "trino", + host: "localhost", + database: "memory", + schema: "tiny", + createdAt: new Date(), + }; + const { result } = renderHook(() => useConnectionForm({ ...defaultProps, onConnect, editConnection })); + act(() => result.current.setSchema(schema)); + await act(async () => { + await result.current.handleConnect(); + }); + expect(onConnect).toHaveBeenCalledWith(expect.objectContaining({ database: "memory" })); + const saved = onConnect.mock.calls[0]![0]; + expect(saved.schema).toBe(schema || undefined); + }); + + test("a schema typed for another engine is not sent", async () => { + const fetchMock = mockGlobalFetch({ + "/api/db/test-connection": { ok: true, json: { success: true, latency: 20 } }, + }); + + const { result } = renderHook(() => useConnectionForm(defaultProps)); + + act(() => { + result.current.setType("postgres"); + result.current.setSchema("default"); + }); + + await act(async () => { + await result.current.handleTestConnection(); + }); + + const testCall = fetchMock.mock.calls.find( + (call) => typeof call[0] === "string" && call[0].includes("/api/db/test-connection"), + ); + const body = JSON.parse(testCall![1]!.body as string); + expect(body.schema).toBeUndefined(); + }); + + test("populates the Trino schema in edit mode, and clears it when there is none", () => { + const withSchema: DatabaseConnection = { + id: "m1", + name: "Shop", + type: "trino", + host: "trino.internal", + port: 8080, + database: "memory", + schema: "default", + createdAt: new Date(), + }; + const withoutSchema: DatabaseConnection = { + id: "m2", + name: "Other", + type: "trino", + host: "trino-2.internal", + port: 8080, + database: "memory", + createdAt: new Date(), + }; + + const { result, rerender } = renderHook((props) => useConnectionForm(props), { + initialProps: { ...defaultProps, editConnection: withSchema }, + }); + + expect(result.current.schema).toBe("default"); + + rerender({ ...defaultProps, editConnection: withoutSchema }); + + expect(result.current.schema).toBe(""); + }); + + test("clearing the modal clears the schema before the next new connection", () => { + const { result, rerender } = renderHook((props) => useConnectionForm(props), { + initialProps: { ...defaultProps, isOpen: true }, + }); + + act(() => { + result.current.setType("trino"); + result.current.setSchema("default"); + }); + + expect(result.current.schema).toBe("default"); + + rerender({ ...defaultProps, isOpen: false }); + + expect(result.current.schema).toBe(""); + }); + // ── buildConnection with MSSQL instanceName ──────────────────────────── test("buildConnection includes MSSQL instanceName", async () => { diff --git a/tests/integration/db/trino-provider.test.ts b/tests/integration/db/trino-provider.test.ts index e0045b22c..2550b21df 100644 --- a/tests/integration/db/trino-provider.test.ts +++ b/tests/integration/db/trino-provider.test.ts @@ -617,6 +617,26 @@ describe("TrinoProvider validation", () => { }); describe("TrinoProvider lifecycle", () => { + test.each(["SELECT * FROM widgets", "CREATE TABLE t (id INTEGER NOT NULL)"])( + "sends the pinned schema for %s", + async (sql) => { + const provider = await connectProvider({ database: "memory", schema: "default" }); + await provider.query(sql); + const submissions = sentHeaders.filter((_, index) => sentMethods[index]?.method === "POST"); + expect(submissions.length).toBeGreaterThan(1); + for (const headers of submissions) { + expect(headers.get("X-Trino-Catalog")).toBe("memory"); + expect(headers.get("X-Trino-Schema")).toBe("default"); + } + }, + ); + + test.each([undefined, ""])("omits the schema header when the connection schema is %s", async (schema) => { + const provider = await connectProvider({ schema }); + await provider.query("SELECT 1"); + expect(sentHeaders.every((headers) => !headers.has("X-Trino-Schema"))).toBe(true); + }); + test("probes the cluster with the cheapest statement there is", async () => { const provider = await connectProvider(); @@ -1004,8 +1024,8 @@ describe("TrinoProvider query preparation", () => { // ============================================================================ describe("TrinoProvider schema", () => { - test("lists every table of the pinned catalog, schema-qualified", async () => { - const provider = await connectProvider(); + test.each([undefined, "tiny"])("lists every catalog table with session schema %s", async (schemaName) => { + const provider = await connectProvider({ schema: schemaName }); const schema = await provider.getSchema(); expect(schema.map((table) => table.name)).toEqual(["sf1.customer", "tiny.nation", "tiny.region"]); @@ -1034,6 +1054,7 @@ describe("TrinoProvider schema", () => { const provider = await connectProvider({ database: undefined }); await expect(provider.getSchema()).rejects.toThrow("pins no Trino catalog"); + await expect(provider.getSchema()).rejects.toThrow("Set a session schema as well"); }); test("surfaces a pinned catalog that does not exist rather than showing an empty tree", async () => { diff --git a/tests/unit/agent-connection-eligibility.test.ts b/tests/unit/agent-connection-eligibility.test.ts index bee4da222..bf2f29432 100644 --- a/tests/unit/agent-connection-eligibility.test.ts +++ b/tests/unit/agent-connection-eligibility.test.ts @@ -107,6 +107,12 @@ describe("which connection a run may be started on", () => { expect(startableId(browserCopy(server, { database: "somewhere-else" }), loaded(server))).toBeNull(); }); + test("a Trino copy with a different session schema is not startable by the seed id", () => { + const server = descriptor({ type: "trino", database: "memory", schema: "default" }); + expect(startableId(browserCopy(server), loaded(server))).toBe("seed:sales"); + expect(startableId(browserCopy(server, { schema: "other" }), loaded(server))).toBeNull(); + }); + test("a copy given different credentials is not startable", () => { const server = descriptor(); diff --git a/tests/unit/db/trino/http-transport.test.ts b/tests/unit/db/trino/http-transport.test.ts index b29c4ddd6..039ca881e 100644 --- a/tests/unit/db/trino/http-transport.test.ts +++ b/tests/unit/db/trino/http-transport.test.ts @@ -463,14 +463,17 @@ describe("TrinoHttpTransport request", () => { // Introspection legitimately reads a catalog other than the pinned one, and the // alternative - USE - is the session mutation this stateless transport discards. test("lets one statement override the catalog and name a schema", async () => { - await makeTransport({ database: "tpch" }).query("SELECT 1", { catalog: "memory", schema: "default" }); + await makeTransport({ database: "tpch", schema: "tiny" }).query("SELECT 1", { + catalog: "memory", + schema: "default", + }); expect(firstCall().headers["X-Trino-Catalog"]).toBe("memory"); expect(firstCall().headers["X-Trino-Schema"]).toBe("default"); }); - test("sends no catalog or schema header when neither is configured", async () => { - await makeTransport().query("SELECT 1", { catalog: "", schema: "" }); + test("lets an explicit empty override omit the pinned catalog and schema headers", async () => { + await makeTransport({ database: "tpch", schema: "tiny" }).query("SELECT 1", { catalog: "", schema: "" }); expect(firstCall().headers["X-Trino-Catalog"]).toBeUndefined(); expect(firstCall().headers["X-Trino-Schema"]).toBeUndefined(); diff --git a/tests/unit/db/trino/introspect.test.ts b/tests/unit/db/trino/introspect.test.ts index 88853f056..a096e04ed 100644 --- a/tests/unit/db/trino/introspect.test.ts +++ b/tests/unit/db/trino/introspect.test.ts @@ -436,7 +436,7 @@ describe("Trino introspection statements", () => { // ============================================================================ describe("Trino getSchema", () => { - test("names every table schema-qualified, because a bare name resolves against no session schema", async () => { + test("names every table schema-qualified, regardless of the connection session schema", async () => { const { runner } = fakeRunner(); const schema = await getSchema(runner, CATALOG); diff --git a/tests/unit/lib/agent/context-snapshot.test.ts b/tests/unit/lib/agent/context-snapshot.test.ts index d9a06af4c..306ad8561 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -1837,6 +1837,7 @@ describe("the identity a held inventory is filed under", () => { // The case B45 describes, and the one an id-keyed hold could not see: same record, // same id, different database. expect(repointed({ database: "staging" })).not.toBe(connectionIdentity(CONNECTION)); + expect(repointed({ schema: "tiny" })).not.toBe(connectionIdentity(CONNECTION)); }); test("a re-pointed host, port or engine is a different identity too", () => { diff --git a/tests/unit/lib/db-ui-config.test.ts b/tests/unit/lib/db-ui-config.test.ts index 9d9561eba..3c5848d62 100644 --- a/tests/unit/lib/db-ui-config.test.ts +++ b/tests/unit/lib/db-ui-config.test.ts @@ -108,7 +108,7 @@ describe("db-ui-config", () => { test("trino exposes its label, coordinator port and connection fields", () => { expect(getDBConfig("trino").label).toBe("Trino"); expect(getDBConfig("trino").defaultPort).toBe("8080"); - expect(getDBConfig("trino").connectionFields).toEqual(["host", "port", "user", "password", "database"]); + expect(getDBConfig("trino").connectionFields).toEqual(["host", "port", "user", "password", "database", "schema"]); }); test("trino keeps the database field, because it selects the catalog", () => { diff --git a/tests/unit/lib/storage/connection-secrets.test.ts b/tests/unit/lib/storage/connection-secrets.test.ts index e5dff1653..3518c6a19 100644 --- a/tests/unit/lib/storage/connection-secrets.test.ts +++ b/tests/unit/lib/storage/connection-secrets.test.ts @@ -91,6 +91,7 @@ describe("the classification is exhaustive by construction", () => { "connectionString", "createdAt", "database", + "schema", "environment", "group", "host", diff --git a/tests/unit/seed/connection-filter.test.ts b/tests/unit/seed/connection-filter.test.ts index f59f312f4..e4f60d7f5 100644 --- a/tests/unit/seed/connection-filter.test.ts +++ b/tests/unit/seed/connection-filter.test.ts @@ -67,6 +67,14 @@ describe("filterByRoles: engine-specific fields", () => { expect(managed.authSource).toBe("admin"); }); + it("carries a Trino connection's session schema through to the managed connection", () => { + const [managed] = filterByRoles( + [{ ...baseConn, type: "trino", port: 8080, database: "memory", schema: "default" }], + ["user"], + ); + + expect(managed.schema).toBe("default"); + }); }); describe("filterByRoles", () => { diff --git a/tests/unit/seed/types.test.ts b/tests/unit/seed/types.test.ts index 6254d5186..5fa9839e9 100644 --- a/tests/unit/seed/types.test.ts +++ b/tests/unit/seed/types.test.ts @@ -220,3 +220,36 @@ describe("SeedConnectionSchema: Cassandra's localDataCenter", () => { expect(result.success).toBe(false); }); }); + +describe("SeedConnectionSchema: Trino's schema", () => { + it("accepts a seeded connection that names its session schema", () => { + const result = SeedConnectionSchema.safeParse({ + id: "memory", + name: "Shop", + type: "trino", + host: "trino.internal", + port: 8080, + database: "memory", + user: "app", + password: "s3cret", + schema: "default", + roles: ["*"], + }); + + expect(result.success).toBe(true); + if (result.success) expect(result.data.schema).toBe("default"); + }); + + it("rejects a session schema that is not a string", () => { + const result = SeedConnectionSchema.safeParse({ + id: "memory", + name: "Shop", + type: "trino", + host: "trino.internal", + schema: 1, + roles: ["*"], + }); + + expect(result.success).toBe(false); + }); +}); From 1da3b49d60b0beabd8543654c54a6e870001d4ae Mon Sep 17 00:00:00 2001 From: "lcj.licunjie" <1067532060@qq.com> Date: Wed, 9 Sep 2026 22:57:42 +0800 Subject: [PATCH 2/2] fix(trino): address schema review feedback --- docs/API_DOCS.md | 5 +++-- docs/SEED_CONNECTIONS.md | 3 +++ docs/providers/trino.md | 3 ++- src/components/ConnectionModal.tsx | 9 ++++++--- src/lib/db/providers/sql/trino/index.ts | 2 +- tests/integration/db/trino-provider.test.ts | 1 + tests/unit/db/trino/http-transport.test.ts | 6 ++++++ tests/unit/seed/types.test.ts | 1 - 8 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/API_DOCS.md b/docs/API_DOCS.md index 951e71b5e..8afb95d54 100644 --- a/docs/API_DOCS.md +++ b/docs/API_DOCS.md @@ -546,7 +546,8 @@ carries a plain statement. Four things differ from the other SQL providers: catalog in full: `SELECT * FROM other_catalog.some_schema.t` runs unchanged. A connection with no catalog runs fully qualified statements fine, but `GET /api/db/schema` refuses with the reason. - **There is no `connectionString`.** `jdbc:trino://host:port/catalog/schema` exists, but the shared - parser does not accept it, so a connection is `host` + `port` (+ optional `database`, `username`). + parser does not accept it, so a connection is `host` + `port` (+ optional `database` catalog, + `schema`, and `username`). A **`password` requires `ssl: true`**: the coordinator answers `401 Password not allowed for insecure authentication` over plain HTTP even with authentication switched off, so a password on an `http://` connection is refused by the provider rather than sent and rejected. @@ -1267,7 +1268,7 @@ Body `{ "connections": [...] }`; returns per-connection health `{ "results": [{ The object is one shape on the wire. Fields the server reads from a request body — and that change how a connection is opened — are the coordinates and credentials (`id`, `name`, `type`, -`host`, `port`, `user`, `password`, `database`, `connectionString`), plus `ssl`, +`host`, `port`, `user`, `password`, `database`, `schema`, `connectionString`), plus `ssl`, `sshTunnel`, `serviceName` (Oracle), `instanceName` (MSSQL), `localDataCenter` (Cassandra), `authSource` (MongoDB), `agentUser`, and `agentPassword`. `color`, `environment`, `group`, `managed`, `seedId`, and `createdAt` are client-side bookkeeping that travel in the same object. diff --git a/docs/SEED_CONNECTIONS.md b/docs/SEED_CONNECTIONS.md index d7f8e52e1..d6af706fc 100644 --- a/docs/SEED_CONNECTIONS.md +++ b/docs/SEED_CONNECTIONS.md @@ -108,6 +108,8 @@ connections: port: 8080 # The client protocol and the web UI share this port database: hive # The CATALOG, not a database. Pins what the tree shows; # a fully qualified name still reaches any other catalog. + schema: default # The session schema for unqualified table names. Without it, + # qualify names as schema.table in every statement. user: "${TRINO_USER}" roles: ["*"] environment: production @@ -149,6 +151,7 @@ connections: | `connections[].host` | No | — | Hostname or IP | | `connections[].port` | No | — | Port number (1-65535) | | `connections[].database` | No | — | Database name (Couchbase: the bucket. Druid has one catalog and ignores it. Trino: the **catalog**) | +| `connections[].schema` | No | — | Trino session schema, used to resolve unqualified table names inside the configured catalog | | `connections[].user` | No | — | Username | | `connections[].password` | No | — | Password (use `${ENV_VAR}` syntax) | | `connections[].connectionString` | No | — | Full connection string (use `${ENV_VAR}`). Druid and Trino have no URI form this build parses — those connections need `host` and are addressed by host and port only | diff --git a/docs/providers/trino.md b/docs/providers/trino.md index 8334e071f..d83a81b4f 100644 --- a/docs/providers/trino.md +++ b/docs/providers/trino.md @@ -421,7 +421,8 @@ Rather than let a user set a session property and watch the next query ignore it attach a `QueryWarning`: > *`"SET SESSION"` succeeded, but each statement is sent on its own connection, so it will not affect -> the next one. Qualify names in full instead.* +> the next one. Set Catalog Name and Schema Name on the Trino connection for a persistent namespace, +> or qualify names in full.* The engine's own remarks travel the same way — measured, a redundant `ORDER BY` in a subquery answers with rows plus `REDUNDANT_ORDER_BY`. They are de-duplicated, because the same remark is repeated on diff --git a/src/components/ConnectionModal.tsx b/src/components/ConnectionModal.tsx index 906a9bfe6..d85464116 100644 --- a/src/components/ConnectionModal.tsx +++ b/src/components/ConnectionModal.tsx @@ -557,9 +557,12 @@ export function ConnectionModal({ {takesConnectionField(type, "schema") && (
- +
+ + +
{ const result = await provider.query("SET SESSION query_max_run_time = '10m'"); expect(result.warnings?.[0]?.message).toContain("will not affect the next one"); + expect(result.warnings?.[0]?.message).toContain("Set Catalog Name and Schema Name"); }); test("carries the engine's own remarks through", async () => { diff --git a/tests/unit/db/trino/http-transport.test.ts b/tests/unit/db/trino/http-transport.test.ts index 039ca881e..8936f7269 100644 --- a/tests/unit/db/trino/http-transport.test.ts +++ b/tests/unit/db/trino/http-transport.test.ts @@ -460,6 +460,12 @@ describe("TrinoHttpTransport request", () => { expect(firstCall().headers["X-Trino-Schema"]).toBeUndefined(); }); + test("pins the connection's schema for unqualified table names", async () => { + await makeTransport({ schema: "tiny" }).query("SELECT 1"); + + expect(firstCall().headers["X-Trino-Schema"]).toBe("tiny"); + }); + // Introspection legitimately reads a catalog other than the pinned one, and the // alternative - USE - is the session mutation this stateless transport discards. test("lets one statement override the catalog and name a schema", async () => { diff --git a/tests/unit/seed/types.test.ts b/tests/unit/seed/types.test.ts index 5fa9839e9..2568f2097 100644 --- a/tests/unit/seed/types.test.ts +++ b/tests/unit/seed/types.test.ts @@ -231,7 +231,6 @@ describe("SeedConnectionSchema: Trino's schema", () => { port: 8080, database: "memory", user: "app", - password: "s3cret", schema: "default", roles: ["*"], });