Skip to content

Commit

Permalink
Resolve issues in UPDATE and DELETE when using schemas
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Jan 28, 2021
1 parent 119c0d2 commit 7e4e4ff
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 29 deletions.
2 changes: 1 addition & 1 deletion dialect_cockroach.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (p *cockroach) Update(s store, model *Model, cols columns.Columns) error {
}

func (p *cockroach) Destroy(s store, model *Model) error {
stmt := p.TranslateSQL(fmt.Sprintf("DELETE FROM %s WHERE %s", p.Quote(model.TableName()), model.whereID()))
stmt := p.TranslateSQL(fmt.Sprintf("DELETE FROM %s AS %s WHERE %s", p.Quote(model.TableName()), model.alias(), model.whereID()))
_, err := genericExec(s, stmt, model.ID())
return err
}
Expand Down
4 changes: 2 additions & 2 deletions dialect_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func genericCreate(s store, model *Model, cols columns.Columns, quoter quotable)
}

func genericUpdate(s store, model *Model, cols columns.Columns, quoter quotable) error {
stmt := fmt.Sprintf("UPDATE %s SET %s WHERE %s", quoter.Quote(model.TableName()), cols.Writeable().QuotedUpdateString(quoter), model.whereNamedID())
stmt := fmt.Sprintf("UPDATE %s AS %s SET %s WHERE %s", quoter.Quote(model.TableName()), model.alias(), cols.Writeable().QuotedUpdateString(quoter), model.whereNamedID())
log(logging.SQL, stmt, model.ID())
_, err := s.NamedExec(stmt, model.Value)
if err != nil {
Expand All @@ -109,7 +109,7 @@ func genericUpdate(s store, model *Model, cols columns.Columns, quoter quotable)
}

func genericDestroy(s store, model *Model, quoter quotable) error {
stmt := fmt.Sprintf("DELETE FROM %s WHERE %s", quoter.Quote(model.TableName()), model.whereID())
stmt := fmt.Sprintf("DELETE FROM %s AS %s WHERE %s", quoter.Quote(model.TableName()), model.alias(), model.whereID())
_, err := genericExec(s, stmt, model.ID())
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion dialect_postgresql.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (p *postgresql) Update(s store, model *Model, cols columns.Columns) error {
}

func (p *postgresql) Destroy(s store, model *Model) error {
stmt := p.TranslateSQL(fmt.Sprintf("DELETE FROM %s WHERE %s", p.Quote(model.TableName()), model.whereID()))
stmt := p.TranslateSQL(fmt.Sprintf("DELETE FROM %s AS %s WHERE %s", p.Quote(model.TableName()), model.alias(), model.whereID()))
_, err := genericExec(s, stmt, model.ID())
if err != nil {
return err
Expand Down
14 changes: 6 additions & 8 deletions model.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,21 +223,19 @@ func (m *Model) touchUpdatedAt() {
}

func (m *Model) whereID() string {
as := m.As
if as == "" {
as = strings.ReplaceAll(m.TableName(), ".", "_")
}

return fmt.Sprintf("%s.%s = ?", as, m.IDField())
return fmt.Sprintf("%s.%s = ?", m.alias(), m.IDField())
}

func (m *Model) whereNamedID() string {
func (m *Model) alias() string {
as := m.As
if as == "" {
as = strings.ReplaceAll(m.TableName(), ".", "_")
}
return as
}

return fmt.Sprintf("%s.%s = :%s", as, m.IDField(), m.IDField())
func (m *Model) whereNamedID() string {
return fmt.Sprintf("%s.%s = :%s", m.alias(), m.IDField(), m.IDField())
}

func (m *Model) isSlice() bool {
Expand Down
26 changes: 26 additions & 0 deletions model_context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package pop

import (
"context"
"io/ioutil"
"os"
"strings"
"testing"
"time"
Expand All @@ -22,6 +24,14 @@ func (t ContextTable) TableName(ctx context.Context) string {
return "context_prefix_" + ctx.Value("prefix").(string) + "_table"
}

func dump(t *testing.T) {
f, err := ioutil.TempFile(os.TempDir(), "sql-dump-*.sql")
require.NoError(t, err)
defer f.Close()
t.Logf(f.Name())
require.NoError(t, PDB.Dialect.DumpSchema(f))
}

func Test_ModelContext(t *testing.T) {
if PDB == nil {
t.Skip("skipping integration tests")
Expand All @@ -39,8 +49,24 @@ func Test_ModelContext(t *testing.T) {
t.Run("prefix="+prefix, func(t *testing.T) {
r := require.New(t)

// PostgreSQL and CockroachDB support schemas which work like a prefix. For those cases, we use
// the schema to ensure that name normalization does not cause query problems.
//
// Since this works only for those two databases, we use underscore for the rest.
//
// While this schema is hardcoded, it would have been too difficult to add this special
// case to the migrations.
switch PDB.Dialect.Name() {
case nameCockroach:
fallthrough
case namePostgreSQL:
prefix = prefix + "." + prefix
dump(t)
}

expected := ContextTable{ID: prefix, Value: prefix}
c := PDB.WithContext(context.WithValue(context.Background(), "prefix", prefix))

r.NoError(c.Create(&expected))

var actual ContextTable
Expand Down
10 changes: 2 additions & 8 deletions sql_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ func (sq *sqlBuilder) buildfromClauses() fromClauses {
fc := sq.Query.fromClauses
for _, m := range models {
tableName := m.TableName()
asName := m.As
if asName == "" {
asName = strings.Replace(tableName, ".", "_", -1)
}
asName := m.alias()
fc = append(fc, fromClause{
From: tableName,
As: asName,
Expand Down Expand Up @@ -216,10 +213,7 @@ var columnCacheMutex = sync.RWMutex{}

func (sq *sqlBuilder) buildColumns() columns.Columns {
tableName := sq.Model.TableName()
asName := sq.Model.As
if asName == "" {
asName = strings.Replace(tableName, ".", "_", -1)
}
asName := sq.Model.alias()
acl := len(sq.AddColumns)
if acl == 0 {
columnCacheMutex.RLock()
Expand Down
39 changes: 30 additions & 9 deletions testdata/migrations/20210104145901_context_tables.up.fizz
Original file line number Diff line number Diff line change
@@ -1,9 +1,30 @@
create_table("context_prefix_a_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}

create_table("context_prefix_b_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}
{{ if eq .Dialect "sqlite3" }}
create_table("context_prefix_a_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}

create_table("context_prefix_b_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}
{{ end }}

{{ if eq .Dialect "mysql" }}
create_table("context_prefix_a_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}

create_table("context_prefix_b_table") {
t.Column("id", "string", { primary: true })
t.Column("value", "string")
}
{{ end }}

{{ if eq .Dialect "postgres" }}
sql("CREATE SCHEMA context_prefix_a")
sql("CREATE SCHEMA context_prefix_b")
sql("CREATE TABLE IF NOT EXISTS \"context_prefix_a\".\"a_table\" (id character varying(255) NOT NULL, value character varying(255) NOT NULL, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL)")
sql("CREATE TABLE IF NOT EXISTS \"context_prefix_b\".\"b_table\" (id character varying(255) NOT NULL, value character varying(255) NOT NULL, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL)")
{{ end }}

0 comments on commit 7e4e4ff

Please sign in to comment.