From 02de8f9931ef8b9103b7e2faff5f4f8e86cbf1e4 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 14 Jul 2021 16:50:15 +0200 Subject: [PATCH 1/4] 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. --- dialect.go | 1 + dialect_cockroach.go | 4 ++++ dialect_common.go | 6 ++++++ dialect_mysql.go | 4 ++++ dialect_postgresql.go | 4 ++++ dialect_sqlite.go | 4 ++++ docker-compose.yml | 1 + executors.go | 13 +++++++++++++ executors_test.go | 37 ++++++++++++++++++++++++++++++++++++- query.go | 10 ++++++++++ sql_builder.go | 30 +++++++++++++++++++++++++++++- test.sh | 7 ++++++- 12 files changed, 118 insertions(+), 3 deletions(-) diff --git a/dialect.go b/dialect.go index dbb4800e..4ff29a3b 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 9e933336..5d200d65 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 79d0d190..1cc18ef3 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 af4dd3b2..06a42f5a 100644 --- a/dialect_mysql.go +++ b/dialect_mysql.go @@ -96,6 +96,10 @@ 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 { + return genericDelete(s, model, query) +} + 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 ad5b2350..6365232e 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 c340afa8..ce755566 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 6c058cc8..54b90986 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 c382f260..eaae29eb 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/executors_test.go b/executors_test.go index bee9a5e8..6ec3386e 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/query.go b/query.go index ba981ef5..3b20b5aa 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 b6e07223..b0b3c5b8 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, diff --git a/test.sh b/test.sh index c6256045..42d7e4a6 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 @@ -68,7 +73,7 @@ function debug_test { dlv test github.com/gobuffalo/pop } -dialects=("postgres" "cockroach" "mysql" "sqlite") +dialects=("sqlite") for dialect in "${dialects[@]}" ; do if [ $DEBUG = 'NO' ]; then From 2f922f3efc41f5d4a697d5ec2d48d1bce41bc405 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 14 Jul 2021 16:55:39 +0200 Subject: [PATCH 2/4] test: re-add temporarily removed dialects --- test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test.sh b/test.sh index 42d7e4a6..4ea9a5f0 100755 --- a/test.sh +++ b/test.sh @@ -73,7 +73,7 @@ function debug_test { dlv test github.com/gobuffalo/pop } -dialects=("sqlite") +dialects=("postgres" "cockroach" "mysql" "sqlite") for dialect in "${dialects[@]}" ; do if [ $DEBUG = 'NO' ]; then From 8d3abb26dfd530824f9566104f355e852868e7be Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 14 Jul 2021 17:09:24 +0200 Subject: [PATCH 3/4] fix: spread args in delete --- dialect_common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialect_common.go b/dialect_common.go index 1cc18ef3..5a9eee9c 100644 --- a/dialect_common.go +++ b/dialect_common.go @@ -119,7 +119,7 @@ func genericDestroy(s store, model *Model, quoter quotable) error { func genericDelete(s store, model *Model, query Query) error { sqlQuery, args := query.ToSQL(model) - _, err := genericExec(s, sqlQuery, args) + _, err := genericExec(s, sqlQuery, args...) return err } From 4b6dbd8c4bf46534965ab0ef1e77a73596144efd Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 14 Jul 2021 17:50:25 +0200 Subject: [PATCH 4/4] fix: mysql 5.7 supports no `DELETE FROM ... AS ...` but only `DELETE FROM ...` --- dialect_mysql.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dialect_mysql.go b/dialect_mysql.go index 06a42f5a..dbd9b49b 100644 --- a/dialect_mysql.go +++ b/dialect_mysql.go @@ -97,7 +97,13 @@ func (m *mysql) Destroy(s store, model *Model) error { } func (m *mysql) Delete(s store, model *Model, query Query) error { - return genericDelete(s, model, query) + 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 {