DB Backend: remove uses of `basestore.NewWithDB()` (!36887) · Merge requests · Warren Gifford / sourcegraph

Merged Warren Gifford requested to merge backend-integration/cc/remove-new-with-db into main 2 years ago

Created by: camdencheek

This removes most uses of basestore.NewWithDB() from our backend. The only instances remaining are uses where the input dbutil.DB is neither a frontend DB (typed as database.DB) nor can it be easily converted to a TransactableHandle. The next step will be to migrate the remaining callers to pass around either a TransactableHandle or raw sql.DB.

The goal here is to get rid of dbutil.DB, and this the next step in a very long process. The problem with dbutil.DB is that it can be a frontend db, it can be a codeinsights db, it can be a *sql.DB, it can be a *sql.Tx, or it can be any number of semi-opaque wrappers we have for those types.

You might say "but isn't that the point of a good interface?" and I'd say "Yes, but..." and go on to talk about how dbutil.DB is a very leaky abstraction because in almost every case, we do actually care about the underlying type. We need to know whether the database handle we are using is pointing to db with the frontend schema or the code insights schema. We need to know whether the handle we're using is a transaction because transactions cannot be used concurrently. To know these things, we resort to hacky interface unwraps that break in subtle and sneaky ways.

It can get better. I promise.

Part of https://github.com/sourcegraph/sourcegraph/issues/26113 in spirit.