Skip to content

Commit aa559f6

Browse files
committed
fix(sqlstorage): fix cursor pagination with json tag lookup and placeholder handling
* Use JSON struct tags instead of Go field names for cursor value extraction * Remove PostgreSQL pre-conversion that conflicted with GORM's placeholders
1 parent 18068a4 commit aa559f6

4 files changed

Lines changed: 40 additions & 32 deletions

File tree

storage/search/lucene/parser_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,37 +134,37 @@ func TestBasicFieldSearch(t *testing.T) {
134134
{
135135
name: "simple field query",
136136
query: "name:john",
137-
wantSQL: []string{`"name"`, "=", "$1"},
137+
wantSQL: []string{`"name"`, "=", "?"},
138138
wantNot: []string{"ILIKE", "LIKE"},
139139
wantParams: []any{"john"},
140140
wantErr: false,
141141
},
142142
{
143143
name: "wildcard prefix",
144144
query: "name:john*",
145-
wantSQL: []string{`"name"`, "ILIKE", "$1"},
145+
wantSQL: []string{`"name"`, "ILIKE", "?"},
146146
wantNot: []string{"="},
147147
wantParams: []any{"john%"},
148148
wantErr: false,
149149
},
150150
{
151151
name: "wildcard suffix",
152152
query: "name:*john",
153-
wantSQL: []string{`"name"`, "ILIKE", "$1"},
153+
wantSQL: []string{`"name"`, "ILIKE", "?"},
154154
wantParams: []any{"%john"},
155155
wantErr: false,
156156
},
157157
{
158158
name: "wildcard contains",
159159
query: "name:*john*",
160-
wantSQL: []string{`"name"`, "ILIKE", "$1"},
160+
wantSQL: []string{`"name"`, "ILIKE", "?"},
161161
wantParams: []any{"%john%"},
162162
wantErr: false,
163163
},
164164
{
165165
name: "email field",
166166
query: `email:"test@example.com"`,
167-
wantSQL: []string{`"email"`, "=", "$1"},
167+
wantSQL: []string{`"email"`, "=", "?"},
168168
wantParams: []any{"test@example.com"},
169169
wantErr: false,
170170
},
@@ -706,7 +706,7 @@ func TestNullValueQueries(t *testing.T) {
706706
name: "field is null (lowercase)",
707707
query: "parent_id:null",
708708
wantSQL: []string{`"parent_id"`, "IS NULL"},
709-
wantNot: []string{"=", "$1"},
709+
wantNot: []string{"=", "?"},
710710
wantParams: []any{},
711711
wantErr: false,
712712
},
@@ -741,7 +741,7 @@ func TestNullValueQueries(t *testing.T) {
741741
{
742742
name: "nil should be treated as literal value (not NULL)",
743743
query: "name:nil",
744-
wantSQL: []string{`"name"`, "=", "$1"},
744+
wantSQL: []string{`"name"`, "=", "?"},
745745
wantParams: []any{"nil"},
746746
wantErr: false,
747747
},
@@ -1206,8 +1206,8 @@ func TestParser_ProviderSpecific(t *testing.T) {
12061206
name: "postgresql placeholder",
12071207
query: "name:john",
12081208
provider: "postgresql",
1209-
wantSQL: []string{"$1"},
1210-
wantNot: []string{"?"},
1209+
wantSQL: []string{"?"},
1210+
wantNot: []string{"$"},
12111211
wantParams: []any{"john"},
12121212
wantErr: false,
12131213
},

storage/search/lucene/sql_driver.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,10 @@ func (s *SQLDriver) RenderParam(e *expr.Expression) (string, []any, error) {
8282
return "", nil, err
8383
}
8484

85-
// Convert ? placeholders to provider-specific format
86-
// PostgreSQL uses $1, $2, $3; MySQL and SQLite use ?
87-
switch s.provider {
88-
case "postgresql":
89-
str = convertToPostgresPlaceholders(str)
90-
case "mysql", "sqlite":
91-
// Already uses ? placeholders, no conversion needed
92-
}
85+
// Keep ? placeholders for all providers.
86+
// GORM's PostgreSQL driver handles ? → $N conversion automatically,
87+
// so pre-converting here would conflict with additional WHERE clauses
88+
// (e.g. cursor pagination) that GORM adds with its own ? placeholders.
9389

9490
return str, params, nil
9591
}
@@ -404,7 +400,7 @@ func validateSubFieldName(subField string) error {
404400
if subField == "" {
405401
return fmt.Errorf("subfield name cannot be empty")
406402
}
407-
403+
408404
if !jsonSubFieldPattern.MatchString(subField) {
409405
return fmt.Errorf("invalid subfield name '%s': contains unsafe characters (only alphanumeric, underscore, and dot allowed)", subField)
410406
}

storage/search/lucene/sql_driver_test.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,9 @@ func TestSQLDriver_RenderParam(t *testing.T) {
204204
if len(params) != tt.wantCount {
205205
t.Errorf("RenderParam() params count = %v, want %v", len(params), tt.wantCount)
206206
}
207-
if provider == "postgresql" {
208-
if !strings.Contains(sql, "$") {
209-
t.Errorf("RenderParam() expected PostgreSQL placeholders ($1, $2), got %v", sql)
210-
}
211-
} else {
212-
if strings.Contains(sql, "$") && !strings.Contains(sql, "?") {
213-
t.Errorf("RenderParam() expected ? placeholders for %v, got %v", provider, sql)
214-
}
207+
// All providers use ? placeholders; GORM handles $N conversion for PostgreSQL
208+
if tt.wantCount > 0 && !strings.Contains(sql, "?") {
209+
t.Errorf("RenderParam() expected ? placeholders for %v, got %v", provider, sql)
215210
}
216211
})
217212
}
@@ -1443,11 +1438,11 @@ func TestSQLDriver_ProviderSpecific(t *testing.T) {
14431438
Left: expr.Column("name"),
14441439
Right: &expr.Expression{Op: expr.Literal, Left: "john"},
14451440
},
1446-
wantSQL: []string{"$1"},
1441+
wantSQL: []string{"?"},
14471442
wantCount: 1,
14481443
checkFunc: func(t *testing.T, sql string, params []any) {
1449-
if !strings.Contains(sql, "$") {
1450-
t.Errorf("expected PostgreSQL placeholder ($1), got %v", sql)
1444+
if !strings.Contains(sql, "?") {
1445+
t.Errorf("expected ? placeholder, got %v", sql)
14511446
}
14521447
},
14531448
},

storage/sql.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,11 @@ func (s *SQLAdapter) executePaginatedQuery(
260260
nextCursor := ""
261261
if destSlice.Len() > limit {
262262
lastItem := destSlice.Index(limit - 1)
263-
field := reflect.Indirect(lastItem).FieldByName(sortKey)
263+
field := findFieldByJSONTag(reflect.Indirect(lastItem), sortKey)
264264
if !field.IsValid() {
265-
slog.Warn("cursor extraction failed: sort_key does not match any exported struct field",
265+
slog.Warn("cursor extraction failed: sort_key does not match any json tag on the struct",
266266
"sort_key", sortKey,
267-
"hint", "sort_key must be the Go struct field name (e.g. 'CreatedAt'), not the DB column name (e.g. 'created_at')")
267+
"hint", "sort_key must match a json tag (e.g. 'created_at'), not the Go field name (e.g. 'CreatedAt')")
268268
} else if field.Kind() != reflect.String {
269269
slog.Warn("cursor extraction failed: struct field is not a string",
270270
"sort_key", sortKey,
@@ -278,6 +278,23 @@ func (s *SQLAdapter) executePaginatedQuery(
278278
return nextCursor, nil
279279
}
280280

281+
// findFieldByJSONTag looks up a struct field by its json tag name.
282+
// This is needed because sortKey uses the JSON/column name (e.g. "id")
283+
// while Go struct fields use PascalCase (e.g. "Id").
284+
func findFieldByJSONTag(v reflect.Value, tag string) reflect.Value {
285+
t := v.Type()
286+
for i := 0; i < t.NumField(); i++ {
287+
jsonTag := t.Field(i).Tag.Get("json")
288+
if idx := strings.Index(jsonTag, ","); idx != -1 {
289+
jsonTag = jsonTag[:idx]
290+
}
291+
if jsonTag == tag {
292+
return v.Field(i)
293+
}
294+
}
295+
return reflect.Value{}
296+
}
297+
281298
func (s *SQLAdapter) List(dest any, sortKey string, filter map[string]any, limit int, cursor string, params ...map[string]any) (string, error) {
282299
sortDirection, err := extractSortDirection(extractParams(params...))
283300
if err != nil {

0 commit comments

Comments
 (0)