feat(kernel): SEA-via-kernel PuPr tail: advanced proxy, Geography, conn/error telemetry, token-caching docs, configurable backoff#412
Open
mani-mathur-arch wants to merge 10 commits into
Open
Conversation
Fill the username / password / bypass_hosts args into the kernel's set_proxy setter (the kernel path passed nil,nil,nil, carrying only the URL). These are the "advanced" fields the HTTP(S)_PROXY / NO_PROXY environment path can't express: a structured bypass list (NO_PROXY is consumed during environment resolution, not forwarded) and out-of-band basic-auth credentials (rather than embedded in the URL userinfo). - New KernelExperimentalConfig proxy fields + WithKernelProxy(url, user, pass, bypassHosts) connector option (kernel-only; the Thrift path rejects it like the other WithKernel* options). - newKernelBackend prefers an explicit WithKernelProxy over the env-var resolution — an explicit proxy is a deliberate override, and consulting both would be ambiguous. resolveKernelProxy is untagged so the explicit-over-env precedence is covered under CGO_ENABLED=0. - Kernel backend Config gains ProxyUsername / ProxyPassword / ProxyBypassHosts; applyProxy passes each as NULL when empty. Tests: experimental-field classification guard + option-wiring + Thrift-reject + DeepCopy extended for the proxy fields (untagged); resolveKernelProxy precedence (untagged); TestSetProxy drives the real cgo setter across all field combinations (tagged); live e2e TestKernelE2EProxyRejectsBadURL (routing through an unreachable proxy must fail — proof the setter is applied). Both build paths + full suite green; go vet clean. Isaac Review clean (0 valid findings). Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
#393 shipped Geometry (WKT string); Geography is the sibling geospatial type left out (split as separate subtasks). No renderer is needed: the kernel maps GEOMETRY and GEOGRAPHY to the same Arrow Utf8 shape (WKT text), distinguished only by a databricks.type_name field-metadata hint that does not change the scanned value — both hit the same string arm on the kernel and Thrift paths. So this is verify-and-close: prove the parity and pin it, no source change. - arrowscan parity test gains a geography_wkt case (kernel == Thrift == "POINT(1 2)"), documenting the shared rendering. - Live e2e TestKernelE2EDataTypes gains a geography case that SKIPs (not fails) when the warehouse rejects the type — GEOGRAPHY is gated on some warehouses, and the case documents the parity without breaking runs where it is unavailable. - doc.go lists GEOMETRY / GEOGRAPHY (WKT) in the result-type parity set. Isaac Review clean (0 actionable findings). Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…d error drop Emit connection-configuration telemetry on the kernel path (PECOBLR-3627) and stop silently dropping connection-scoped errors (PECOBLR-3628). Neither backend populated DriverConnectionParameters before: the struct existed but was never filled, and recordConnection was dead code. This is a net-new emission on top of the #399 per-statement telemetry (which already emits EXECUTE_STATEMENT / CLOSE_STATEMENT + chunk details and per-statement errors — not duplicated here). - telemetry: telemetryMetric gains connParams; createTelemetryRequest serializes it (and mirrors AuthMech onto the top-level AuthType) only for a "connection" metric — a statement/operation/error metric leaves it nil and serializes byte-identically to before (pinned by a test). Interceptor.RecordConnectionConfig replaces the dead recordConnection. - aggregator: a non-terminal error with no statement to attach to (a connection-scoped error, empty statementID) was silently dropped. It now flushes immediately instead — the connection-scoped error-log path (3628). Both backends benefit. - driver: the connector emits RecordConnectionConfig at connect, KERNEL PATH ONLY (gated on the backend type, not just WithUseKernel), so the default (Thrift) path's telemetry is byte-identical. The payload is built by untagged kernelConnectionTelemetry(cfg): mode=SEA (the closed DatabricksClientType enum has no "kernel" member — emitting it NULLs the field on ingestion), auth_mech ∈ {PAT, OAUTH} + auth_flow ∈ {CLIENT_CREDENTIALS, BROWSER_BASED_AUTHENTICATION} (the closed enums, not the driver's own auth names), proxy usage, arrow, query tags, metric-view — mirroring Python's TelemetryHelper enum mapping so all drivers land the same values. Tests: untagged builder test; serializer + "statement metric unchanged" + full-loop (RecordConnectionConfig lands; connection-scoped error not dropped) tests. Both build paths + full suite green; go vet clean. Isaac Review clean (0 actionable findings). Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…e kernel PECOBLR-3606 (token caching) and PECOBLR-3626 (client tokens / connection reuse) are verify-and-close: both are handled entirely inside the kernel, below the C ABI, so there is no driver work and no driver-side knob. Verified against kernel source: OAuth U2M tokens persist to disk (~/.config/databricks-sql-kernel/oauth/, with refresh-token lifecycle), M2M tokens cache in-memory with background refresh, and a single pooled reqwest client is reused per session across the control-plane, CloudFetch, and auth-refresh calls (pool_max_idle_per_host). doc.go now states this so the inherited behavior is documented rather than silently assumed. Isaac Review clean (0 findings). Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Honor the driver's WithRetries policy (RetryWaitMin / RetryWaitMax / RetryMax) on the kernel path by forwarding it to the kernel's new kernel_session_config_set_retry_config C-ABI setter (PECOBLR-3598). Before, WithRetries was silently ignored on the kernel path and the disable form (RetryMax < 0) was rejected at connect — the kernel had exponential backoff with configurable HttpConfig::retry_* fields but no C-ABI setter, so the caller's policy could not reach it. - kernel backend Config gains Retry *RetryConfig; applyRetry forwards it via the setter (a no-op when nil, so the kernel's default policy — 5 retries, 1s..60s, 900s budget — is preserved). - Untagged kernelRetryConfig resolves WithRetries into the descriptor: the connector's WithDefaults guarantees positive waits + RetryMax 4; a negative RetryMax (the disable form) maps to MaxRetries=0 and is honored EVEN with zero waits (WithRetries(-1,0,0) is idiomatic) by substituting a valid placeholder range the setter accepts (backoff unused with no retries). A non-disabling degenerate range returns nil (keep kernel default) so a stray zero can't fail the connect. - validateKernelConfig no longer rejects WithRetries(-1). - WithKernelRetryOverallTimeout adds the 4th knob (cumulative retry budget) — kernel-only, since the Thrift WithRetries surface has no overall-budget equivalent; mirrors the pyo3/napi retry_overall_timeout. - KERNEL_REV bumped to the kernel PR head that adds the setter (b5f6dda); re-pin to the squash-merge SHA once that kernel PR lands. cgo note: cgo silently drops the direct declaration of the retry setter (a parser quirk — valid C, compiles + links, but omitted from the Go bindings). A static inline shim in backend.go's preamble forwards to it; the shim must be in the same file as its caller (a shim in another cgo preamble in this package is dropped the same way). The kernel header is unchanged (valid C, used verbatim by the C-only ODBC consumer). Tests: untagged TestKernelRetryConfig (defaults / disable incl. zero waits / overall timeout / degenerate range); field-classification guards updated (RetryMax/Wait* now forwarded; RetryOverallTimeout classified); tagged TestSetRetry drives the real cgo setter incl. the InvalidArgument rejections; live e2e retry config / disable / overall-timeout. Both build paths + full suite green; go vet + gofmt clean. Isaac Review clean. Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
golangci-lint's gosec G101 ("potential hardcoded credentials") flagged
three non-credential string literals: the ProxyPassword test literals in
TestResolveKernelProxy / TestSetProxy, and the CLIENT_CREDENTIALS
telemetry auth_flow enum value. None is a real secret. Suppress each with
//nolint:gosec + a reason (the repo's established pattern), clearing the
Lint CI gate. gosec anchors the struct-literal G101 to the composite-lit
line, so the nolint sits there, not on the field.
Co-authored-by: Isaac <isaac@databricks.com>
Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
kernelRows implemented only driver.Rows, so database/sql fell back to ""
DatabaseTypeName and an interface{} ScanType for every column on the
kernel backend — whereas the Thrift path returns BIGINT/DECIMAL/DATE
names and int64/float64/time.Time/sql.RawBytes scan types (PECOBLR-3692).
ORMs and typed-scan tooling (GORM, sqlx, schema reflection, BI drivers)
rely on that metadata to pick Go destination types, so the omission was a
silent behavioral regression vs Thrift that value-level tests can't see.
- New shared, pure-Go arrowscan.ColumnTypeInfoFor(arrow.DataType) maps an
Arrow column type to {DatabaseTypeName, ScanType, Length} matching the
Thrift backend exactly (getScanType / GetDBTypeName / ColumnTypeLength).
It lives next to ScanCellCached and covers precisely the Arrow types the
scanner produces, so the reported type and the scanned value stay in
lockstep. Ground truth for the mapping was captured live from the Thrift
backend across all 21 supported types.
- kernelRows now implements RowsColumnTypeScanType /
DatabaseTypeName / Nullable / Length, computing per-column info once at
construction from the schema it already imports for Columns(). Nullable
returns ok=false like Thrift (no reliable per-column flag); Length
reports MaxInt64 for variable-length types and (0,false) otherwise.
- DECIMAL reports sql.RawBytes (Thrift's DECIMAL scan type) though the
value renders as an exact string; VARCHAR/CHAR/VARIANT/GEOMETRY and both
INTERVAL types collapse to STRING, matching what the prod-default Thrift
server declares (documented inline, incl. the native-interval caveat).
Why this wasn't caught: the kernel-vs-Thrift parity suites compare
scanned VALUES via sql.RawBytes, which is structurally blind to the
Rows.ColumnType* metadata (it was a missing-interface gap, not a wrong
value). This adds the guards that would have caught it: pure-Go
TestColumnTypeInfoFor / ...CoversScanner (run in the default CGO_ENABLED=0
CI build, no warehouse) and the live TestKernelThriftColumnTypeParity,
which compares full sql.ColumnType metadata across both backends for every
type. Verified: neutering the mapper makes the live test fail (empty
name / nil scan type), confirming it's a real regression guard.
Both build paths + full suite green; go vet + gofmt + golangci-lint clean.
Live kernel parity/datatype suites pass. Isaac Review clean.
Co-authored-by: Isaac <isaac@databricks.com>
Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…nel) decimalfmt.ExactString rendered every DECIMAL cell via big.Int.String() plus ~4 heap allocations, and it is on the shared render path both backends use (the Thrift arrow path's ValueString and the kernel scan path). On decimal-heavy result sets that per-cell allocation churn was a measurable throughput/RSS cost on the kernel backend vs Thrift, whose server-pre-formatted decimals arrive as zero-copy Arrow STRING (PECOBLR-3691). Rewrite the renderer to allocate only the returned string: - render the unscaled magnitude right-to-left into a stack scratch and place the decimal point by slice copy, with math/big only as the fallback for a true 128-bit magnitude (hi != 0) — which every DECIMAL(<=18), and every DECIMAL value under 2^64, never reaches. - factor an exported Append(dst, n, scale) so a caller can render into a reused buffer with zero heap allocation (byte-identical to ExactString). Output is unchanged: this is a cost-only rewrite of a value on the prod Thrift path, so it must be byte-for-byte identical for every input. The guard is TestExactStringOracleParity, which keeps the pre-rewrite implementation verbatim as an oracle and fuzzes the two against each other across both signs, the full scale range, the uint64/128-bit boundary, and the DECIMAL(38) / 2^127-1 extremes. Micro-bench: 38.9ns/18B/1alloc -> ~12ns/0B/0alloc; Append into a reused buffer is ~9ns/0alloc. The kernel scan path continues to render top-level decimals through this shared renderer (one string per cell, now alloc-cheaper); the further zero-alloc batch-arena optimization is deliberately deferred to a follow-up so this first kernel release carries no unsafe. Also adds the live TestKernelThriftDecimalScaleParity (50k rows spanning multiple kernel batches, incl. a DECIMAL(38,4) past float64 range) to pin the integrated kernel scan path byte-identical to Thrift at scale. Both build paths + full suite green; go vet + gofmt + golangci-lint clean. Live kernel parity/datatype suites pass. Isaac Review clean. Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…sInMemory Give callers a way to bound the kernel's peak RSS on large result sets (PECOBLR-3691, the RSS half of the decimal-heavy launch risk). The kernel holds cloudfetch_max_chunks_in_memory (default 16) decompressed CloudFetch chunks at once; on wide, row-heavy results that drives ~1.2 GB peak, vs Thrift's flat ~+200 MB. Lowering the bound trades a little throughput for a large RSS reduction (measured: chunks=4 -> ~265 MB vs chunks=32 -> ~856 MB peak on a 3M-row query, a 3.2x swing driven purely by the knob). - New EXPERIMENTAL kernel-only option WithKernelMaxChunksInMemory(n), carried on KernelExperimentalConfig.MaxChunksInMemory. A value <= 0 (the default) leaves the kernel's built-in default (16) untouched. - buildKernelConfig injects it into the KERNEL backend's own SessionConf as the client-only "cloudfetch_max_chunks_in_memory" key (config.KernelMaxChunksInMemoryConfKey) — NOT via EffectiveSessionParams, which both backends share, so it can never leak onto the Thrift path or the server. The kernel (PR that pairs with this KERNEL_REV bump) reads the key in Session::open, folds it into its result config, and strips it before the SEA wire; the key is absent from the kernel's server allowlist so it never becomes a server SET param. - No new C-ABI symbol: it rides the existing kernel_session_config_set_session_conf setter. - KERNEL_REV bumped to the kernel head that adds the override reader; re-pin to the squash-merge SHA once that kernel PR lands. Tests: the experimental-field classification guard gains the new field (so it can't be silently dropped); option->config wiring + Thrift-reject + DeepCopy cases; buildKernelConfig subtests assert the key is injected when set, absent when unset/zero, and NOT present in EffectiveSessionParams (the kernel-only invariant); live TestKernelE2EMaxChunksInMemory drains a 1M-row CloudFetch result with the bound lowered (proves the key is accepted by the kernel and stripped before the wire — a bad key would error server-side). Both build paths + full suite green; go vet + gofmt + golangci-lint clean. Isaac Review clean. Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Collaborator
Author
Code Review Squad — ReviewScore: 85/100 — MODERATE RISK Large additive PR closing out the SEA-via-kernel new-items backlog (~8 features). The high-risk internals are solid: the alloc-free decimal renderer is correct at the uint64↔128-bit boundary (benchmarked 12.4ns/0-alloc), carries no injection surface, and proxy credentials/tokens never reach logs or telemetry — no Critical and no security findings. Remaining findings cluster around one real correctness gap (WithRetries silently ignored on a user-reachable input) plus advertised-but-thin items (a stale rejection message, a telemetry fix targeting a currently-dead path) and documentation/test-coverage polish. A separately-tracked KERNEL_REV re-pin (acknowledged by the author, to be fixed before finalizing) was dismissed from this review. All findings verified against head SHA 1cff39c. Medium Findings[Medium] Thrift-path rejection message names a stale 2-option subset — (Posted here rather than inline: the cited lines are pre-existing code not present in this PR's diff, so GitHub can't anchor an inline comment there.) The rejection error hardcodes Suggested fixGeneralize the message to "a WithKernel* option" (drop the specific list) or enumerate all five current options; update the guarding comment (connector.go:48-52) to drop "TLS." Posted by code-review-squad · |
…, telemetry framing, coltype parity Address the 9 findings from the PR #412 code review (Isaac Review clean on the result). One real correctness fix (WithRetries silently ignored on a user-reachable input) plus advertised-but-thin items and doc/test polish. - kernel_config: WithRetries(n, 0, 0) now forwards the caller's RetryMax on the kernel path instead of dropping to the kernel's default policy. Because WithDefaults() runs before options, WithRetries(n, 0, 0) — valid per its godoc — overwrote the waits to zero and hit the degenerate-range guard, so the caller's attempt count was silently ignored (a cross-backend divergence: the Thrift path honored the same input). It now substitutes placeholder waits while keeping RetryMax; only a zero-value range with no attempt count falls back to the kernel default. New WithRetries(n,0,0) test; the disable constants are renamed kernelRetryPlaceholderWait* since both paths use them. - connector: the Thrift-path rejection message named only the two TLS options, but all five WithKernel* options trip the same gate — a caller who set WithKernelProxy and forgot WithUseKernel was misdirected. Generalized to name the WithKernel* family; the test now guards against a stale subset. - telemetry/aggregator: reframed the "error" metric branch (and its test) as defensive — no exported API emits a standalone "error" metric today, so the branch is unreachable from production and guards routing for a future producer, rather than being an active connection-scoped-error fix. - arrowscan/coltype: added UINT8/16/32/64 -> BIGINT/int64 (matching ScanCellCached's int64 widening), so the scanner-coverage guard no longer silently omits them; fixed a doc comment that named a nonexistent guard test. - internal/rows: new pure-Go TestColumnTypeInfoMatchesThriftMapping cross-checks ColumnTypeInfoFor against the Thrift getScanType / GetDBTypeName / length logic, so a Thrift-mapping drift (e.g. DECIMAL's scan type) fails in CGO_ENABLED=0 PR CI instead of only the warehouse-gated nightly parity test. - doc.go: added WithKernelMaxChunksInMemory to the experimental-options catalog. - kernel_newitems_e2e: new TestKernelE2EProxyForwardsCredentials routes through a local CONNECT proxy and asserts the captured Proxy-Authorization decodes to exactly user:pass (a slot swap would show pass:user) — the credential/bypass forwarding TestSetProxy couldn't cover against the opaque kernel C config. - dropped history-reference comments that described pre-setter behavior. Both build paths green (CGO_ENABLED=0 go test ./... + CGO_ENABLED=1 go test -tags databricks_kernel ./...), go vet clean both paths, gofmt clean, golangci-lint 0 issues on the changed files. Co-authored-by: Isaac <isaac@databricks.com> Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
changed the title
Collaborator
Author
|
Fixed in 29edbba (the last remaining Medium — the stale rejection message at All five The guarding comment was de-scoped from "TLS" to cover the trusted-CA bundle, hostname-verify skip, proxy, retry budget, and CloudFetch chunk cap. With this, all 9 findings from the review are addressed; a follow-up |
mani-mathur-arch
marked this pull request as ready for review
July 19, 2026 10:39
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
1 participant
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.
The genuinely-new SEA-via-kernel driver work from the new-items backlog: the short PuPr telemetry/config/Geography tail plus advanced proxy and configurable backoff, followed by a datatype-parity + decimal-perf + RSS pass. Each is re-implemented clean on
main(post-#399) — the pre-#399 POC was the spec, not the source. Every commit was reviewed with Isaac Review before commit.Commits
PuPr new-items tail
feat(kernel): advanced HTTP proxy via WithKernelProxy(PECOBLR-3607) — fill the username / password / bypass_hosts args into the kernel'sset_proxysetter (the kernel path passed the URL only). NewWithKernelProxy(url, user, pass, bypassHosts)connector option; explicit proxy takes precedence over the env-var path.test(kernel): cover Geography as the WKT sibling of Geometry(PECOBLR-3620) — verify-and-close: GEOGRAPHY maps to the same Arrow Utf8/WKT shape as GEOMETRY (no renderer needed). Parity + live-e2e (skip-on-reject) coverage.feat(kernel): emit connection-config telemetry + fix connection-scoped error drop(PECOBLR-3627 / 3628) — emitDriverConnectionParametersat connect (kernel-path only;mode=SEA,auth_mech/auth_flowclosed enums, proxy/arrow/query-tags/metric-view). Additive on top of feat(kernel): consolidated DAIS gap-closure + PuPr feature set + nightly workflows for e2e tests #399's per-statement telemetry (not duplicated). The aggregator's"error"metric branch is routed correctly (a connection-scoped, no-statement error flushes standalone rather than being dropped), but it is defensive: no exported API emits a standalone"error"metric today, so the branch guards routing for a future producer rather than changing observable behavior now (clarified in review — see below).docs(kernel): note token caching + client reuse are inherited from the kernel(PECOBLR-3606 / 3626) — verify-and-close, doc-only: both are handled inside the kernel below the C ABI.feat(kernel): configurable HTTP retry / backoff(PECOBLR-3598) — wireWithRetries(RetryWaitMin/Max/RetryMax, incl. the disable form) into the kernel's newset_retry_configC-ABI setter, plus a kernel-onlyWithKernelRetryOverallTimeout.Datatype parity + decimal perf + RSS
feat(kernel): report column-type metadata on the kernel Rows path(PECOBLR-3692) —kernelRowsimplemented onlydriver.Rows, sodatabase/sqlfell back to""DatabaseTypeName /interface{}ScanType for every column vs Thrift's typed metadata (a silent regression for ORMs / typed-scan tooling that value-level tests can't see). New shared pure-Goarrowscan.ColumnTypeInfoFormaps Arrow →{DatabaseTypeName, ScanType, Length}matching Thrift for all 21 types;kernelRowsnow implements the 4 optionalRowsColumnType*interfaces. Pure-Go guard + liveTestKernelThriftColumnTypeParity.perf(decimal): alloc-free exact decimal renderer (shared Thrift + kernel)(PECOBLR-3691) — rewritedecimalfmt.ExactString(on the shared render path) to allocate only the returned string: uint64 fast path renders into a stack scratch,big.Intonly for true 128-bit magnitudes. Byte-identical output, pinned by an oracle-parity test (old impl kept verbatim as reference); 38.9ns/18B/1alloc → ~12ns/0B/0alloc. Live 50k-row multi-batch parity vs Thrift.feat(kernel): tune CloudFetch in-memory chunks via WithKernelMaxChunksInMemory(PECOBLR-3691, RSS) — bound how many decompressed CloudFetch chunks the kernel holds in memory (default 16), the knob that trades throughput for peak RSS on large results (measured 265 MB @ chunks=4 vs 856 MB @ chunks=32 on a 3M-row query). Kernel-only option forwarded as the client-onlycloudfetch_max_chunks_in_memorysession conf, which the kernel folds into its result config and strips before the SEA wire — never reaching the server or the Thrift path. No new C-ABI symbol.Kernel dependency
Two commits need new kernel behavior on the same kernel branch (PR #178):
set_retry_configC-ABI symbol (backoff commit), andapply_client_result_overridesreader that appliescloudfetch_max_chunks_in_memory(chunk-knob commit).KERNEL_REVis pinned to that branch's head (ffa73c0), so the tagged CI links both. Re-pin to the squash-merge SHA once the kernel PR lands. The other commits are driver-only and need no kernel change.Testing
Both build paths green (
CGO_ENABLED=0 go test ./...+CGO_ENABLED=1 go test -tags databricks_kernel ./...),go vetclean, gofmt clean,golangci-lint run ./...0 issues. Pure-Go tests (option wiring, field-classification guards, ColumnType mapper, decimal oracle-parity, retry/proxy/telemetry resolvers, telemetry full-loop) run underCGO_ENABLED=0; tagged tests drive the real cgo setters; live e2e tests (proxy-rejects-bad-URL, proxy-credential-forwarding, geography, retry config/disable/overall-timeout, ColumnType parity, 50k-row decimal parity, CloudFetch chunk-knob drain) run against the staging warehouse. Each commit was reviewed with Isaac Review and its findings addressed before commit.Review findings addressed
fix(kernel): address code-review findings(29edbba) resolves all 9 findings from the code review; a follow-upisaac reviewcame back clean (0 posted comments). All threads are resolved.WithRetries(N, 0, 0)silently reverted to the kernel default —kernelRetryConfignow forwards the caller'sRetryMax(with placeholder waits) on a degenerate range instead of returningnil. BecauseWithDefaults()runs before options,WithRetries(n, 0, 0)zeroed the waits and dropped the attempt count to the kernel's 5-retry default — a silent cross-backend divergence from Thrift. Only a zero-value range with no attempt count still falls back to the kernel default. New regression subtest.WithKernel*options trip the same gate; generalized to name theWithKernel*family, and the test now guards against a stale subset."error"branch and its test as defensive (no current producer), rather than an active behavior change.UINT8/16/32/64 → BIGINT/int64toColumnTypeInfoFor(matching the scanner's int64 widening) and to both guard tests.WithKernelMaxChunksInMemoryabsent from thedoc.gocatalog — added the bullet.TestColumnTypeInfoMatchesThriftMappingcross-checksColumnTypeInfoForagainst the ThriftgetScanType/GetDBTypeName/ length logic, so drift fails inCGO_ENABLED=0PR CI (not just the warehouse-gated nightly).TestKernelE2EProxyForwardsCredentialscaptures theProxy-Authorizationa local CONNECT proxy sees and asserts it decodes to exactlyuser:pass(a slot swap would showpass:user).This pull request and its description were written by Isaac.