Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,57 @@
"database/sql"
"fmt"
"strings"
"sync"

// Database drivers would be imported here:
// _ "github.com/lib/pq" // PostgreSQL driver
// _ "github.com/go-sql-driver/mysql" // MySQL driver
// _ "github.com/mattn/go-sqlite3" // SQLite driver

"github.com/jackc/pgx/v5/stdlib"
"github.com/opentdf/platform/service/entityresolution/multi-strategy/types"
)

var (
// driverRegMu guards lazy driver registration to prevent duplicate-register panics.
driverRegMu sync.Mutex
registeredDrivers = make(map[string]struct{})
)

// ensureDriverRegistered lazily registers the named database/sql driver the first
// time a SQL provider for that driver is created. This avoids the need for
// consumers to add blank driver imports to their own binaries.
//
// Uses pgx/v5/stdlib for postgres (already a platform dependency). Other drivers
// (mysql, sqlite) are not currently auto-registered and must be imported by the
// consumer. Consumers that have already registered the driver themselves are
// handled gracefully via a sql.Drivers() pre-check.
func ensureDriverRegistered(driver string) {
// Normalize to lowercase so "Postgres", "POSTGRES", and "postgres" all resolve
// to the same registered driver name. sql.Register is case-sensitive.
driver = strings.ToLower(strings.TrimSpace(driver))
Comment thread
coderabbitai[bot] marked this conversation as resolved.

driverRegMu.Lock()
defer driverRegMu.Unlock()

if _, ok := registeredDrivers[driver]; ok {
return
}

// Check whether the driver was already registered externally (e.g. via a
// blank import in the consumer binary) before attempting to register it.
// Use strings.EqualFold so the pre-check is also case-insensitive.
for _, d := range sql.Drivers() {
if strings.EqualFold(d, driver) {
registeredDrivers[driver] = struct{}{}
return
}
}

switch driver {

Check failure on line 50 in service/entityresolution/multi-strategy/providers/sql/sql_provider.go

View workflow job for this annotation

GitHub Actions / go (service)

singleCaseSwitch: should rewrite switch statement to if statement (gocritic)
case "postgres":
sql.Register("postgres", stdlib.GetDefaultDriver())
registeredDrivers[driver] = struct{}{}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
// mysql and sqlite require imports not present in this module's dependencies.
// Add cases here when those drivers are added to go.mod.
}

// Provider implements the Provider interface for SQL databases
type Provider struct {
name string
Expand All @@ -24,6 +66,15 @@

// NewProvider creates a new SQL provider
func NewProvider(ctx context.Context, name string, config Config) (*Provider, error) {
// Normalize the driver name so "Postgres", "POSTGRES", and "postgres" all
// resolve correctly through ensureDriverRegistered and sql.Open, both of
// which use case-sensitive driver name matching.
config.Driver = strings.ToLower(strings.TrimSpace(config.Driver))

// Register the database/sql driver for this provider's configured driver name
// if it has not already been registered.
ensureDriverRegistered(config.Driver)
Comment on lines 68 to +76
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If config.Driver is provided with mixed casing (e.g., "Postgres" or "POSTGRES"), it can lead to two issues:

  1. Panic in ensureDriverRegistered: If "postgres" is already registered externally (e.g., via lib/pq in the consumer binary), the case-sensitive check d == driver (where d is "postgres" and driver is "Postgres") will fail. The function will then proceed to call sql.Register("postgres", ...) which panics because the driver is already registered.
  2. Unknown driver error in sql.Open: sql.Open(config.Driver, ...) will be called with "Postgres", which will fail because the registered driver name is lowercase "postgres".

Normalizing config.Driver to lowercase at the beginning of NewProvider resolves both issues cleanly.

Suggested change
func NewProvider(ctx context.Context, name string, config Config) (*Provider, error) {
// Register the database/sql driver for this provider's configured driver name
// if it has not already been registered.
ensureDriverRegistered(config.Driver)
func NewProvider(ctx context.Context, name string, config Config) (*Provider, error) {
config.Driver = strings.ToLower(config.Driver)
// Register the database/sql driver for this provider's configured driver name
// if it has not already been registered.
ensureDriverRegistered(config.Driver)


provider := &Provider{
name: name,
config: config,
Expand Down
Loading