Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic PRAGMA URI parameters #1248

Open
ncruces opened this issue Jun 8, 2024 · 11 comments
Open

Generic PRAGMA URI parameters #1248

ncruces opened this issue Jun 8, 2024 · 11 comments

Comments

@ncruces
Copy link

ncruces commented Jun 8, 2024

Other drivers, in particular modernc and ncruces support a generic _pragma URI parameter.

Basically, potentially multiple PRAGMA statements are executed in the order specified in the URI for every connection opened.

Example:

sql.Open("sqlite3", "file:demo.db?_pragma=busy_timeout(10000)&_pragma=foreign_keys(on)")

This leads to the following SQL being executed:

PRAGMA busy_timeout(10000);
PRAGMA foreing_keys(on);

The values of the _pragma params are reproduced literally after URL decoding. Using the parenthesis form of PRAGMA is just a convenience for readability (_pragma=busy_timeout%3D10000 would be just as valid).

These would serve the same purpose as many of the custom parameters this driver specifies (like _fk), but would be more portable across drivers, allowing libraries like GORM, goqite or redka to be driver agnostic and customize their connections in a driver agnostic way.

@rittneje
Copy link
Collaborator

rittneje commented Jul 1, 2024

Can you clarify the workflow here? Where are these query parameters coming from in the case of libraries like GORM? Inside the library itself, or from the user of the library?

@ncruces
Copy link
Author

ncruces commented Jul 1, 2024

From inside the library.

See this for a good example.

The default pragmas used are things like:

"journal_mode": "wal",
"synchronous":  "normal",
"temp_store":   "memory",
"mmap_size":    "268435456",
"foreign_keys": "on",

@rittneje
Copy link
Collaborator

rittneje commented Jul 1, 2024

I don't really understand how that works. Presumably the user of the library still has to provide the URI containing the database path. But then, if they've also included the non-standard query parameters for this or any other driver, then the library's own defaults would counteract the user's intent.

And in the specific case of the library you linked, it takes a pre-created sql.DB instance. So I don't understand how the library being able to generically provide pragmas in the query string would help.

@ncruces ncruces changed the title Generic PRAMA URI parameters Generic PRAGMA URI parameters Jul 1, 2024
@ncruces
Copy link
Author

ncruces commented Jul 1, 2024

I don't really understand how that works. Presumably the user of the library still has to provide the URI containing the database path. But then, if they've also included the non-standard query parameters for this or any other driver, then the library's own defaults would counteract the user's intent.

Yes, that happens. But if a library that's managing its own schema in the database really needs foreign key support, what else do you expect them to do, besides try and force them on? Etc.

And in the specific case of the library you linked, it takes a pre-created sql.DB instance.

You can also provide a "path", which is what happens in most of the example code, and I guess, the way most people use it: https://github.com/nalgeon/redka/blob/main/redka.go#L94

I think it would be instructive for contributors of this driver to search GitHub for "PRAGMA" and "mattn" and see just how many people erroneously set pragmas on the connection pool, and never realize they need to register a custom driver with a ConnectHook.

@rittneje
Copy link
Collaborator

rittneje commented Jul 1, 2024

If another library wishes to set pragmas in a driver-agnostic way, I think it would make more sense for it to do so via driver.Connector + sql.OpenDB. Then it will truly be agnostic, instead of relying on all possible drivers supporting some de facto query parameter format.

@ncruces
Copy link
Author

ncruces commented Jul 2, 2024

I'd be very interested in seeing how you could do that.

That is: implement a library that can work with both mattn and modernc (ignore my driver, it's not popular enough to matter), enable foreign keys on all connections it makes to a database, and does so without importing either driver, letting users of the library pick the driver, without build tags, without getting both drivers statically linked into the binary (which defeats the entire purpose, and is actually dangerous because of SQLite locking issues).

@jtarchie
Copy link

jtarchie commented Jul 15, 2024

A workaround till decided maybe,

	sql.Register("sqlite3_with_hook_example",
		&sqlite3.SQLiteDriver{
			ConnectHook: func(conn *sqlite3.SQLiteConn) error {
				conn.Exec(`PRAGMA ...`)
				return nil
			},
		})

This would ensure that every new connection gets the pragma. I think. I want to verify, but time. 😆

@rittneje
Copy link
Collaborator

Here is a primitive version of how to do it in a driver-agnostic way:

type connector struct {
	d   driver.Driver
	dsn string
}

func newConnector(driverName string, dsn string) (driver.Connector, error) {
	db, err := sql.Open(driverName, "")
	if err != nil {
		return nil, err
	}
	defer db.Close()

	return &connector{
		d:   db.Driver(),
		dsn: dsn,
	}, nil
}

func (c *connector) Connect(context.Context) (driver.Conn, error) {
	conn, err := c.d.Open(c.dsn)
	if err != nil {
		return nil, err
	}

	if err := exec(conn, `PRAGMA foreign_keys=ON`); err != nil {
		conn.Close()
		return nil, err
	}

	return conn, nil
}

func (c *connector) Driver() driver.Driver {
	return c.d
}

func exec(conn driver.Conn, query string) error {
	stmt, err := conn.Prepare(query)
	if err != nil {
		return err
	}
	defer stmt.Close()

	_, err = stmt.Exec(nil)
	return err
}

Then your code would do something like:

c, err := newConnector("sqlite3", "file:...")
if err != nil {
   ...
}

return sql.OpenDB(c)

@ncruces
Copy link
Author

ncruces commented Jul 15, 2024

That's a good trick: open and close a connection to get the driver.

Someone should make this a Gist; many libraries needlessly hard code a driver, or are buggy in terms of setting their needed PRAGMAs/etc.

@jtarchie
Copy link

@ncruces, done. I have verified it works. Please see the README for example usage.

@rittneje, thanks for the lead.

@ncruces
Copy link
Author

ncruces commented Sep 17, 2024

Feel free to close the issue.

But the real issue, IMO, is the number of users of this driver that will continue to botch this, because there's not a simple, obvious, documented way of doing it right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants