Merge pull request #853 from rqlite/fk-cli

Support Foreign Key Constraint control via command-line options

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 6.2.0 (unreleased)
 - [PR #851](https://github.com/rqlite/rqlite/pull/851): rqlite CLI properly supports PRAGMA directives.
+- [PR #853](https://github.com/rqlite/rqlite/pull/853): Support enabling Foreign Key constraints via command-line options.
 
 ## 6.1.0 (August 5th 2021)
 This release makes significant changes to SQLite database connection handling, resulting in proper support for high-performance concurrent reads of in-memory databases (the default choice for rqlite). 

DOC/FOREIGN_KEY_CONSTRAINTS.md

@@ -1,3 +1,5 @@
 # Foreign Key Constraints
 
-Since SQLite does not enforce foreign key constraints by default, neither does rqlite. However you can enable foreign key constraints on rqlite simply by sending `PRAGMA foreign_keys=ON` via the [CLI](https://github.com/rqlite/rqlite/tree/master/cmd/rqlite) or the [write API](https://github.com/rqlite/rqlite/blob/master/DOC/DATA_API.md). Constraints will then remain enabled, even across restarts, until the statement `PRAGMA foreign_keys=OFF` is issued.
+Since SQLite does not enforce foreign key constraints by default, neither does rqlite. However you can enable foreign key constraints in rqlite via the command line option `-fk=true`. Setting this command line will enable Foreign Key constraints on all connections that rqlite makes to the underlying SQLite database.
+
+Issuing the `PRAGMA foreign_keys = boolean` usually results in unpredicatable behaviour, since rqlite doesn't offer connection-level control of the underlying SQLite database. It is not recommended.

DOC/RESTORE_FROM_SQLITE.md

@@ -59,4 +59,9 @@ $ rqlite
 **Note that you must convert the SQLite file (in the above examples the file named `restore.sqlite`) to the list of SQL commands**. You cannot restore using the actual SQLite database file.
 
 ## Caveats
-The behavior of the restore operation when data already exists on the cluster is undefined -- you should only restore to a cluster that has no data, or a brand-new cluster. Also, please **note that SQLite dump files normally contain a command to disable Foreign Key constraints**. If you wish to re-enable Foreign Key constraints after the load operation completes, check out [this documentation](https://github.com/rqlite/rqlite/blob/master/DOC/FOREIGN_KEY_CONSTRAINTS.md).
+The behavior of the restore operation when data already exists on the cluster is undefined -- you should only restore to a cluster that has no data, or a brand-new cluster. Also, please **note that SQLite dump files normally contain a command to disable Foreign Key constraints**. If you are running with Foreign Key Constraints enabled, and wish to re-enable this, this is the one time you should explicitly re-enable those constraints via the following `curl` command:
+```bash
+curl -XPOST 'localhost:4001/db/execute?pretty' -H "Content-Type: application/json" -d '[
+    "PRAGMA foreign_keys = 1"
+]'
+```

cmd/rqlited/main.go

@@ -62,6 +62,7 @@ var discoID string
 var expvar bool
 var pprofEnabled bool
 var onDisk bool
+var fkConstraints bool
 var raftLogLevel string
 var raftNonVoter bool
 var raftSnapThreshold uint64
@@ -109,6 +110,7 @@ func init() {
 	flag.BoolVar(&expvar, "expvar", true, "Serve expvar data on HTTP server")
 	flag.BoolVar(&pprofEnabled, "pprof", true, "Serve pprof data on HTTP server")
 	flag.BoolVar(&onDisk, "on-disk", false, "Use an on-disk SQLite database")
+	flag.BoolVar(&fkConstraints, "fk", false, "Enable SQLite foreign key constraints")
 	flag.BoolVar(&showVersion, "version", false, "Show version information and exit")
 	flag.BoolVar(&raftNonVoter, "raft-non-voter", false, "Configure as non-voting node")
 	flag.StringVar(&raftHeartbeatTimeout, "raft-timeout", "1s", "Raft heartbeat timeout")
@@ -196,6 +198,7 @@ func main() {
 		log.Fatalf("failed to determine absolute data path: %s", err.Error())
 	}
 	dbConf := store.NewDBConfig(!onDisk)
+	dbConf.FKConstraints = fkConstraints
 
 	str := store.New(raftTn, &store.StoreConfig{
 		DBConf: dbConf,

db/db.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"math/rand"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -92,8 +93,8 @@ type Rows struct {
 }
 
 // Open opens a file-based database, creating it if it does not exist.
-func Open(dbPath string) (*DB, error) {
+func Open(dbPath string, fkEnabled bool) (*DB, error) {
-	rwDSN := fmt.Sprintf("file:%s", dbPath)
+	rwDSN := fmt.Sprintf("file:%s?_fk=%s", dbPath, strconv.FormatBool(fkEnabled))
 	rwDB, err := sql.Open("sqlite3", rwDSN)
 	if err != nil {
 		return nil, err
@@ -101,6 +102,7 @@ func Open(dbPath string) (*DB, error) {
 
 	roOpts := []string{
 		"mode=ro",
+		fmt.Sprintf("_fk=%s", strconv.FormatBool(fkEnabled)),
 	}
 
 	roDSN := fmt.Sprintf("file:%s?%s", dbPath, strings.Join(roOpts, "&"))
@@ -130,13 +132,14 @@ func Open(dbPath string) (*DB, error) {
 }
 
 // OpenInMemory returns a new in-memory database.
-func OpenInMemory() (*DB, error) {
+func OpenInMemory(fkEnabled bool) (*DB, error) {
 	inMemPath := fmt.Sprintf("file:/%s", randomString())
 
 	rwOpts := []string{
 		"mode=rw",
 		"vfs=memdb",
 		"_txlock=immediate",
+		fmt.Sprintf("_fk=%s", strconv.FormatBool(fkEnabled)),
 	}
 
 	rwDSN := fmt.Sprintf("%s?%s", inMemPath, strings.Join(rwOpts, "&"))
@@ -156,6 +159,7 @@ func OpenInMemory() (*DB, error) {
 		"mode=ro",
 		"vfs=memdb",
 		"_txlock=deferred",
+		fmt.Sprintf("_fk=%s", strconv.FormatBool(fkEnabled)),
 	}
 
 	roDSN := fmt.Sprintf("%s?%s", inMemPath, strings.Join(roOpts, "&"))
@@ -181,13 +185,13 @@ func OpenInMemory() (*DB, error) {
 // LoadIntoMemory loads an in-memory database with that at the path.
 // Not safe to call while other operations are happening with the
 // source database.
-func LoadIntoMemory(dbPath string) (*DB, error) {
+func LoadIntoMemory(dbPath string, fkEnabled bool) (*DB, error) {
-	dstDB, err := OpenInMemory()
+	dstDB, err := OpenInMemory(fkEnabled)
 	if err != nil {
 		return nil, err
 	}
 
-	srcDB, err := Open(dbPath)
+	srcDB, err := Open(dbPath, false)
 	if err != nil {
 		return nil, err
 	}
@@ -206,7 +210,7 @@ func LoadIntoMemory(dbPath string) (*DB, error) {
 // DeserializeIntoMemory loads an in-memory database with that contained
 // in the byte slide. The byte slice must not be changed or garbage-collected
 // until after this function returns.
-func DeserializeIntoMemory(b []byte) (retDB *DB, retErr error) {
+func DeserializeIntoMemory(b []byte, fkEnabled bool) (retDB *DB, retErr error) {
 	// Get a plain-ol' in-memory database.
 	tmpDB, err := sql.Open("sqlite3", ":memory:")
 	if err != nil {
@@ -221,7 +225,7 @@ func DeserializeIntoMemory(b []byte) (retDB *DB, retErr error) {
 
 	// tmpDB will still be using memory in Go space, so tmpDB needs to be explicitly
 	// copied to a new database, which we create now.
-	db, err := OpenInMemory()
+	db, err := OpenInMemory(fkEnabled)
 	if err != nil {
 		return nil, fmt.Errorf("DeserializeIntoMemory: %s", err.Error())
 	}
@@ -609,7 +613,7 @@ func (db *DB) queryWithConn(req *command.Request, xTime bool, conn *sql.Conn) ([
 // Backup writes a consistent snapshot of the database to the given file.
 // This function can be called when changes to the database are in flight.
 func (db *DB) Backup(path string) error {
-	dstDB, err := Open(path)
+	dstDB, err := Open(path, false)
 	if err != nil {
 		return err
 	}

db/db_test.go

@@ -24,7 +24,7 @@ func Test_DbFileCreation(t *testing.T) {
 	defer os.RemoveAll(dir)
 	dbPath := path.Join(dir, "test_db")
 
-	db, err := Open(dbPath)
+	db, err := Open(dbPath, false)
 	if err != nil {
 		t.Fatalf("failed to open new database: %s", err.Error())
 	}
@@ -113,6 +113,68 @@ func Test_TableCreationInMemory(t *testing.T) {
 	}
 }
 
+// Test_TableCreationInMemoryFK ensures foreign key constraints work
+func Test_TableCreationInMemoryFK(t *testing.T) {
+	createTableFoo := "CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)"
+	createTableBar := "CREATE TABLE bar (fooid INTEGER NOT NULL PRIMARY KEY, FOREIGN KEY(fooid) REFERENCES foo(id))"
+	insertIntoBar := "INSERT INTO bar(fooid) VALUES(1)"
+
+	db := mustCreateInMemoryDatabase()
+	defer db.Close()
+
+	r, err := db.ExecuteStringStmt(createTableFoo)
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = db.ExecuteStringStmt(createTableBar)
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = db.ExecuteStringStmt(insertIntoBar)
+	if err != nil {
+		t.Fatalf("failed to insert record: %s", err.Error())
+	}
+	if exp, got := `[{"last_insert_id":1,"rows_affected":1}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	// Now, do same testing with FK constraints enabled.
+	dbFK := mustCreateInMemoryDatabaseFK()
+	defer dbFK.Close()
+
+	r, err = dbFK.ExecuteStringStmt(createTableFoo)
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = dbFK.ExecuteStringStmt(createTableBar)
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = dbFK.ExecuteStringStmt(insertIntoBar)
+	if err != nil {
+		t.Fatalf("failed to insert record: %s", err.Error())
+	}
+	if exp, got := `[{"error":"FOREIGN KEY constraint failed"}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+}
+
 func Test_SQLiteMasterTable(t *testing.T) {
 	db, path := mustCreateDatabase()
 	defer db.Close()
@@ -150,7 +212,7 @@ func Test_LoadIntoMemory(t *testing.T) {
 		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
 	}
 
-	inmem, err := LoadIntoMemory(path)
+	inmem, err := LoadIntoMemory(path, false)
 	if err != nil {
 		t.Fatalf("failed to create loaded in-memory database: %s", err.Error())
 	}
@@ -204,7 +266,7 @@ func Test_DeserializeIntoMemory(t *testing.T) {
 		t.Fatalf("failed to read database on disk: %s", err.Error())
 	}
 
-	newDB, err := DeserializeIntoMemory(b)
+	newDB, err := DeserializeIntoMemory(b, false)
 	if err != nil {
 		t.Fatalf("failed to deserialize database: %s", err.Error())
 	}
@@ -1068,7 +1130,7 @@ func Test_Backup(t *testing.T) {
 		t.Fatalf("failed to backup database: %s", err.Error())
 	}
 
-	newDB, err := Open(dstDB)
+	newDB, err := Open(dstDB, false)
 	if err != nil {
 		t.Fatalf("failed to open backup database: %s", err.Error())
 	}
@@ -1117,7 +1179,7 @@ func Test_Copy(t *testing.T) {
 
 	dstFile := mustTempFile()
 	defer os.Remove(dstFile)
-	dstDB, err := Open(dstFile)
+	dstDB, err := Open(dstFile, false)
 	if err != nil {
 		t.Fatalf("failed to open destination database: %s", err)
 	}
@@ -1186,7 +1248,7 @@ func Test_Serialize(t *testing.T) {
 		t.Fatalf("failed to write serialized database to file: %s", err.Error())
 	}
 
-	newDB, err := Open(dstDB.Name())
+	newDB, err := Open(dstDB.Name(), false)
 	if err != nil {
 		t.Fatalf("failed to open on-disk serialized database: %s", err.Error())
 	}
@@ -1230,7 +1292,7 @@ func Test_DumpMemory(t *testing.T) {
 	defer db.Close()
 	defer os.Remove(path)
 
-	inmem, err := LoadIntoMemory(path)
+	inmem, err := LoadIntoMemory(path, false)
 	if err != nil {
 		t.Fatalf("failed to create loaded in-memory database: %s", err.Error())
 	}
@@ -1409,7 +1471,7 @@ func Test_JSON1(t *testing.T) {
 func mustCreateDatabase() (*DB, string) {
 	var err error
 	f := mustTempFile()
-	db, err := Open(f)
+	db, err := Open(f, false)
 	if err != nil {
 		panic("failed to open database")
 	}
@@ -1418,13 +1480,21 @@ func mustCreateDatabase() (*DB, string) {
 }
 
 func mustCreateInMemoryDatabase() *DB {
-	db, err := OpenInMemory()
+	db, err := OpenInMemory(false)
 	if err != nil {
 		panic("failed to open in-memory database")
 	}
 	return db
 }
 
+func mustCreateInMemoryDatabaseFK() *DB {
+	db, err := OpenInMemory(true)
+	if err != nil {
+		panic("failed to open in-memory database with foreign key constraints")
+	}
+	return db
+}
+
 func mustWriteAndOpenDatabase(b []byte) (*DB, string) {
 	var err error
 	f := mustTempFile()
@@ -1433,7 +1503,7 @@ func mustWriteAndOpenDatabase(b []byte) (*DB, string) {
 		panic("failed to write file")
 	}
 
-	db, err := Open(f)
+	db, err := Open(f, false)
 	if err != nil {
 		panic("failed to open database")
 	}

store/db_config.go

@@ -2,7 +2,11 @@ package store
 
 // DBConfig represents the configuration of the underlying SQLite database.
 type DBConfig struct {
-	Memory bool // Whether the database is in-memory only.
+	// Whether the database is in-memory only.
+	Memory bool `json:"memory"`
+
+	// Enforce Foreign Key constraints
+	FKConstraints bool `json:"fk_constraints"`
 }
 
 // NewDBConfig returns a new DB config instance.

store/store.go

@@ -803,9 +803,9 @@ func (s *Store) Noop(id string) error {
 // database will be initialized with the contents of b.
 func (s *Store) createInMemory(b []byte) (db *sql.DB, err error) {
 	if b == nil {
-		db, err = sql.OpenInMemory()
+		db, err = sql.OpenInMemory(s.dbConf.FKConstraints)
 	} else {
-		db, err = sql.DeserializeIntoMemory(b)
+		db, err = sql.DeserializeIntoMemory(b, s.dbConf.FKConstraints)
 	}
 	return
 }
@@ -822,7 +822,7 @@ func (s *Store) createOnDisk(b []byte) (*sql.DB, error) {
 			return nil, err
 		}
 	}
-	return sql.Open(s.dbPath)
+	return sql.Open(s.dbPath, s.dbConf.FKConstraints)
 }
 
 // setLogInfo records some key indexs about the log.

store/store_test.go

@@ -229,6 +229,31 @@ func Test_SingleNodeExecuteQueryTx(t *testing.T) {
 	}
 }
 
+func Test_SingleNodeInMemFK(t *testing.T) {
+	s := mustNewStoreFK(true)
+	defer os.RemoveAll(s.Path())
+
+	if err := s.Open(true); err != nil {
+		t.Fatalf("failed to open single-node store: %s", err.Error())
+	}
+	defer s.Close(true)
+	s.WaitForLeader(10 * time.Second)
+
+	er := executeRequestFromStrings([]string{
+		`CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)`,
+		`CREATE TABLE bar (fooid INTEGER NOT NULL PRIMARY KEY, FOREIGN KEY(fooid) REFERENCES foo(id))`,
+	}, false, false)
+	_, err := s.Execute(er)
+	if err != nil {
+		t.Fatalf("failed to execute on single node: %s", err.Error())
+	}
+
+	res, err := s.Execute(executeRequestFromString("INSERT INTO bar(fooid) VALUES(1)", false, false))
+	if got, exp := asJSON(res), `[{"error":"FOREIGN KEY constraint failed"}]`; exp != got {
+		t.Fatalf("unexpected results for execute\nexp: %s\ngot: %s", exp, got)
+	}
+}
+
 func Test_SingleNodeBackupBinary(t *testing.T) {
 	t.Parallel()
 
@@ -1169,8 +1194,10 @@ func Test_State(t *testing.T) {
 	}
 }
 
-func mustNewStoreAtPath(path string, inmem bool) *Store {
+func mustNewStoreAtPath(path string, inmem, fk bool) *Store {
 	cfg := NewDBConfig(inmem)
+	cfg.FKConstraints = fk
+
 	s := New(mustMockLister("localhost:0"), &StoreConfig{
 		DBConf: cfg,
 		Dir:    path,
@@ -1183,7 +1210,11 @@ func mustNewStoreAtPath(path string, inmem bool) *Store {
 }
 
 func mustNewStore(inmem bool) *Store {
-	return mustNewStoreAtPath(mustTempDir(), inmem)
+	return mustNewStoreAtPath(mustTempDir(), inmem, false)
+}
+
+func mustNewStoreFK(inmem bool) *Store {
+	return mustNewStoreAtPath(mustTempDir(), inmem, true)
 }
 
 type mockSnapshotSink struct {