Skip to content

Commit

Permalink
Make the limit/offset API cleaner
Browse files Browse the repository at this point in the history
The 'Limit()' function of SelectStmt is misleading as it also sets the OFFSET.
In some case we want thiner control on limit and offset, or not set both
at the same time.

The new functions consists of:
- Limit(value) -> set LIMIT
- Offset(value) -> set OFFSET
- LimitOffset(limit, offset) -> set both LIMIT and OFFSET

This change *breaks the API*
  • Loading branch information
cdevienne committed Feb 22, 2017
1 parent 80fdf52 commit ca840ee
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 11 deletions.
11 changes: 9 additions & 2 deletions compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,15 @@ func (c SQLCompiler) VisitSelect(context *CompilerContext, selectStmt SelectStmt
addLine(sql)
}

if (selectStmt.OffsetValue != nil) && (selectStmt.LimitValue != nil) {
addLine(fmt.Sprintf("LIMIT %d OFFSET %d", *selectStmt.LimitValue, *selectStmt.OffsetValue))
if (selectStmt.OffsetValue != nil) || (selectStmt.LimitValue != nil) {
var tokens []string
if selectStmt.LimitValue != nil {
tokens = append(tokens, fmt.Sprintf("LIMIT %d", *selectStmt.LimitValue))
}
if selectStmt.OffsetValue != nil {
tokens = append(tokens, fmt.Sprintf("OFFSET %d", *selectStmt.OffsetValue))
}
addLine(strings.Join(tokens, " "))
}

if selectStmt.ForUpdateClause != nil {
Expand Down
18 changes: 15 additions & 3 deletions select.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,25 @@ func (s SelectStmt) Having(aggregate AggregateClause, op string, value interface
return s
}

// Limit sets the offset & count values of the select statement
func (s SelectStmt) Limit(offset int, limit int) SelectStmt {
s.OffsetValue = &offset
// Limit sets the limit number of rows
func (s SelectStmt) Limit(limit int) SelectStmt {
s.LimitValue = &limit
return s
}

// Offset sets the offset
func (s SelectStmt) Offset(value int) SelectStmt {
s.OffsetValue = &value
return s
}

// LimitOffset sets the limit & offset values of the select statement
func (s SelectStmt) LimitOffset(limit, offset int) SelectStmt {
s.LimitValue = &limit
s.OffsetValue = &offset
return s
}

// ForUpdate adds a "FOR UPDATE" clause
func (s SelectStmt) ForUpdate(tables ...TableElem) SelectStmt {
s.ForUpdateClause = &ForUpdateClause{tables}
Expand Down
14 changes: 8 additions & 6 deletions select_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,30 @@ func (suite *SelectTestSuite) TestOrderByLimit() {
From(suite.sessions).
Where(Eq(suite.sessions.C("user_id"), 5)).
OrderBy(suite.sessions.C("id")).Desc().
Limit(0, 20)
Limit(20)

sql, binds := asDefSQLBinds(selOrderByDesc)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id DESC\nLIMIT 20 OFFSET 0", sql)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id DESC\nLIMIT 20", sql)
assert.Equal(suite.T(), []interface{}{5}, binds)

selWithoutOrder := Select(suite.sessions.C("id")).
From(suite.sessions).
Where(Eq(suite.sessions.C("user_id"), 5)).
OrderBy(suite.sessions.C("id"))
OrderBy(suite.sessions.C("id")).
Offset(12)

sql, binds = asDefSQLBinds(selWithoutOrder)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id ASC", sql)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id ASC\nOFFSET 12", sql)
assert.Equal(suite.T(), []interface{}{5}, binds)

selOrderByAsc := Select(suite.sessions.C("id")).
From(suite.sessions).
Where(Eq(suite.sessions.C("user_id"), 5)).
OrderBy(suite.sessions.C("id")).Asc()
OrderBy(suite.sessions.C("id")).Asc().
OffsetLimit(12, 20)

sql, binds = asDefSQLBinds(selOrderByAsc)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id ASC", sql)
assert.Equal(suite.T(), "SELECT id\nFROM sessions\nWHERE user_id = ?\nORDER BY id ASC\nLIMIT 20 OFFSET 12", sql)
assert.Equal(suite.T(), []interface{}{5}, binds)
}

Expand Down

0 comments on commit ca840ee

Please sign in to comment.