Refactor orderby and support arguments #2150
Merged
lunny
merged 14 commits from lunny/orderby
into master
1 month ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'lunny/orderby'
Deleting a branch is permanent. It CANNOT be undone. Continue?
To support OrderBy with arguments, this PR have a total refactor about how to generate SQL of Select, Delete and Update.
8c10203fb5
to213e4a7775
1 month agod3e98adee8
to5fa0a473c7
1 month agoThe 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
.f9a6990ecb
into master 1 month agoMy 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?
f9a6990ecb
.