Refactor orderby and support arguments #2150
No reviewers
Labels
No Label
backport/done
backport/v1
blocked
db
oracle
db
sqlserver
duplicate
feature
cache
invalid
kind
breaking
kind
bug
kind
build
kind
dependencies
kind
docs
kind
driver
kind
enhancement
kind
feature
kind
performance
kind
proposal
kind
question
kind
refactor
kind
testing
need
feedback
need
test
proposal:accepted
RaspBerry Pi
regression
skip-changelog
upstream
wip
wontfix
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: xorm/xorm#2150
Loading…
Reference in New Issue
No description provided.
Delete Branch "lunny/orderby"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
To support OrderBy with arguments, this PR have a total refactor about how to generate SQL of Select, Delete and Update.
8c10203fb5
to213e4a7775
d3e98adee8
to5fa0a473c7
The change to the signature of OrderBy is an API break... If someone has an interface that expects
OrderBy(string) *xorm.Session
that won't work anymore.This would be rare - for example Gitea doesn't have such an interface - but it is still an API break.
If you're happy with the breaking change relating the function signature change - it may be advisable to further change the signature to
OrderBy(order interface{}, args ...interface{})
at the same time:Patch
I don't think we should support
builder.Builder
butbuilder.Express
depends on xorm/builder#86SQLite's documentation has a good BNF for
ORDER BY
:https://www.sqlite.org/lang_select.html#x2027
An ORDER BY is followed by a comma separated list of ordering-terms
which is:
Now a
*builder.cond
is esssentially anexpr
here so it would be valid.But we could make an explicit
*builder.OrderingTerm
.My comment was that we should support general conditions - as they represent an SQL expression, see https://www.sqlite.org/syntax/expr.html
builder.Expression
provides string way of doing this but actuallybuilder.Cond
is the general way of doing this.I don't think
Order By
supports allbuilder.Cond
. Here we usebuilder.Expression
make the range minimal when possible.I've linked the SQLite Documentation above. It does.
But what about other databases?