From 7d4aa4cdabad3bcf2fb4e855680592776c8d4229 Mon Sep 17 00:00:00 2001 From: go-jet Date: Fri, 16 Jan 2026 14:11:22 +0100 Subject: [PATCH] Fix for SELECT_JSON_ARR generates incorrect order by, limit and offset clauses for mariadb. --- internal/jet/clause.go | 12 +++ internal/testutils/test_utils.go | 14 ++++ mysql/select_json.go | 59 +++++++++++--- tests/mysql/select_json_test.go | 131 ++++++++++++++++++++++--------- 4 files changed, 168 insertions(+), 48 deletions(-) diff --git a/internal/jet/clause.go b/internal/jet/clause.go index dab2f81..5604209 100644 --- a/internal/jet/clause.go +++ b/internal/jet/clause.go @@ -190,6 +190,10 @@ type ClauseOrderBy struct { SkipNewLine bool } +func (o *ClauseOrderBy) serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { + o.Serialize(statementType, out, options...) +} + // Serialize serializes clause into SQLBuilder func (o *ClauseOrderBy) Serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { if o.List == nil { @@ -219,6 +223,10 @@ type ClauseLimit struct { Count int64 } +func (o *ClauseLimit) serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { + o.Serialize(statementType, out, options...) +} + // Serialize serializes clause into SQLBuilder func (l *ClauseLimit) Serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { if l.Count >= 0 { @@ -233,6 +241,10 @@ type ClauseOffset struct { Count IntegerExpression } +func (o *ClauseOffset) serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { + o.Serialize(statementType, out, options...) +} + // Serialize serializes clause into SQLBuilder func (o *ClauseOffset) Serialize(statementType StatementType, out *SQLBuilder, options ...SerializeOption) { if is.Nil(o.Count) { diff --git a/internal/testutils/test_utils.go b/internal/testutils/test_utils.go index 88f00ab..f2a2c8d 100644 --- a/internal/testutils/test_utils.go +++ b/internal/testutils/test_utils.go @@ -136,6 +136,20 @@ func SaveJSONFile(v interface{}, testRelativePath string) { throw.OnError(err) } +func ReadJSONFile(t require.TestingT, testRelativePath string, dest any) { + if _, ok := t.(*testing.B); ok { + return // skip assert for benchmarks + } + + filePath := getFullPath(testRelativePath) + fileJSONData, err := os.ReadFile(filePath) // #nosec G304 + require.NoError(t, err) + + err = json.Unmarshal(fileJSONData, dest) + + require.NoError(t, err) +} + // AssertJSONFile check if data json representation is the same as json at testRelativePath func AssertJSONFile(t require.TestingT, data interface{}, testRelativePath string) { if _, ok := t.(*testing.B); ok { diff --git a/mysql/select_json.go b/mysql/select_json.go index cfeb30f..914f885 100644 --- a/mysql/select_json.go +++ b/mysql/select_json.go @@ -30,26 +30,46 @@ func SELECT_JSON_OBJ(projections ...Projection) SelectJsonStatement { type selectJsonStatement struct { *selectStatementImpl + + projections []Projection + statementType jet.StatementType + + // SELECT_JSON_ARR internal clauses + arrOrderBy *jet.ClauseOrderBy + arrLimit *jet.ClauseLimit + arrOffset *jet.ClauseOffset } func newSelectStatementJson(projections []Projection, statementType jet.StatementType) SelectJsonStatement { - newSelect := &selectJsonStatement{ + newSelectJson := &selectJsonStatement{ selectStatementImpl: newSelectStatement(statementType, nil, nil), + + projections: projections, + statementType: statementType, + + arrOrderBy: &jet.ClauseOrderBy{}, + arrLimit: &jet.ClauseLimit{Count: -1}, + arrOffset: &jet.ClauseOffset{}, } - newSelect.Select.ProjectionList = ProjectionList{constructJsonFunc(projections, statementType).AS("json")} + newSelectJson.constructProjectionList() - return newSelect + return newSelectJson } -func constructJsonFunc(projections []Projection, statementType jet.StatementType) Expression { - jsonObj := Func("JSON_OBJECT", CustomExpression(jet.JsonObjProjectionList(projections))) +func (s *selectJsonStatement) constructProjectionList() { + jsonProjection := Func("JSON_OBJECT", CustomExpression(jet.JsonObjProjectionList(s.projections))) - if statementType == jet.SelectJsonArrStatementType { - return Func("JSON_ARRAYAGG", jsonObj) + if s.statementType == jet.SelectJsonArrStatementType { + jsonProjection = Func("JSON_ARRAYAGG", CustomExpression( + jsonProjection, + s.arrOrderBy, + s.arrLimit, + s.arrOffset, + )) } - return jsonObj + s.Select.ProjectionList = ProjectionList{jsonProjection.AS("json")} } func (s *selectJsonStatement) FROM(table ReadableTable) SelectJsonStatement { @@ -63,17 +83,32 @@ func (s *selectJsonStatement) WHERE(condition BoolExpression) SelectJsonStatemen return s } -func (s *selectJsonStatement) ORDER_BY(orderByClauses ...OrderByClause) SelectJsonStatement { - s.OrderBy.List = orderByClauses +func (s *selectJsonStatement) ORDER_BY(orderBy ...OrderByClause) SelectJsonStatement { + if s.statementType == jet.SelectJsonArrStatementType { + s.arrOrderBy.List = orderBy + } else { + s.OrderBy.List = orderBy + } + return s } func (s *selectJsonStatement) LIMIT(limit int64) SelectJsonStatement { - s.Limit.Count = limit + if s.statementType == jet.SelectJsonArrStatementType { + s.arrLimit.Count = limit + } else { + s.Limit.Count = limit + } + return s } func (s *selectJsonStatement) OFFSET(offset int64) SelectJsonStatement { - s.Offset.Count = Int(offset) + if s.statementType == jet.SelectJsonArrStatementType { + s.arrOffset.Count = Int(offset) + } else { + s.Offset.Count = Int(offset) + } + return s } diff --git a/tests/mysql/select_json_test.go b/tests/mysql/select_json_test.go index a50afb4..390bfe3 100644 --- a/tests/mysql/select_json_test.go +++ b/tests/mysql/select_json_test.go @@ -2,8 +2,8 @@ package mysql import ( "context" - "fmt" "github.com/go-jet/jet/v2/qrm" + "slices" "strings" "testing" @@ -127,32 +127,91 @@ WHERE actor.actor_id = ?; } func TestSelectJsonArr(t *testing.T) { - stmt := SELECT_JSON_ARR(Actor.AllColumns). - FROM(Actor). - ORDER_BY(Actor.ActorID) + onlyMariaDB(t) - testutils.AssertDebugStatementSql(t, stmt, ` + var savedActors []model.Actor + testutils.ReadJSONFile(t, "./testdata/results/mysql/all_actors.json", &savedActors) + + t.Run("order by", func(t *testing.T) { + stmt := SELECT_JSON_ARR(Actor.AllColumns). + FROM(Actor). + ORDER_BY( + Actor.LastName.DESC(), + Actor.FirstName.ASC(), + Actor.ActorID.ASC(), + ) + + testutils.AssertDebugStatementSql(t, stmt, ` SELECT JSON_ARRAYAGG(JSON_OBJECT( 'actorID', actor.actor_id, 'firstName', actor.first_name, 'lastName', actor.last_name, 'lastUpdate', DATE_FORMAT(actor.last_update,'%Y-%m-%dT%H:%i:%s.%fZ') - )) AS "json" -FROM dvds.actor -ORDER BY actor.actor_id; + ) + ORDER BY actor.last_name DESC, actor.first_name ASC, actor.actor_id ASC) AS "json" +FROM dvds.actor; `) - var dest []model.Actor + var dest []model.Actor - err := stmt.Query(db, &dest) - require.Nil(t, err) + err := stmt.Query(db, &dest) + require.Nil(t, err) + + // sort by actor.LastName desc + slices.SortFunc(savedActors, func(a, b model.Actor) int { + if l := strings.Compare(b.LastName, a.LastName); l != 0 { + return l + } + + if f := strings.Compare(a.FirstName, b.FirstName); f != 0 { + return f + } + + return int(a.ActorID) - int(b.ActorID) + }) + + require.Equal(t, dest, savedActors) + requireLogged(t, stmt) + requireQueryLogged(t, stmt, 1) + }) + + t.Run("order by, limit, offset", func(t *testing.T) { + stmt := SELECT_JSON_ARR(Actor.AllColumns). + FROM(Actor). + ORDER_BY(Actor.ActorID.DESC()). + LIMIT(5). + OFFSET(10) + + testutils.AssertStatementSql(t, stmt, ` +SELECT JSON_ARRAYAGG(JSON_OBJECT( + 'actorID', actor.actor_id, + 'firstName', actor.first_name, + 'lastName', actor.last_name, + 'lastUpdate', DATE_FORMAT(actor.last_update,'%Y-%m-%dT%H:%i:%s.%fZ') + ) + ORDER BY actor.actor_id DESC + LIMIT ? + OFFSET ?) AS "json" +FROM dvds.actor; +`) + + var dest []model.Actor + + err := stmt.Query(db, &dest) + require.Nil(t, err) + + slices.SortFunc(savedActors, func(a, b model.Actor) int { + return int(b.ActorID) - int(a.ActorID) + }) + + require.Equal(t, dest, savedActors[10:15]) + }) - testutils.AssertJSONFile(t, dest, "./testdata/results/mysql/all_actors.json") - requireLogged(t, stmt) - requireQueryLogged(t, stmt, 1) } func TestSelectJsonArr_NestedArr(t *testing.T) { + onlyMariaDB(t) + stmt := SELECT_JSON_ARR( Actor.AllColumns, @@ -198,16 +257,16 @@ SELECT JSON_ARRAYAGG(JSON_OBJECT( 'rating', film.rating, 'specialFeatures', film.special_features, 'lastUpdate', DATE_FORMAT(film.last_update,'%Y-%m-%dT%H:%i:%s.%fZ') - )) AS "json" + ) + ORDER BY film.length DESC) AS "json" FROM dvds.film_actor INNER JOIN dvds.film ON ((film.film_id = film_actor.film_id) AND (actor.actor_id = film_actor.actor_id)) WHERE (film.film_id % 17) = 0 - ORDER BY film.length DESC ) - )) AS "json" + ) + ORDER BY actor.actor_id) AS "json" FROM dvds.actor -WHERE actor.actor_id BETWEEN 1 AND 3 -ORDER BY actor.actor_id; +WHERE actor.actor_id BETWEEN 1 AND 3; `) var dest []struct { @@ -217,8 +276,8 @@ ORDER BY actor.actor_id; } err := stmt.QueryContext(ctx, db, &dest) - fmt.Println(err) - require.Nil(t, err) + require.NoError(t, err) + testutils.AssertJSON(t, dest, ` [ { @@ -234,21 +293,6 @@ ORDER BY actor.actor_id; "LastName": "WAHLBERG", "LastUpdate": "2006-02-15T04:34:33Z", "Films": [ - { - "FilmID": 357, - "Title": "GILBERT PELICAN", - "Description": "A Fateful Tale of a Man And a Feminist who must Conquer a Crocodile in A Manhattan Penthouse", - "ReleaseYear": 2006, - "LanguageID": 1, - "OriginalLanguageID": null, - "RentalDuration": 7, - "RentalRate": 0.99, - "Length": 114, - "ReplacementCost": 13.99, - "Rating": "G", - "SpecialFeatures": "Trailers,Commentaries", - "LastUpdate": "2006-02-15T05:03:42Z" - }, { "FilmID": 561, "Title": "MASK PEACH", @@ -263,6 +307,21 @@ ORDER BY actor.actor_id; "Rating": "NC-17", "SpecialFeatures": "Commentaries,Deleted Scenes", "LastUpdate": "2006-02-15T05:03:42Z" + }, + { + "FilmID": 357, + "Title": "GILBERT PELICAN", + "Description": "A Fateful Tale of a Man And a Feminist who must Conquer a Crocodile in A Manhattan Penthouse", + "ReleaseYear": 2006, + "LanguageID": 1, + "OriginalLanguageID": null, + "RentalDuration": 7, + "RentalRate": 0.99, + "Length": 114, + "ReplacementCost": 13.99, + "Rating": "G", + "SpecialFeatures": "Trailers,Commentaries", + "LastUpdate": "2006-02-15T05:03:42Z" } ] },