add pluggable Interpolator and ConnectionCache extension points#2
Open
odemkovych wants to merge 3 commits into
Open
add pluggable Interpolator and ConnectionCache extension points#2odemkovych wants to merge 3 commits into
odemkovych wants to merge 3 commits into
Conversation
Add the Interpolator interface and SQLDatasource.Interpolator field. A custom Interpolator owns the full rewrite call, so plugin-attached state and plugin-defined macro contexts live on the plugin's own wrapper type rather than on sqlds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a ConnectionCache extension point so plugins can install a custom
cache (TTL, LRU, …) for the per-ConnectionArgs *sql.DB pool managed by
Connector. The default (NewSyncMapCache) preserves the pre-extension
behaviour byte-for-byte.
Surface:
- CachedConnection interface (DB, Settings, Close) — exposed by three
adapter methods on the existing unexported dbConnection struct; the
struct's fields and internal access patterns are unchanged
- ConnectionCache interface (Load, Store, Range, Dispose) — non-generic;
contract documents the no-wrap requirement
- NewSyncMapCache() — default sync.Map-backed impl, no background work
- ConnectorOption + WithCache(ConnectionCache); NewConnector gains a
trailing variadic opts parameter (additive, existing call sites
compile unchanged)
- SQLDatasource.ConnectionCacheFactory func() ConnectionCache — nil
resolves to the default; the factory is invoked once per Connector
Connector.connections sync.Map is replaced by an unexported cache
ConnectionCache field. Internal accessors delegate to the cache and
type-assert Load's result back to the concrete dbConnection (safe under
the no-wrap contract; fails loudly otherwise). Nil-cache guards in the
internal accessors keep external &Connector{} test fixtures usable.
Tests cover the default impl contract, concurrent Store/Load with
-race, Dispose-closes-every-entry, factory wiring, and bootstrap entry
placement via WithCache. Adds the connection-cache spec and tasks under
openspec/changes/add-connection-cache.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Adds two extension points to
SQLDatasourceso downstream plugins can replace internal behavior without forkingsqlds:SQLDatasource.Interpolator— owns the full SQL-rewrite call. Plugins use this to install AST-aware interpolation or carry plugin-defined macro context on their own wrapper type.SQLDatasource.ConnectionCacheFactory— returns aConnectionCacheused byConnectorfor the per-ConnectionArgs*sql.DBpool. Plugins use this to install TTL/LRU caches and capture per-cache config (size cap, dependencies) by closure.Both are additive and fully backwards-compatible: nil values resolve to defaults that preserve the pre-extension behavior byte-for-byte. Existing call sites compile and run unchanged.
Motivation
We maintain a downstream Grafana data-source plugin built on
sqlds. Two needs have repeatedly pushed us toward forking:Interpolate(driver, q)free function offers no extension surface beyond the legacysqlutil.MacroFuncmap.ConnectionArgs*sql.DBentries in the internalsync.Mapforever. A TTL/LRU policy needs to replace storage, not wrap it.Rather than carry a fork, these patches expose minimal interfaces so plugins (ours and others') can swap in custom implementations.
API surface
Interpolator
SQLDatasource.Interpolatoris the assignment point;nil→DefaultInterpolator{}.ConnectionCache
SQLDatasource.ConnectionCacheFactory func() ConnectionCache— invoked once perConnector;nil→NewSyncMapCache(). AConnectorOption/WithCache(...)is exposed for directNewConnectorcallers;NewConnectorgains a trailing variadicopts ...ConnectorOptionparameter (additive).The unexported
dbConnectionstruct satisfiesCachedConnectionvia three adapter methods; its internal field-access patterns elsewhere in the package are unchanged.Contract:
Loadmust return the exact value previously passed toStore— no wrapping or decoration. Internal code type-asserts back to the concrete struct; a wrapping impl panics loudly at the call site (documented).Compatibility
NewConnector's new param is variadic — existing call sites compile unchanged.Interpolatorand nilConnectionCacheFactory, behavior is byte-for-byte identical to the pre-patch path (the default interpolator delegates tosqlutil.Interpolate;NewSyncMapCacheruns no goroutines and performs no eviction).Connector's internal accessors keep external&Connector{}test fixtures usable.Tests
interpolator_test.go— default impl parity, nil-dshandling, custom impl wiring throughhandleQuery.cache_test.go— default impl contract (Load/Store/Range/Dispose), overwrite semantics, early-stop Range, concurrent Store/Load under-race, compile-time assert thatdbConnectionsatisfiesCachedConnection.connector_cache_test.go— default cache when no option,WithCacheroutes the bootstrap entry into the custom cache,ConnectionCacheFactoryinvoked exactly once perConnector, nil factory falls back to the default, existing call sites unaffected.All existing tests pass unchanged.
Files
Test plan
go test ./... -race(covered by CI)NewConnectorcallers in dependent plugins still compile (variadic opts are additive)Interpolator, noConnectionCacheFactoryset) behave identically tomain