feat: accept an org's Trino catalog name as a logical catalog alias - #1177
Merged
Conversation
A Duckgres session may now connect with `database=org_<database_name>` — the same catalog name the org has on Trino — and get the physical DuckLake catalog under that name. current_database(), pg_database, information_schema, three-part references, and `USE` all answer to it. The point is SQLMesh: it sees ONE catalog name on both the Duckgres and Trino engines, so moving a project between engines needs no state rewrite. PR #651's invariant is preserved. The startup `database` is still never used to find, select, or route to an org. Identity stays SNI-only, and the alias is validated AGAINST the org SNI has already resolved — it is compared to that org's own catalog name, never used as a lookup key. A sibling tenant's catalog name is just another unrecognized string and fails closed, exactly as today. Sessions connecting with "ducklake" or nothing are unchanged; the alias is opt-in per connection. The name derivation moves to configstore (untagged) so every build has it; provisioner.TrinoCatalogName now delegates to that one definition. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
logical_catalog_alias connects the cnpg tenant with its own Trino catalog name and asserts the alias renames without redirecting: current_database() and pg_database report it, a three-part reference and `USE <alias>` reach the real catalog, and a session connected the ordinary way sees the same row. It also asserts the security half — a sibling tenant's catalog name and an arbitrary name both get 3D000. Docs: CLAUDE.md gains the alias contract and the PR #651 invariant that governs it; the harness path references are corrected to the directory that exists (tests/mw-dev/e2e/, not tests/e2e-mw-dev/). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
`requestedCatalog != ""` cannot be false where it sits — an empty startup database already set CatalogValid — and `c.database != ""` is subsumed by the EqualFold against a non-empty USE target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
Test Impact PlanDeterministic summary of how this PR changes tests, CI runners, and coverage-risk signals. Summary
Signals
Coverage risk: neutral or increased No coverage-reduction warnings detected. |
dcb043d dropped `c.database != ""` on the claim that it was "subsumed by the EqualFold against a non-empty USE target". That claim was wrong: the USE target is only checked for emptiness BEFORE quote-stripping, so `USE ""` passes that check as the two-character `""` and then unquotes to "". A session whose database is unset then matched its own empty name, and invalid SQL was silently rewritten into `USE ducklake.main` instead of passing through to DuckDB and erroring. No cross-tenant exposure — it resolves to the catalog the session already executes against — but an invalid statement must not silently succeed. The other guard that commit dropped (`requestedCatalog != ""` in store.go) is genuinely unreachable and stays dropped. TestRewriteDirectQueryEmptyQuotedUseIsNotAnAlias pins it at the rewrite entry point, plus an `USE ""` case in the alias table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
The e2e assertion batched `USE <alias>; SELECT ...` into one psql -c, which duckgres cannot split. handleQuery splits a multi-statement simple query only when pg_query parses it (conn.go: `parseErr == nil && len(tree.Stmts) > 1`), and `USE` is not PostgreSQL syntax — so the batch reached rewriteDirectQuery whole, its USE target was `<alias>; SELECT ...` (matching no catalog name, so correctly left alone), and DuckDB split it and failed the bare USE. The product code was right; the assertion was written in a shape the simple query protocol does not support here. `USE ducklake; SELECT ...` fails the same way on main, so this is pre-existing and not alias-specific. pg_script feeds a script on stdin, where psql sends each statement as its own simple-query message on ONE session. Verified against a local standalone server: state set by the first statement is visible to the third, on one pid. Tests: TestLogicalCatalogAliasThroughConnectionSetup drives the alias through the REAL setup path — NewClientConn plus the exported setters control.go calls, in order, including the post-worker-switch replay — for both the simple and the extended query composition. That closes the gap that let this reach CI: the old tests hand-built a clientConn and never exercised how the session fields get populated. TestUseStatementIsNeverSplitOutOfASimpleQueryBatch pins the batching limitation so the next harness author does not rediscover it, and fails loudly if pg_query ever learns to parse USE. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
`SET search_path = '<alias>.main'` is not rewritten and fails on the worker, while the physical `'ducklake.main'` works. The catalog name sits in a string literal rather than a RangeVar, so LogicalCatalogTransform has nothing to match and the USE pass does not look at SET. Deliberately left alone: SQLMesh selects a catalog with `USE <catalog>` as its own statement, and touches search_path only in dbt code it marks unsupported. Also record in the harness that project_reader_isolation already issues its USE as a separate message (psql -c/-c) and is not a latent failure, and that the retry case list now has four copies worth folding together later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A Duckgres session can now connect with
database=org_<database_name>— the same catalog name the org already has on Trino — and get the physical DuckLake catalog under that name.ducklakestays the physical ATTACH alias; the new name is a logical alias, not a second attachment.The point is SQLMesh. It has to see ONE catalog name on both the Duckgres and the Trino engine, so moving a project between engines needs no state rewrite.
Opt-in per connection. A session that connects with
ducklakeor with no database is byte-for-byte unchanged.How PR #651's invariant is preserved
This is the section to read first. #651 made the startup
databaseparameter pure catalog selection, not identity: identity comes from the managed hostname (SNI) only, and "there is no logical-name masking, so an arbitrary name no longer routes anywhere." All of that still holds.ResolvePostgresConnectionstill resolves the org fromresolveSNIPrefixFromSnapshot(snapshot, sniPrefix)and nothing else.requestedCatalog == TrinoCatalogName(databaseName), wheredatabaseNameis the third return value of that same SNI resolution — the org SNI has already authenticated. It answers "is this the catalog name of the org I already have?", never "which org owns this name?". It is not a key intoDatabaseOrg,Orgs, or any other map."",ducklake, or this org's own catalog name is refused with the same 3D000 as before. A sibling tenant's catalog name is just another unrecognized string here; it is refused, not routed. A code comment at the check says so, in the terms above.EffectiveCatalogstaysducklakeand remains what every statement executes against. The alias only changes the name reported on the wire.Two consequences worth stating plainly:
database_names that predateValidateDatabaseName. Two such orgs could derive the same catalog name — but each session still resolves to its own SNI-authenticated org and its own catalog, so a convergent name grants no access to anything. It is a display collision, not a routing one. (The Trino side separately holds those orgs back viarejectPrincipalCollisions.)database_name, which every org has. This is deliberate: a project can target the engine-agnostic name before Trino is provisioned.How it works
EffectiveCatalog— always the physicalducklakeLogicalCatalog/sessionMetadataResult.visibleCatalogvisibleCatalogName(logical, effective)is the single place that picks between them. It feeds:sessionmeta.InitSessionDatabaseMetadataWithAccess, socurrent_database(),pg_database, and theinformation_schemaviews report the logical name. Those surfaces were already fully parameterized on a caller-supplied name, and their compat views already mapducklake/memorytocurrent_database()— nothing there needed changing.clientConn.database, so the transpiler's existingLogicalCatalogTransformrewritesorg_x.public.ttoducklake.main.t.rewriteDirectQueryalso learns the alias, soUSE <alias>expands toducklake.mainthe wayUSE ducklakedoes. Without it, a client that only ever sees the alias could not switch to its own catalog by name.TrinoCatalogNamemoves fromprovisioner(kubernetes-tagged, and it importsconfigstore, so the dependency could not go the other way) intoconfigstore, where every build can reach it.provisioner.TrinoCatalogNameandtrinoSanitizenow delegate to that one definition, so the three-sided pin between this function,opa.ManagedCatalogPattern, andpolicy.regois unchanged.Passthrough sessions get the catalog but not the renaming — they skip the PG compat layer entirely by design, so there is nothing to rename. Unchanged behavior.
Tests
configstore:TestResolvePostgresConnectionLogicalCatalog— the three accepted values, case/space normalization, an arbitrary name, another org's catalog name, no-managed-SNI, and an unknown hostname.controlplane:TestVisibleCatalogName.server:TestInitSessionDatabaseMetadataReportsLogicalCatalogAlias(real DuckDB: every pg surface reports the alias, and unqualified DDL still lands inducklake),TestRewriteDirectQueryLogicalCatalogAlias,TestNewTranspilerRewritesLogicalCatalogAlias.logical_catalog_aliasintests/mw-dev/e2e/harness.sh— connects the cnpg tenant as its own catalog name, asserts the pg surfaces, a three-part write/read,USE <alias>, that an ordinaryducklakesession sees the same row, and that a sibling tenant's catalog name and an arbitrary name both get 3D000.go buildclean with and without-tags kubernetes;golangci-lint runreports nothing new (the twonode_modulesgovet hits are pre-existing). The Postgres-backed and docker-compose test lanes could not run locally — a stale local test database and unavailable docker networking, both reproduced onorigin/main— so CI is the verdict there.Docs
CLAUDE.mdgains the alias contract and the #651 invariant that governs it. Drive-by: the e2e harness path inCLAUDE.md,controlplane/admin/README.md, andcmd/cache-proxy/README.mdpointed attests/e2e-mw-dev/, which does not exist — corrected totests/mw-dev/e2e/.🤖 Generated with Claude Code
https://claude.ai/code/session_01WCY5Jf2BQPCVKJTZU1TpEe