Repository: gobuffalo/pop
Pull request #658 (2021-07-14 14:52:47)

feat: add query.Delete
This allows writing delete queries without knowing the exact primary key or for composite keys.
`Destroy` only allows to delete by primary key, but there are many cases where you want to delete multiple rows or by some other query than the ID.

Contributes to #29 


Patch:
diff --git a/dialect.go b/dialect.go
index dbb4800..4ff29a3 100644
--- a/dialect.go
+++ b/dialect.go
@@ -13,6 +13,7 @@ type crudable interface {
 	Create(store, *Model, columns.Columns) error
 	Update(store, *Model, columns.Columns) error
 	Destroy(store, *Model) error
+	Delete(store, *Model, Query) error
 }
 
 type fizzable interface {
diff --git a/dialect_cockroach.go b/dialect_cockroach.go
index 9e93333..5d200d6 100644
--- a/dialect_cockroach.go
+++ b/dialect_cockroach.go
@@ -110,6 +110,10 @@ func (p *cockroach) Destroy(s store, model *Model) error {
 	return err
 }
 
+func (p *cockroach) Delete(s store, model *Model, query Query) error {
+	return genericDelete(s, model, query)
+}
+
 func (p *cockroach) SelectOne(s store, model *Model, query Query) error {
 	return genericSelectOne(s, model, query)
 }
diff --git a/dialect_common.go b/dialect_common.go
index 79d0d19..5a9eee9 100644
--- a/dialect_common.go
+++ b/dialect_common.go
@@ -117,6 +117,12 @@ func genericDestroy(s store, model *Model, quoter quotable) error {
 	return nil
 }
 
+func genericDelete(s store, model *Model, query Query) error {
+	sqlQuery, args := query.ToSQL(model)
+	_, err := genericExec(s, sqlQuery, args...)
+	return err
+}
+
 func genericExec(s store, stmt string, args ...interface{}) (sql.Result, error) {
 	log(logging.SQL, stmt, args...)
 	res, err := s.Exec(stmt, args...)
diff --git a/dialect_mysql.go b/dialect_mysql.go
index af4dd3b..dbd9b49 100644
--- a/dialect_mysql.go
+++ b/dialect_mysql.go
@@ -96,6 +96,16 @@ func (m *mysql) Destroy(s store, model *Model) error {
 	return errors.Wrap(err, "mysql destroy")
 }
 
+func (m *mysql) Delete(s store, model *Model, query Query) error {
+	sb := query.toSQLBuilder(model)
+
+	sql := fmt.Sprintf("DELETE FROM %s", m.Quote(model.TableName()))
+	sql = sb.buildWhereClauses(sql)
+
+	_, err := genericExec(s, sql, sb.Args()...)
+	return errors.Wrap(err, "mysql delete")
+}
+
 func (m *mysql) SelectOne(s store, model *Model, query Query) error {
 	return errors.Wrap(genericSelectOne(s, model, query), "mysql select one")
 }
diff --git a/dialect_postgresql.go b/dialect_postgresql.go
index ad5b235..6365232 100644
--- a/dialect_postgresql.go
+++ b/dialect_postgresql.go
@@ -100,6 +100,10 @@ func (p *postgresql) Destroy(s store, model *Model) error {
 	return nil
 }
 
+func (p *postgresql) Delete(s store, model *Model, query Query) error {
+	return genericDelete(s, model, query)
+}
+
 func (p *postgresql) SelectOne(s store, model *Model, query Query) error {
 	return genericSelectOne(s, model, query)
 }
diff --git a/dialect_sqlite.go b/dialect_sqlite.go
index c340afa..ce75556 100644
--- a/dialect_sqlite.go
+++ b/dialect_sqlite.go
@@ -112,6 +112,10 @@ func (m *sqlite) Destroy(s store, model *Model) error {
 	})
 }
 
+func (m *sqlite) Delete(s store, model *Model, query Query) error {
+	return genericDelete(s, model, query)
+}
+
 func (m *sqlite) SelectOne(s store, model *Model, query Query) error {
 	return m.locker(m.smGil, func() error {
 		return errors.Wrap(genericSelectOne(s, model, query), "sqlite select one")
diff --git a/docker-compose.yml b/docker-compose.yml
index 6c058cc..54b9098 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -25,6 +25,7 @@ services:
       - ./sqldumps:/docker-entrypoint-initdb.d
   cockroach:
     image: cockroachdb/cockroach:v20.2.4
+    user: ${CURRENT_UID:?"Please run as follows 'CURRENT_UID=$(id -u):$(id -g) docker-compose up'"}
     ports:
       - "26257:26257"
     volumes:
diff --git a/executors.go b/executors.go
index c382f26..eaae29e 100644
--- a/executors.go
+++ b/executors.go
@@ -439,3 +439,16 @@ func (c *Connection) Destroy(model interface{}) error {
 		})
 	})
 }
+
+func (q *Query) Delete(model interface{}) error {
+	q.Operation = Delete
+
+	return q.Connection.timeFunc("Delete", func() error {
+		m := NewModel(model, q.Connection.Context())
+		err := q.Connection.Dialect.Delete(q.Connection.Store, m, *q)
+		if err != nil {
+			return err
+		}
+		return m.afterDestroy(q.Connection)
+	})
+}
diff --git a/query.go b/query.go
index ba981ef..3b20b5a 100644
--- a/query.go
+++ b/query.go
@@ -7,6 +7,13 @@ import (
 	"github.com/gobuffalo/pop/v5/logging"
 )
 
+type operation string
+
+const (
+	Select operation = "SELECT"
+	Delete operation = "DELETE"
+)
+
 // Query is the main value that is used to build up a query
 // to be executed against the `Connection`.
 type Query struct {
@@ -25,6 +32,7 @@ type Query struct {
 	havingClauses           havingClauses
 	Paginator               *Paginator
 	Connection              *Connection
+	Operation               operation
 }
 
 // Clone will fill targetQ query with the connection used in q, if
@@ -42,6 +50,7 @@ func (q *Query) Clone(targetQ *Query) {
 	targetQ.groupClauses = q.groupClauses
 	targetQ.havingClauses = q.havingClauses
 	targetQ.addColumns = q.addColumns
+	targetQ.Operation = q.Operation
 
 	if q.Paginator != nil {
 		paginator := *q.Paginator
@@ -196,6 +205,7 @@ func Q(c *Connection) *Query {
 		eager:       c.eager,
 		eagerFields: c.eagerFields,
 		eagerMode:   eagerModeNil,
+		Operation:   Select,
 	}
 }
 
diff --git a/sql_builder.go b/sql_builder.go
index b6e0722..b0b3c5b 100644
--- a/sql_builder.go
+++ b/sql_builder.go
@@ -84,7 +84,14 @@ func (sq *sqlBuilder) compile() {
 				sq.sql = sq.Query.RawSQL.Fragment
 			}
 		} else {
-			sq.sql = sq.buildSelectSQL()
+			switch sq.Query.Operation {
+			case Select:
+				sq.sql = sq.buildSelectSQL()
+			case Delete:
+				sq.sql = sq.buildDeleteSQL()
+			default:
+				panic("unexpected query operation " + sq.Query.Operation)
+			}
 		}
 
 		if inRegex.MatchString(sq.sql) {
@@ -114,6 +121,27 @@ func (sq *sqlBuilder) buildSelectSQL() string {
 	return sql
 }
 
+func (sq *sqlBuilder) buildDeleteSQL() string {
+	fc := sq.buildfromClauses()
+
+	sql := fmt.Sprintf("DELETE FROM %s", fc)
+
+	sql = sq.buildWhereClauses(sql)
+
+	// paginated delete supported by sqlite and mysql
+	// > If SQLite is compiled with the SQLITE_ENABLE_UPDATE_DELETE_LIMIT compile-time option [...] - from https://www.sqlite.org/lang_delete.html
+	//
+	// not generic enough IMO, therefore excluded
+	//
+	//switch sq.Query.Connection.Dialect.Name() {
+	//case nameMySQL, nameSQLite3:
+	//	sql = sq.buildOrderClauses(sql)
+	//	sql = sq.buildPaginationClauses(sql)
+	//}
+
+	return sql
+}
+
 func (sq *sqlBuilder) buildfromClauses() fromClauses {
 	models := []*Model{
 		sq.Model,


Test patch:
diff --git a/executors_test.go b/executors_test.go
index bee9a5e..6ec3386 100644
--- a/executors_test.go
+++ b/executors_test.go
@@ -313,7 +313,7 @@ func Test_Exec(t *testing.T) {
 		r := require.New(t)
 
 		user := User{Name: nulls.NewString("Mark 'Awesome' Bates")}
-		tx.Create(&user)
+		r.NoError(tx.Create(&user))
 
 		ctx, _ := tx.Count(user)
 		r.Equal(1, ctx)
@@ -1649,3 +1649,38 @@ func Test_TruncateAll(t *testing.T) {
 		r.Equal(count, ctx)
 	})
 }
+
+func Test_Delete(t *testing.T) {
+	if PDB == nil {
+		t.Skip("skipping integration tests")
+	}
+
+	transaction(func(tx *Connection) {
+		r := require.New(t)
+
+		songTitles := []string{"Yoshimi Battles the Pink Robots, Pt. 1", "Face Down In The Gutter Of Your Love"}
+		user := User{Name: nulls.NewString("Patrik")}
+
+		r.NoError(tx.Create(&user))
+		r.NotZero(user.ID)
+
+		count, err := tx.Count("songs")
+		r.NoError(err)
+
+		for _, title := range songTitles {
+			err = tx.Create(&Song{Title: title, UserID: user.ID})
+			r.NoError(err)
+		}
+
+		ctx, err := tx.Count("songs")
+		r.NoError(err)
+		r.Equal(count+len(songTitles), ctx)
+
+		err = tx.Where("u_id = ?", user.ID).Delete("songs")
+		r.NoError(err)
+
+		ctx, err = tx.Count("songs")
+		r.NoError(err)
+		r.Equal(count, ctx)
+	})
+}
diff --git a/test.sh b/test.sh
index c625604..4ea9a5f 100755
--- a/test.sh
+++ b/test.sh
@@ -37,6 +37,11 @@ function cleanup {
 # defer cleanup, so it will be executed even after premature exit
 trap cleanup EXIT
 
+# The cockroach volume is created by the root user if no user is set.
+# Therefore we set the current user according to https://github.com/docker/compose/issues/1532#issuecomment-619548112
+CURRENT_UID="$(id -u):$(id -g)"
+export CURRENT_UID
+
 docker-compose up -d
 sleep 5 # Ensure mysql is online