Rework SET IDENTITY_INSERT for Go 1.10

.gitignore

@@ -2,4 +2,5 @@
 .idea/
 *.cover
 coverage.txt
+internal/test/sql/*_combined.tmp.sql
 reform-database.sqlite3

Makefile

@@ -44,11 +44,16 @@ test-db:
 		internal/test/sql/$(REFORM_DATABASE)_drop.sql
 	reform-db -db-driver="$(REFORM_DRIVER)" -db-source="$(REFORM_ROOT_SOURCE)" exec \
 		internal/test/sql/$(REFORM_DATABASE)_create.sql
-	reform-db -db-driver="$(REFORM_DRIVER)" -db-source="$(REFORM_INIT_SOURCE)" exec \
+
+	cat \
 		internal/test/sql/$(REFORM_DATABASE)_init.sql \
 		internal/test/sql/data.sql \
 		internal/test/sql/$(REFORM_DATABASE)_data.sql \
-		internal/test/sql/$(REFORM_DATABASE)_set.sql
+		internal/test/sql/$(REFORM_DATABASE)_set.sql \
+		> internal/test/sql/$(REFORM_DATABASE)_combined.tmp.sql
+	reform-db -db-driver="$(REFORM_DRIVER)" -db-source="$(REFORM_INIT_SOURCE)" exec \
+		internal/test/sql/$(REFORM_DATABASE)_combined.tmp.sql
+
 	go test $(REFORM_TEST_FLAGS) -covermode=count -coverprofile=reform-db.cover gopkg.in/reform.v1/reform-db
 	go test $(REFORM_TEST_FLAGS) -covermode=count -coverprofile=reform.cover
 	gocoverutil -coverprofile=coverage.txt merge *.cover

base_test.go

@@ -45,19 +45,29 @@ func checkForeignKeys(t *testing.T, q *reform.Querier) {
 	require.True(t, enabled)
 }
 
-// setIdentityInsert allows or disallows insertions of rows with set primary keys for MS SQL.
-func setIdentityInsert(t *testing.T, q *reform.Querier, table string, allow bool) {
+// withIdentityInsert executes an action with MS SQL IDENTITY_INSERT enabled for a table
+func withIdentityInsert(suite *ReformSuite, q *reform.Querier, table string, action func()) {
 	if q.Dialect != mssql.Dialect && q.Dialect != sqlserver.Dialect {
+		action()
 		return
 	}
 
-	allowString := "OFF"
-	if allow {
-		allowString = "ON"
-	}
-	sql := fmt.Sprintf("SET IDENTITY_INSERT %s %s", q.QuoteIdentifier(table), allowString)
-	_, err := q.Exec(sql)
-	require.NoError(t, err)
+	t := suite.T()
+	sqlTemplate := fmt.Sprintf("SET IDENTITY_INSERT %s %%s", q.QuoteIdentifier(table))
+
+	_, onErr := q.Exec(fmt.Sprintf(sqlTemplate, "ON"))
+	require.NoError(t, onErr)
+
+	action()
+
+	_, offErr := q.Exec(fmt.Sprintf(sqlTemplate, "OFF"))
+	require.NoError(t, offErr)
+}
+
+func insertPersonWithID(suite *ReformSuite, q *reform.Querier, str reform.Struct) error {
+	var err error
+	withIdentityInsert(suite, q, "people", func() { err = q.Insert(str) })
+	return err
 }
 
 type ReformSuite struct {
@@ -80,8 +90,6 @@ func (s *ReformSuite) SetupTest() {
 	s.Require().NoError(err)
 
 	s.q = s.tx.WithTag("test")
-
-	setIdentityInsert(s.T(), s.q, "people", false)
 }
 
 func (s *ReformSuite) TearDownTest() {
@@ -161,8 +169,6 @@ func (s *ReformSuite) TestPlaceholders() {
 }
 
 func (s *ReformSuite) TestTimezones() {
-	setIdentityInsert(s.T(), s.q, "people", true)
-
 	t1 := time.Now()
 	t2 := t1.UTC()
 	vlat, err := time.LoadLocation("Asia/Vladivostok")
@@ -176,8 +182,11 @@ func (s *ReformSuite) TestTimezones() {
 		q := fmt.Sprintf(`INSERT INTO people (id, name, created_at) VALUES `+
 			`(11, '11', %s), (12, '12', %s), (13, '13', %s), (14, '14', %s)`,
 			s.q.Placeholder(1), s.q.Placeholder(2), s.q.Placeholder(3), s.q.Placeholder(4))
-		_, err := s.q.Exec(q, t1, t2, tVLAT, tHST)
-		s.NoError(err)
+
+		withIdentityInsert(s, s.q, "people", func() {
+			_, err := s.q.Exec(q, t1, t2, tVLAT, tHST)
+			s.NoError(err)
+		})
 
 		q = `SELECT created_at, created_at FROM people WHERE id IN (11, 12, 13, 14) ORDER BY id`
 		rows, err := s.q.Query(q)
@@ -200,8 +209,11 @@ func (s *ReformSuite) TestTimezones() {
 		q := fmt.Sprintf(`INSERT INTO projects (id, name, start) VALUES `+
 			`('11', '11', %s), ('12', '12', %s), ('13', '13', %s), ('14', '14', %s)`,
 			s.q.Placeholder(1), s.q.Placeholder(2), s.q.Placeholder(3), s.q.Placeholder(4))
-		_, err := s.q.Exec(q, t1, t2, tVLAT, tHST)
-		s.NoError(err)
+
+		withIdentityInsert(s, s.q, "people", func() {
+			_, err := s.q.Exec(q, t1, t2, tVLAT, tHST)
+			s.NoError(err)
+		})
 
 		q = `SELECT start, start FROM projects WHERE id IN ('11', '12', '13', '14') ORDER BY id`
 		rows, err := s.q.Query(q)

db_test.go

@@ -15,13 +15,11 @@ func (s *ReformSuite) TestBeginCommit() {
 	s.Require().NoError(s.tx.Rollback())
 	s.q = nil
 
-	setIdentityInsert(s.T(), DB.Querier, "people", true)
-
 	person := &Person{ID: 42, Email: pointer.ToString(faker.Internet().Email())}
 
 	tx, err := DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person))
+	s.NoError(insertPersonWithID(s, tx.Querier, person))
 	s.NoError(tx.Commit())
 	s.Equal(tx.Commit(), reform.ErrTxDone)
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
@@ -33,13 +31,11 @@ func (s *ReformSuite) TestBeginRollback() {
 	s.Require().NoError(s.tx.Rollback())
 	s.q = nil
 
-	setIdentityInsert(s.T(), DB.Querier, "people", true)
-
 	person := &Person{ID: 42, Email: pointer.ToString(faker.Internet().Email())}
 
 	tx, err := DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person))
+	s.NoError(insertPersonWithID(s, tx.Querier, person))
 	s.NoError(tx.Rollback())
 	s.Equal(tx.Commit(), reform.ErrTxDone)
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
@@ -55,17 +51,15 @@ func (s *ReformSuite) TestErrorInTransaction() {
 	s.Require().NoError(s.tx.Rollback())
 	s.q = nil
 
-	setIdentityInsert(s.T(), DB.Querier, "people", true)
-
 	person1 := &Person{ID: 42, Email: pointer.ToString(faker.Internet().Email())}
 	person2 := &Person{ID: 43, Email: pointer.ToString(faker.Internet().Email())}
 
 	// commit works
 	tx, err := DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person1))
-	s.Error(tx.Insert(person1))   // duplicate PK
-	s.NoError(tx.Insert(person2)) // INSERT works
+	s.NoError(insertPersonWithID(s, tx.Querier, person1))
+	s.Error(insertPersonWithID(s, tx.Querier, person1))   // duplicate PK
+	s.NoError(insertPersonWithID(s, tx.Querier, person2)) // INSERT works
 	s.NoError(tx.Commit())
 	s.Equal(tx.Commit(), reform.ErrTxDone)
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
@@ -77,9 +71,9 @@ func (s *ReformSuite) TestErrorInTransaction() {
 	// rollback works
 	tx, err = DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person1))
-	s.Error(tx.Insert(person1))   // duplicate PK
-	s.NoError(tx.Insert(person2)) // INSERT works
+	s.NoError(insertPersonWithID(s, tx.Querier, person1))
+	s.Error(insertPersonWithID(s, tx.Querier, person1))   // duplicate PK
+	s.NoError(insertPersonWithID(s, tx.Querier, person2)) // INSERT works
 	s.NoError(tx.Rollback())
 	s.Equal(tx.Commit(), reform.ErrTxDone)
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
@@ -97,28 +91,30 @@ func (s *ReformSuite) TestAbortedTransaction() {
 	s.Require().NoError(s.tx.Rollback())
 	s.q = nil
 
-	setIdentityInsert(s.T(), DB.Querier, "people", true)
-
 	person1 := &Person{ID: 42, Email: pointer.ToString(faker.Internet().Email())}
 	person2 := &Person{ID: 43, Email: pointer.ToString(faker.Internet().Email())}
 
 	// commit fails
 	tx, err := DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person1))
-	s.EqualError(tx.Insert(person1), `pq: duplicate key value violates unique constraint "people_pkey"`)
-	s.EqualError(tx.Insert(person2), `pq: current transaction is aborted, commands ignored until end of transaction block`)
-	s.EqualError(tx.Commit(), `pq: Could not complete operation in a failed transaction`)
+	s.NoError(insertPersonWithID(s, tx.Querier, person1))
+	s.Contains(insertPersonWithID(s, tx.Querier, person1).Error(), `duplicate key value violates unique constraint "people_pkey"`)
+	s.Contains(insertPersonWithID(s, tx.Querier, person2).Error(), `current transaction is aborted, commands ignored until end of transaction block`)
+	err = tx.Commit()
+	s.Require().Error(err)
+	if err.Error() != `pq: Could not complete operation in a failed transaction` && err.Error() != `commit unexpectedly resulted in rollback` {
+		s.Failf("unexpected error", "actual: %q", err)
+	}
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
 	s.EqualError(DB.Reload(person1), reform.ErrNoRows.Error())
 	s.EqualError(DB.Reload(person2), reform.ErrNoRows.Error())
 
 	// rollback works
 	tx, err = DB.Begin()
 	s.Require().NoError(err)
-	s.NoError(tx.Insert(person1))
-	s.EqualError(tx.Insert(person1), `pq: duplicate key value violates unique constraint "people_pkey"`)
-	s.EqualError(tx.Insert(person2), `pq: current transaction is aborted, commands ignored until end of transaction block`)
+	s.NoError(insertPersonWithID(s, tx.Querier, person1))
+	s.Contains(insertPersonWithID(s, tx.Querier, person1).Error(), `duplicate key value violates unique constraint "people_pkey"`)
+	s.Contains(insertPersonWithID(s, tx.Querier, person2).Error(), `current transaction is aborted, commands ignored until end of transaction block`)
 	s.NoError(tx.Rollback())
 	s.Equal(tx.Commit(), reform.ErrTxDone)
 	s.Equal(tx.Rollback(), reform.ErrTxDone)
@@ -130,13 +126,11 @@ func (s *ReformSuite) TestInTransaction() {
 	s.Require().NoError(s.tx.Rollback())
 	s.q = nil
 
-	setIdentityInsert(s.T(), DB.Querier, "people", true)
-
 	person := &Person{ID: 42, Email: pointer.ToString(faker.Internet().Email())}
 
 	// error in closure
 	err := DB.InTransaction(func(tx *reform.TX) error {
-		s.NoError(tx.Insert(person))
+		s.NoError(insertPersonWithID(s, tx.Querier, person))
 		return errors.New("epic error")
 	})
 	s.EqualError(err, "epic error")
@@ -145,16 +139,16 @@ func (s *ReformSuite) TestInTransaction() {
 	// panic in closure
 	s.Panics(func() {
 		err = DB.InTransaction(func(tx *reform.TX) error {
-			s.NoError(tx.Insert(person))
+			s.NoError(insertPersonWithID(s, tx.Querier, person))
 			panic("epic panic!")
 		})
 	})
 	s.Equal(DB.Reload(person), reform.ErrNoRows)
 
 	// duplicate PK in closure
 	err = DB.InTransaction(func(tx *reform.TX) error {
-		s.NoError(tx.Insert(person))
-		err := tx.Insert(person)
+		s.NoError(insertPersonWithID(s, tx.Querier, person))
+		err := insertPersonWithID(s, tx.Querier, person)
 		s.Error(err)
 		return err
 	})
@@ -163,7 +157,7 @@ func (s *ReformSuite) TestInTransaction() {
 
 	// no error
 	err = DB.InTransaction(func(tx *reform.TX) error {
-		s.NoError(tx.Insert(person))
+		s.NoError(insertPersonWithID(s, tx.Querier, person))
 		return nil
 	})
 	s.NoError(err)

querier_commands_test.go

@@ -53,11 +53,9 @@ func (s *ReformSuite) TestInsertWithValues() {
 }
 
 func (s *ReformSuite) TestInsertWithPrimaryKey() {
-	setIdentityInsert(s.T(), s.q, "people", true)
-
 	newEmail := faker.Internet().Email()
 	person := &Person{ID: 50, Email: &newEmail}
-	err := s.q.Insert(person)
+	err := insertPersonWithID(s, s.q, person)
 	s.NoError(err)
 	s.Equal(int32(50), person.ID)
 	s.Equal("", person.Name)
@@ -163,13 +161,14 @@ func (s *ReformSuite) TestInsertMulti() {
 }
 
 func (s *ReformSuite) TestInsertMultiWithPrimaryKeys() {
-	setIdentityInsert(s.T(), s.q, "people", true)
-
 	newEmail := faker.Internet().Email()
 	newName := faker.Name().Name()
 	person1, person2 := &Person{ID: 50, Email: &newEmail}, &Person{ID: 51, Name: newName}
-	err := s.q.InsertMulti(person1, person2)
-	s.NoError(err)
+
+	withIdentityInsert(s, s.q, "people", func() {
+		err := s.q.InsertMulti(person1, person2)
+		s.NoError(err)
+	})
 
 	s.Equal(int32(50), person1.ID)
 	s.Equal("", person1.Name)
@@ -365,15 +364,16 @@ func (s *ReformSuite) TestSave() {
 }
 
 func (s *ReformSuite) TestSaveWithPrimaryKey() {
-	setIdentityInsert(s.T(), s.q, "people", true)
-
 	newName := faker.Name().Name()
 	person := &Person{ID: 99, Name: newName}
-	err := s.q.Save(person)
-	s.NoError(err)
+
+	withIdentityInsert(s, s.q, "people", func() {
+		err := s.q.Save(person)
+		s.NoError(err)
+	})
 
 	// that should cause no-op UPDATE, see https://github.com/go-reform/reform/issues/131
-	err = s.q.Save(person)
+	err := s.q.Save(person)
 	s.NoError(err)
 }