diff --git a/sql_builder.go b/sql_builder.go index ad734a12..9816dcf9 100644 --- a/sql_builder.go +++ b/sql_builder.go @@ -1,12 +1,15 @@ package pop import ( + "cmp" "fmt" + "reflect" "regexp" "strings" "sync" "github.com/jmoiron/sqlx" + "github.com/ory/pop/v6/columns" "github.com/ory/pop/v6/logging" ) @@ -137,11 +140,11 @@ func (sq *sqlBuilder) buildDeleteSQL() string { // // not generic enough IMO, therefore excluded // - //switch sq.Query.Connection.Dialect.Name() { - //case nameMySQL, nameSQLite3: + // switch sq.Query.Connection.Dialect.Name() { + // case nameMySQL, nameSQLite3: // sql = sq.buildOrderClauses(sql) // sql = sq.buildPaginationClauses(sql) - //} + // } return sql } @@ -243,22 +246,48 @@ func (sq *sqlBuilder) buildPaginationClauses(sql string) string { var columnCache = map[string]columns.Columns{} var columnCacheMutex = sync.RWMutex{} +func cacheKey(val any) (string, bool) { + if val == nil { + return "", false + } + ty := reflect.TypeOf(val) + for ty.Kind() == reflect.Pointer || + ty.Kind() == reflect.Slice || + ty.Kind() == reflect.Array { + ty = ty.Elem() + } + if ty.Kind() != reflect.Struct { + return "", false + } + + return cmp.Or(ty.PkgPath(), "builtin") + "." + ty.Name(), true +} + func (sq *sqlBuilder) buildColumns() columns.Columns { tableName := sq.Model.TableName() asName := sq.Model.Alias() acl := len(sq.AddColumns) if acl == 0 { - columnCacheMutex.RLock() - cols, ok := columnCache[tableName] - columnCacheMutex.RUnlock() - // if alias is the same, don't remake columns - if ok && cols.TableAlias == asName { - return cols + key, shouldCache := cacheKey(sq.Model.Value) + + if shouldCache { + columnCacheMutex.RLock() + cols, ok := columnCache[key] + columnCacheMutex.RUnlock() + + // if alias is the same, don't remake columns + if ok && cols.TableAlias == asName { + return cols + } } - cols = columns.ForStructWithAlias(sq.Model.Value, tableName, asName, sq.Model.IDField()) - columnCacheMutex.Lock() - columnCache[tableName] = cols - columnCacheMutex.Unlock() + cols := columns.ForStructWithAlias(sq.Model.Value, tableName, asName, sq.Model.IDField()) + + if shouldCache { + columnCacheMutex.Lock() + columnCache[key] = cols + columnCacheMutex.Unlock() + } + return cols } diff --git a/sql_builder_test.go b/sql_builder_test.go new file mode 100644 index 00000000..b4cb46cc --- /dev/null +++ b/sql_builder_test.go @@ -0,0 +1,74 @@ +package pop + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +type v struct{} +type vv []v +type vptr *v +type vvptr []*v + +func TestCacheKey(t *testing.T) { + tests := []struct { + name string + val any + want string + wantOK bool + }{{ + name: "struct value", + val: v{}, + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "struct pointer", + val: &v{}, + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "vptr", + val: vptr(nil), + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "slice of struct values", + val: vv{}, + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "slice of struct pointers", + val: vvptr{}, + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "slice of struct pointer nil", + val: vvptr{nil}, + want: "github.com/ory/pop/v6.v", + wantOK: true, + }, { + name: "string", + val: "hello", + }, { + name: "nil", + val: nil, + }, { + name: "int", + val: 42, + }, { + name: "int pointer", + val: func() *int { i := 42; return &i }(), + }, { + name: "map", + val: map[string]int{"a": 1}, + }} + + for _, tc := range tests { + t.Run("case="+tc.name, func(t *testing.T) { + got, ok := cacheKey(tc.val) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.want, got) + }) + } +}