Experimental postgres query command (PR 1/4: scaffold)#5135
Open
simonfaltum wants to merge 6 commits intomainfrom
Open
Experimental postgres query command (PR 1/4: scaffold)#5135simonfaltum wants to merge 6 commits intomainfrom
simonfaltum wants to merge 6 commits intomainfrom
Conversation
Move the Postgres autoscaling and provisioned target-resolution helpers out of cmd/psql/ into a shared package so a second consumer (the new experimental postgres query command, in a follow-up commit) can reuse the same SDK shapes. cmd/psql keeps its interactive UX by wrapping the shared AutoSelect* helpers with errors.As fallbacks on AmbiguousError. No behavior change for cmd/psql; existing acceptance tests pass. Co-authored-by: Isaac
Scaffolds 'databricks experimental postgres query', a scriptable SQL
runner against a Lakebase Postgres autoscaling endpoint that does not
require a system psql binary.
This PR ships the smallest useful slice:
- Single positional SQL statement.
- --target (autoscaling resource path), --project, --branch,
--endpoint targeting forms; provisioned-shaped targets return a
pointer at 'databricks psql' for now.
- Connect retry on idle/waking endpoints (08xxx SQLSTATE family,
dial errors).
- Text output (static table for rows-producing statements, command
tag for command-only).
Provisioned support, JSON/CSV streaming output, multi-statement input,
cancellation, and integration tests come in follow-up PRs.
Driver: github.com/jackc/pgx/v5 v5.9.1 (MIT). Already a direct dep of
the universe monorepo's Lakebase services; aligning here keeps the SDK
surface consistent.
Co-authored-by: Isaac
- Replace exported PathSegmentProjects/ExtractID with focused helpers (ProjectIDFromName/BranchIDFromName/EndpointIDFromName); keeps SDK literals out of call sites. - Type AmbiguousError.Kind as a typed enum (KindProject/Branch/Endpoint/Instance) so producers and the pluralisation switch stay in sync. - Stop setting Choice.DisplayName when it equals the ID; Error() relies on empty-suppression rather than mixed empty/equal-to-ID checks. - Add 57P03 (cannot_connect_now) to the connect-retry allow-list. Postgres emits this during server startup and Lakebase autoscaling can plausibly return it during the wake-up handshake. Tests exercise 57P03/57P01/57014 to lock the boundary. - Require --branch when --endpoint is set. The auto-select-then-look-up flow produces confusing errors when the auto-selected branch does not contain the requested endpoint, and this command is non-interactive so asking the user to be explicit is friendlier. - Reject --max-retries < 1 explicitly instead of silently clamping. Help text already advertised the constraint; matching it at validation time is consistent with the repo's "reject incompatible inputs early" rule. - Harmonise the "endpoint is not ready" error in cmd/psql to include the state, matching the experimental command and giving operators something to act on. - Restore comments removed during the cmd/psql refactor and add a breadcrumb at the GetProvisioned call site about the Name patch. - Add doc comments to AutoSelect* helpers documenting the returned string shape (trailing ID for autoscaling vs full name for provisioned). - Reject trailing components after endpoint in ParseAutoscalingPath; new acceptance test in cmd/psql exercises this. - Drop dead GroupID: "" assignment. Co-authored-by: Isaac
- Fix selectAmbiguous: fall back to ID when DisplayName is empty. Round-1 fix to Choice semantics left producers emitting empty DisplayName for branches/endpoints/instances; the psql interactive selector passed that straight to cmdio.Tuple.Name and rendered blank rows. Add the documented fallback. - Drop unused BranchIDFromName / EndpointIDFromName exports; only ProjectIDFromName has callers in this PR. Re-add when first consumed. - Convert chained ifs in isRetryableConnectError to a switch. Co-authored-by: Isaac
…imental Two reductions for an experimental command, per a maintainer comment: - Move libs/lakebase/target into experimental/postgres/cmd/internal/target so the experiment is self-contained. cmd/psql is no longer touched (no refactor, no behavior change). When/if this command graduates from experimental, that's the right time to extract the shared package. - Drop acceptance tests for the new command. Aitools (the other experimental command) has none either; locking down user-visible wording for an experimental surface is overinvestment. Unit tests still cover argument validation, retry classification, and rendering. Acceptance tests can be added when the command graduates. Net diff on cmd/psql is now zero. The experiment lives entirely under experimental/postgres/cmd/. Co-authored-by: Isaac
Contributor
|
Tab and newline characters in cell values corrupt text-mode output
🔍 Reviewed by nitpicker |
arsenyinfo
approved these changes
Apr 30, 2026
This PR only uses autoscaling targeting; provisioned helpers in internal/target/provisioned.go have no caller in PR 1's net diff, which the task deadcode check (run by CI's lint job, not lint-q) correctly flags. Provisioned support lands in PR 2; the necessary subset of helpers (GetProvisioned, ProvisionedCredential) is added there alongside the first caller. Co-authored-by: Isaac
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.
PR Stack
Why
Talking to Lakebase Postgres from a script today goes through one of two unpleasant paths:
databricks psql -- -c "SQL". Works on macOS / Linux when psql is installed. Fails on Windows 11 by default and on minimal containers / sandboxed CI. No JSON / CSV.psycopg. Forces every consumer to manage OAuth tokens, SSL mode, autocommit, etc.This series adds a third path: a native CLI command that runs SQL against any Lakebase endpoint, returns results in text/JSON/CSV (later PRs), and works without a system psql.
databricks psqlkeeps owning the interactive REPL surface; this PR does not touch psql.Changes
Before: No CLI command runs SQL against Lakebase from Go. Users either shell out to
psql(requires the system binary) or writepsycopgglue.Now:
databricks experimental postgres query --target projects/foo/branches/main/endpoints/primary "SELECT 1"returns a text-rendered result. Provisioned instances and richer output formats land in follow-up PRs.The experimental command is fully contained under
experimental/postgres/cmd/:experimental/postgres/cmd/cmd.go,query.go,targeting.go,connect.go,execute.go,render.go— command implementation.experimental/postgres/cmd/internal/target/— Lakebase target resolution helpers (parsing, SDK wrappers, auto-select-when-exactly-one). Internal sub-package so it can't accidentally be imported from outside the experiment. When/if this command graduates from experimental, that's the right time to consider extracting tolibs/.Single positional SQL, autoscaling targeting only (
--target,--project,--branch,--endpoint),--max-retries,--connect-timeout,--database. Driver isgithub.com/jackc/pgx/v5 v5.9.1(MIT). Connect retry uses a typed predicate (08xxx SQLSTATE family +57P03cannot_connect_now +net.OpErrorwithOp == "dial"); auth (28xxx) and permission (42501) errors do not retry. Text rendering is buffered (no streaming yet); rows-producing vs command-only is decided at runtime viaFieldDescriptions().Outside the experimental tree, this PR only:
cmd/experimental/experimental.go(2 lines).go.modSPDX annotation,NOTICEentry,NEXT_CHANGELOG.mddependency-updates entry).pgxis already a direct dep of the universe monorepo's Lakebase services; aligning here keeps the SDK surface consistent.Test plan
go test ./experimental/postgres/...(target parsing, validateTargeting, retry classification, render)go test ./internal/build/...(license + NOTICE completeness)go tool ... golangci-lint run ./experimental/postgres/...(0 issues)./task checks(whitespace, links, deadcode)