vtab modules reach a caller-constructed Driver but functions and collations do not

Driver holds four categories of registration state - functions, collations, connection hooks and virtual table modules - but only three of them are read off the receiver. The module path reads and writes the package-level instance instead:

  • vtab.go:99-103 - registerModule writes d.modules, the package-level d
  • vtab.go:109-110 - (*conn).registerModules ranges over d.modules, likewise
  • driver.go:192,198,204 - functions, collations and hooks are read off the receiver

The receiver in Driver.Open is itself named d and shadows the package variable, which is plausibly how the two came to differ without anyone noticing.

A caller-constructed Driver is therefore half-isolated:

package main

import (
	"database/sql"
	"database/sql/driver"
	"errors"
	"fmt"

	"modernc.org/sqlite"
	"modernc.org/sqlite/vtab"
)

type mod struct{}

func (mod) Create(vtab.Context, []string) (vtab.Table, error) {
	return nil, errors.New("MODULE REACHED")
}
func (mod) Connect(vtab.Context, []string) (vtab.Table, error) {
	return nil, errors.New("MODULE REACHED")
}

func main() {
	// Both registrations go through the package-level API.
	sqlite.MustRegisterDeterministicScalarFunction("myfunc", 0,
		func(*sqlite.FunctionContext, []driver.Value) (driver.Value, error) {
			return int64(1), nil
		})
	if err := vtab.RegisterModule(nil, "mymod", mod{}); err != nil {
		panic(err)
	}

	sql.Register("mine", &sqlite.Driver{})
	db, err := sql.Open("mine", ":memory:")
	if err != nil {
		panic(err)
	}
	defer db.Close()

	_, err = db.Exec(`SELECT myfunc()`)
	fmt.Printf("function on &sqlite.Driver{}: %v\n", err)

	_, err = db.Exec(`CREATE VIRTUAL TABLE t USING mymod()`)
	fmt.Printf("module   on &sqlite.Driver{}: %v\n", err)
}
function on &sqlite.Driver{}: SQL logic error: no such function: myfunc (1)
module   on &sqlite.Driver{}: SQL logic error: MODULE REACHED (1)

The modules field on Driver is consequently dead weight: it is only ever written and read through the package-level instance, so it is process-global state wearing a per-instance field.

Why this was documented rather than fixed

e7a39d2d describes the behavior as it stands, because both consistent readings break existing users, and unequally:

  • Inherit - have Open also apply the package-level functions and collations. Breaks silently. A registered function replaces a SQLite built-in of the same name, and RegisterScalarFunction("upper", 1) is accepted without error (the duplicate check at sqlite.go consults only d.udfs, not SQLite's built-ins). A constructed Driver that today evaluates upper(x) with SQLite's built-in would begin evaluating it with the override. Wrong results, no error, no diagnostic.
  • Isolate - have registerModules read the receiver. Breaks loudly, with no such module at CREATE VIRTUAL TABLE, for anyone registering a module globally while using a private driver. Narrower and louder, but still a regression - and full isolation is not currently reachable anyway, since vtab.RegisterModule has no per-driver form: it discards its db argument at vtab/vtab.go:317.

Proposed direction

The root problem is that Driver is half-built: four categories of state and one exported registration method, RegisterConnectionHook. Constructing one is a supported pattern - the sql.Register("mine", &Driver{...}) analogue of mattn/go-sqlite3's &SQLiteDriver{ConnectHook: ...}, which is how migrants from that driver will reach for per-connection setup - yet there is no way to populate the rest of it.

An additive fix breaks nobody:

  • (*Driver).RegisterFunction, (*Driver).RegisterScalarFunction, (*Driver).RegisterDeterministicScalarFunction, (*Driver).RegisterCollationUtf8, mirroring the package-level functions
  • a per-Driver module registration path, so vtab.RegisterModule's db argument can finally carry meaning

A constructed Driver then becomes a legitimate blank slate the caller fills in rather than a trap, and the module inconsistency can be resolved afterwards against a type that makes sense - at which point isolating is a defensible change with a deprecation period behind it instead of a silent surprise.

Follow-up from #253 (closed), where this surfaced while adding NewConnector.

Edited by cznic