From 790309db8be6930ecc821ff308feb8ff9ab1c9ab Mon Sep 17 00:00:00 2001 From: Andriy Massimilla Date: Wed, 14 May 2025 10:13:25 -0400 Subject: [PATCH 1/2] Fix order by clause in sqlite --- .../testdata/order_by_binds/sqlite/exec.json | 3 + .../testdata/order_by_binds/sqlite/go/db.go | 31 ++++++ .../order_by_binds/sqlite/go/models.go | 15 +++ .../order_by_binds/sqlite/go/query.sql.go | 101 ++++++++++++++++++ .../testdata/order_by_binds/sqlite/query.sql | 13 +++ .../testdata/order_by_binds/sqlite/schema.sql | 5 + .../testdata/order_by_binds/sqlite/sqlc.json | 12 +++ internal/engine/sqlite/convert.go | 61 +++++++++++ 8 files changed, 241 insertions(+) create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/exec.json create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/go/db.go create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/go/models.go create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/query.sql create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/schema.sql create mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/exec.json b/internal/endtoend/testdata/order_by_binds/sqlite/exec.json new file mode 100644 index 0000000000..2e996ca79d --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/exec.json @@ -0,0 +1,3 @@ +{ + "contexts": ["base"] +} diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go new file mode 100644 index 0000000000..a92cd6e8eb --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go @@ -0,0 +1,31 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.29.0 + +package querytest + +import ( + "context" + "database/sql" +) + +type DBTX interface { + ExecContext(context.Context, string, ...interface{}) (sql.Result, error) + PrepareContext(context.Context, string) (*sql.Stmt, error) + QueryContext(context.Context, string, ...interface{}) (*sql.Rows, error) + QueryRowContext(context.Context, string, ...interface{}) *sql.Row +} + +func New(db DBTX) *Queries { + return &Queries{db: db} +} + +type Queries struct { + db DBTX +} + +func (q *Queries) WithTx(tx *sql.Tx) *Queries { + return &Queries{ + db: tx, + } +} diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go new file mode 100644 index 0000000000..d3b600c584 --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go @@ -0,0 +1,15 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.29.0 + +package querytest + +import ( + "database/sql" +) + +type Author struct { + ID int64 + Name string + Bio sql.NullString +} diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go new file mode 100644 index 0000000000..804844cbd8 --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go @@ -0,0 +1,101 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.29.0 +// source: query.sql + +package querytest + +import ( + "context" +) + +const listAuthorsColumnSort = `-- name: ListAuthorsColumnSort :many +SELECT id, name, bio FROM authors +WHERE id > ?1 +ORDER BY CASE WHEN ?2 = 'name' THEN name END +` + +type ListAuthorsColumnSortParams struct { + MinID int64 + SortColumn interface{} +} + +func (q *Queries) ListAuthorsColumnSort(ctx context.Context, arg ListAuthorsColumnSortParams) ([]Author, error) { + rows, err := q.db.QueryContext(ctx, listAuthorsColumnSort, arg.MinID, arg.SortColumn) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Author + for rows.Next() { + var i Author + if err := rows.Scan(&i.ID, &i.Name, &i.Bio); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listAuthorsColumnSortFnWtihArg = `-- name: ListAuthorsColumnSortFnWtihArg :many +SELECT id, name, bio FROM authors +ORDER BY id % ? +` + +func (q *Queries) ListAuthorsColumnSortFnWtihArg(ctx context.Context, id int64) ([]Author, error) { + rows, err := q.db.QueryContext(ctx, listAuthorsColumnSortFnWtihArg, id) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Author + for rows.Next() { + var i Author + if err := rows.Scan(&i.ID, &i.Name, &i.Bio); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listAuthorsNameSort = `-- name: ListAuthorsNameSort :many +SELECT id, name, bio FROM authors +WHERE id > ?1 +ORDER BY name ASC +` + +func (q *Queries) ListAuthorsNameSort(ctx context.Context, minID int64) ([]Author, error) { + rows, err := q.db.QueryContext(ctx, listAuthorsNameSort, minID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Author + for rows.Next() { + var i Author + if err := rows.Scan(&i.ID, &i.Name, &i.Bio); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/query.sql b/internal/endtoend/testdata/order_by_binds/sqlite/query.sql new file mode 100644 index 0000000000..72b1f21f4d --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/query.sql @@ -0,0 +1,13 @@ +-- name: ListAuthorsColumnSort :many +SELECT * FROM authors +WHERE id > sqlc.arg(min_id) +ORDER BY CASE WHEN sqlc.arg(sort_column) = 'name' THEN name END; + +-- name: ListAuthorsColumnSortFnWtihArg :many +SELECT * FROM authors +ORDER BY id % ?; + +-- name: ListAuthorsNameSort :many +SELECT * FROM authors +WHERE id > sqlc.arg(min_id) +ORDER BY name ASC; diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/schema.sql b/internal/endtoend/testdata/order_by_binds/sqlite/schema.sql new file mode 100644 index 0000000000..eaa9aab806 --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/schema.sql @@ -0,0 +1,5 @@ +CREATE TABLE authors ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL, + bio TEXT +); diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json b/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json new file mode 100644 index 0000000000..cd66df063b --- /dev/null +++ b/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json @@ -0,0 +1,12 @@ +{ + "version": "1", + "packages": [ + { + "path": "go", + "engine": "sqlite", + "name": "querytest", + "schema": "schema.sql", + "queries": "query.sql" + } + ] +} diff --git a/internal/engine/sqlite/convert.go b/internal/engine/sqlite/convert.go index 29c06fb285..d8530354cd 100644 --- a/internal/engine/sqlite/convert.go +++ b/internal/engine/sqlite/convert.go @@ -512,8 +512,11 @@ func (c *cc) convertMultiSelect_stmtContext(n *parser.Select_stmtContext) ast.No } limitCount, limitOffset := c.convertLimit_stmtContext(n.Limit_stmt()) + sortClause := c.buildSortClause(n.Order_by_stmt()) + selectStmt.LimitCount = limitCount selectStmt.LimitOffset = limitOffset + selectStmt.SortClause = sortClause selectStmt.WithClause = &ast.WithClause{Ctes: &ctes} return selectStmt } @@ -1212,3 +1215,61 @@ func (c *cc) convert(node node) ast.Node { return todo("convert(case=default)", n) } } + +// buildSortClause converts an IOrder_by_stmtContext into an *ast.List of *ast.SortBy nodes. +func (c *cc) buildSortClause(orderByNode parser.IOrder_by_stmtContext) *ast.List { + if orderByNode == nil { + return nil + } + + orderByCtx, ok := orderByNode.(*parser.Order_by_stmtContext) + if !ok { + if debug.Active { + log.Printf("sqlite.buildSortClause: unexpected type %T for IOrder_by_stmtContext", orderByNode) + } + return nil + } + + if len(orderByCtx.AllOrdering_term()) == 0 { + return nil + } + + sortItems := &ast.List{Items: []ast.Node{}} + for _, otermIP := range orderByCtx.AllOrdering_term() { + oterm, ok := otermIP.(*parser.Ordering_termContext) + if !ok { + if debug.Active { + log.Printf("sqlite.buildSortClause: unexpected type %T for IOrdering_termContext", otermIP) + } + continue + } + + sortByDir := ast.SortByDirDefault + if adNode := oterm.Asc_desc(); adNode != nil { + // Asc_descContext has ASC_() and DESC_() methods which return TerminalNode + if adNode.ASC_() != nil { + sortByDir = ast.SortByDirAsc + } else if adNode.DESC_() != nil { + sortByDir = ast.SortByDirDesc + } + } + + sortByNulls := ast.SortByNullsDefault + if oterm.NULLS_() != nil { // NULLS_() is a TerminalNode + if oterm.FIRST_() != nil { // FIRST_() is a TerminalNode + sortByNulls = ast.SortByNullsFirst + } else if oterm.LAST_() != nil { // LAST_() is a TerminalNode + sortByNulls = ast.SortByNullsLast + } + } + + sortItems.Items = append(sortItems.Items, &ast.SortBy{ + Node: c.convert(oterm.Expr()), + SortbyDir: sortByDir, + SortbyNulls: sortByNulls, + UseOp: &ast.List{}, // Typically empty for standard SQLite ORDER BY + Location: oterm.GetStart().GetStart(), + }) + } + return sortItems +} From 1c9ee2ec91396c442b5aa92b1a565b887ec1be83 Mon Sep 17 00:00:00 2001 From: Andriy Massimilla Date: Thu, 15 May 2025 16:17:28 -0400 Subject: [PATCH 2/2] Add test for mixed param case --- .../testdata/order_by_binds/sqlite/exec.json | 3 -- .../testdata/order_by_binds/sqlite/go/db.go | 2 +- .../order_by_binds/sqlite/go/models.go | 2 +- .../order_by_binds/sqlite/go/query.sql.go | 42 ++++++++++++++++++- .../testdata/order_by_binds/sqlite/query.sql | 11 +++++ .../testdata/order_by_binds/sqlite/sqlc.json | 2 +- 6 files changed, 55 insertions(+), 7 deletions(-) delete mode 100644 internal/endtoend/testdata/order_by_binds/sqlite/exec.json diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/exec.json b/internal/endtoend/testdata/order_by_binds/sqlite/exec.json deleted file mode 100644 index 2e996ca79d..0000000000 --- a/internal/endtoend/testdata/order_by_binds/sqlite/exec.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "contexts": ["base"] -} diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go index a92cd6e8eb..2ccb891ff2 100644 --- a/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/db.go @@ -2,7 +2,7 @@ // versions: // sqlc v1.29.0 -package querytest +package order_by_binds import ( "context" diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go index d3b600c584..64954a0b2a 100644 --- a/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/models.go @@ -2,7 +2,7 @@ // versions: // sqlc v1.29.0 -package querytest +package order_by_binds import ( "database/sql" diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go b/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go index 804844cbd8..1d51ba7d68 100644 --- a/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go +++ b/internal/endtoend/testdata/order_by_binds/sqlite/go/query.sql.go @@ -3,7 +3,7 @@ // sqlc v1.29.0 // source: query.sql -package querytest +package order_by_binds import ( "context" @@ -43,6 +43,46 @@ func (q *Queries) ListAuthorsColumnSort(ctx context.Context, arg ListAuthorsColu return items, nil } +const listAuthorsColumnSortDirection = `-- name: ListAuthorsColumnSortDirection :many +SELECT id, name, bio FROM authors +WHERE id > ? +ORDER BY + CASE + WHEN ? = 'asc' THEN name + END ASC, + CASE + WHEN ? = 'desc' OR ? IS NULL THEN name + END DESC +` + +type ListAuthorsColumnSortDirectionParams struct { + ID int64 + OrderBy interface{} +} + +func (q *Queries) ListAuthorsColumnSortDirection(ctx context.Context, arg ListAuthorsColumnSortDirectionParams) ([]Author, error) { + rows, err := q.db.QueryContext(ctx, listAuthorsColumnSortDirection, arg.ID, arg.OrderBy) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Author + for rows.Next() { + var i Author + if err := rows.Scan(&i.ID, &i.Name, &i.Bio); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listAuthorsColumnSortFnWtihArg = `-- name: ListAuthorsColumnSortFnWtihArg :many SELECT id, name, bio FROM authors ORDER BY id % ? diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/query.sql b/internal/endtoend/testdata/order_by_binds/sqlite/query.sql index 72b1f21f4d..b554036f96 100644 --- a/internal/endtoend/testdata/order_by_binds/sqlite/query.sql +++ b/internal/endtoend/testdata/order_by_binds/sqlite/query.sql @@ -3,6 +3,17 @@ SELECT * FROM authors WHERE id > sqlc.arg(min_id) ORDER BY CASE WHEN sqlc.arg(sort_column) = 'name' THEN name END; +-- name: ListAuthorsColumnSortDirection :many +SELECT * FROM authors +WHERE id > ? +ORDER BY + CASE + WHEN @order_by = 'asc' THEN name + END ASC, + CASE + WHEN @order_by = 'desc' OR @order_by IS NULL THEN name + END DESC; + -- name: ListAuthorsColumnSortFnWtihArg :many SELECT * FROM authors ORDER BY id % ?; diff --git a/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json b/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json index cd66df063b..36d9a495cc 100644 --- a/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json +++ b/internal/endtoend/testdata/order_by_binds/sqlite/sqlc.json @@ -4,7 +4,7 @@ { "path": "go", "engine": "sqlite", - "name": "querytest", + "name": "order_by_binds", "schema": "schema.sql", "queries": "query.sql" }