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

feat: add query.Delete #658

Merged
merged 4 commits into from Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions dialect.go
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions dialect_cockroach.go
Expand Up @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions dialect_common.go
Expand Up @@ -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...)
Expand Down
4 changes: 4 additions & 0 deletions dialect_mysql.go
Expand Up @@ -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")
}
Expand Down
4 changes: 4 additions & 0 deletions dialect_postgresql.go
Expand Up @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions dialect_sqlite.go
Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Expand Up @@ -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'"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment in test.sh

ports:
- "26257:26257"
volumes:
Expand Down
13 changes: 13 additions & 0 deletions executors.go
Expand Up @@ -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)
})
}
37 changes: 36 additions & 1 deletion executors_test.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})
}
10 changes: 10 additions & 0 deletions query.go
Expand Up @@ -7,6 +7,13 @@ import (
"github.com/gobuffalo/pop/v5/logging"
)

type operation string

const (
Select operation = "SELECT"
Delete operation = "DELETE"
)
Comment on lines +12 to +15
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For bulk inserts/updates this list will likely grow.


// Query is the main value that is used to build up a query
// to be executed against the `Connection`.
type Query struct {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -196,6 +205,7 @@ func Q(c *Connection) *Query {
eager: c.eager,
eagerFields: c.eagerFields,
eagerMode: eagerModeNil,
Operation: Select,
}
}

Expand Down
30 changes: 29 additions & 1 deletion sql_builder.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
//}
Comment on lines +131 to +140
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this part, but I think it is better to leave it out. Should I delete the comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah leave the feature out, but the comment in :)


return sql
}

func (sq *sqlBuilder) buildfromClauses() fromClauses {
models := []*Model{
sq.Model,
Expand Down
7 changes: 6 additions & 1 deletion test.sh
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down