fix(pagination): add table name to fields in keyset pagination (#699)

ory · Jun 26, 2023 · 0d30de8 · 0d30de8
1 parent ffcc8bd
commit 0d30de8
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 15 deletions.
diff --git a/pagination/keysetpagination/paginator.go b/pagination/keysetpagination/paginator.go
@@ -84,33 +84,33 @@ func (p *Paginator) ToOptions() []Option {
 	}
 }
 
-func (p *Paginator) multipleOrderFieldsQuery(q *pop.Query, idField string, cols map[string]*columns.Column, quote func(string) string) {
+func (p *Paginator) multipleOrderFieldsQuery(q *pop.Query, idField string, cols map[string]*columns.Column, quoteAndContextualize func(string) string) {
 	tokenParts := p.Token().Parse(idField)
 	idValue := tokenParts[idField]
 
 	column, ok := cols[p.additionalColumn.name]
 	if !ok {
-		q.Where(fmt.Sprintf(`%s > ?`, quote(idField)), idValue)
+		q.Where(fmt.Sprintf(`%s > ?`, quoteAndContextualize(idField)), idValue)
 		return
 	}
 
-	quoteName := quote(column.Name)
+	quoteName := quoteAndContextualize(column.Name)
 
 	value, ok := tokenParts[column.Name]
 
 	if !ok {
-		q.Where(fmt.Sprintf(`%s > ?`, quote(idField)), idValue)
+		q.Where(fmt.Sprintf(`%s > ?`, quoteAndContextualize(idField)), idValue)
 		return
 	}
 
 	sign, keyword, err := p.additionalColumn.order.extract()
 	if err != nil {
-		q.Where(fmt.Sprintf(`%s > ?`, quote(idField)), idValue)
+		q.Where(fmt.Sprintf(`%s > ?`, quoteAndContextualize(idField)), idValue)
 		return
 	}
 
 	q.
-		Where(fmt.Sprintf("(%s %s ? OR (%s = ? AND %s > ?))", quoteName, sign, quoteName, quote(idField)), value, value, idValue).
+		Where(fmt.Sprintf("(%s %s ? OR (%s = ? AND %s > ?))", quoteName, sign, quoteName, quoteAndContextualize(idField)), value, value, idValue).
 		Order(fmt.Sprintf("%s %s", quoteName, keyword))
 
 }
@@ -130,10 +130,14 @@ func Paginate[I any, PI interface {
 }](p *Paginator) pop.ScopeFunc {
 	model := pop.Model{Value: new(I)}
 	id := model.IDField()
+	tableName := model.Alias()
 	return func(q *pop.Query) *pop.Query {
-		eid := q.Connection.Dialect.Quote(id)
+		eid := q.Connection.Dialect.Quote(tableName + "." + id)
 
-		p.multipleOrderFieldsQuery(q, id, model.Columns().Cols, q.Connection.Dialect.Quote)
+		quoteAndContextualize := func(name string) string {
+			return q.Connection.Dialect.Quote(tableName + "." + name)
+		}
+		p.multipleOrderFieldsQuery(q, id, model.Columns().Cols, quoteAndContextualize)
 
 		return q.
 			Limit(p.Size() + 1).

diff --git a/pagination/keysetpagination/paginator_test.go b/pagination/keysetpagination/paginator_test.go
@@ -35,7 +35,7 @@ func TestPaginator(t *testing.T) {
 		q = q.Scope(Paginate[testItem](paginator))
 
 		sql, args := q.ToSQL(&pop.Model{Value: new(testItem)})
-		assert.Equal(t, "SELECT test_items.created_at, test_items.pk FROM test_items AS test_items WHERE \"pk\" > $1 ORDER BY \"pk\" ASC LIMIT 11", sql)
+		assert.Equal(t, `SELECT test_items.created_at, test_items.pk FROM test_items AS test_items WHERE "test_items"."pk" > $1 ORDER BY "test_items"."pk" ASC LIMIT 11`, sql)
 		assert.Equal(t, []interface{}{"token"}, args)
 	})
 
@@ -49,7 +49,7 @@ func TestPaginator(t *testing.T) {
 		q = q.Scope(Paginate[testItem](paginator))
 
 		sql, args := q.ToSQL(&pop.Model{Value: new(testItem)})
-		assert.Equal(t, "SELECT test_items.created_at, test_items.pk FROM test_items AS test_items WHERE `pk` > ? ORDER BY `pk` ASC LIMIT 11", sql)
+		assert.Equal(t, "SELECT test_items.created_at, test_items.pk FROM test_items AS test_items WHERE `test_items.pk` > ? ORDER BY `test_items.pk` ASC LIMIT 11", sql)
 		assert.Equal(t, []interface{}{"token"}, args)
 	})
 
@@ -185,31 +185,31 @@ func TestPaginateWithAdditionalColumn(t *testing.T) {
 		{
 			d:    "with sort by created_at DESC",
 			opts: []Option{WithToken(MapPageToken{"pk": "token_value", "created_at": "timestamp"}), WithColumn("created_at", "DESC")},
-			e:    `WHERE ("created_at" < $1 OR ("created_at" = $2 AND "pk" > $3)) ORDER BY "created_at" DESC, "pk" ASC`,
+			e:    `WHERE ("test_items"."created_at" < $1 OR ("test_items"."created_at" = $2 AND "test_items"."pk" > $3)) ORDER BY "test_items"."created_at" DESC, "test_items"."pk" ASC`,
 			args: []interface{}{"timestamp", "timestamp", "token_value"},
 		},
 		{
 			d:    "with sort by created_at ASC",
 			opts: []Option{WithToken(MapPageToken{"pk": "token_value", "created_at": "timestamp"}), WithColumn("created_at", "ASC")},
-			e:    `WHERE ("created_at" > $1 OR ("created_at" = $2 AND "pk" > $3)) ORDER BY "created_at" ASC, "pk" ASC`,
+			e:    `WHERE ("test_items"."created_at" > $1 OR ("test_items"."created_at" = $2 AND "test_items"."pk" > $3)) ORDER BY "test_items"."created_at" ASC, "test_items"."pk" ASC`,
 			args: []interface{}{"timestamp", "timestamp", "token_value"},
 		},
 		{
 			d:    "with unknown column",
 			opts: []Option{WithToken(MapPageToken{"pk": "token_value", "created_at": "timestamp"}), WithColumn("unknown_column", "ASC")},
-			e:    `WHERE "pk" > $1 ORDER BY "pk"`,
+			e:    `WHERE "test_items"."pk" > $1 ORDER BY "test_items"."pk"`,
 			args: []interface{}{"token_value"},
 		},
 		{
 			d:    "with no token value",
 			opts: []Option{WithToken(MapPageToken{"pk": "token_value"}), WithColumn("created_at", "ASC")},
-			e:    `WHERE "pk" > $1 ORDER BY "pk"`,
+			e:    `WHERE "test_items"."pk" > $1 ORDER BY "test_items"."pk"`,
 			args: []interface{}{"token_value"},
 		},
 		{
 			d:    "with unknown order",
 			opts: []Option{WithToken(MapPageToken{"pk": "token_value", "created_at": "timestamp"}), WithColumn("created_at", Order("unknown order"))},
-			e:    `WHERE "pk" > $1 ORDER BY "pk"`,
+			e:    `WHERE "test_items"."pk" > $1 ORDER BY "test_items"."pk"`,
 			args: []interface{}{"token_value"},
 		},
 	} {