xorm/xorm
xorm/xorm
63
356
Fork 206
Code Issues 370 Pull Requests 18 Actions 10 Releases 35 Activity

Make OrderBy(order interface{}, args ...interface{}) #2149

Closed
zeripath wants to merge 6 commits from order-by-interface into master
pull from: order-by-interface
merge into: xorm:master
Conversation 5 Commits 6 Files Changed 8 +81 -36
zeripath commented 2022-05-29 09:44:04 +00:00
Contributor
Copy Link

Instead of forcing OrderBy to be a string this PR allows
OrderBy to pass in either a builder.Cond or a string-args pair.

Signed-off-by: Andrew Thornton art27@cantab.net

Instead of forcing OrderBy to be a string this PR allows OrderBy to pass in either a builder.Cond or a string-args pair. Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added 1 commit 2022-05-29 09:44:05 +00:00
Make OrderBy(order interface{}, args ...interface{}) 
Instead of forcing OrderBy to be a string this PR allows
OrderBy to pass in either a builder.Cond or a string-args pair.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Some checks failed
continuous-integration/drone/pr Build is failing
Details
bccf0686ef
zeripath commented 2022-05-29 09:45:47 +00:00
Author
Contributor
Copy Link

This PR does change the interface of Engine so we might want to create a different function name for this addtional functionality.

Edit: I've changed this to create a new func OrderByWithArgs instead... see below.

This PR does change the interface of Engine so we might want to create a different function name for this addtional functionality. --- Edit: I've changed this to create a new func OrderByWithArgs instead... see below.
lunny reviewed 2022-05-29 11:45:07 +00:00
internal/statements/statement.go
@ -459,0 +468,4 @@
}
case string:
rawOrder = order.(string)
statement.RawParams = args
lunny commented 2022-05-29 11:45:07 +00:00
Owner
Copy Link

Is this necessary?

Is this necessary?
zeripath commented 2022-05-29 12:21:25 +00:00
Author
Contributor
Copy Link

Nope it's a mistake.

Nope it's a mistake.
zeripath marked this conversation as resolved
zeripath added 1 commit 2022-05-29 12:21:46 +00:00
oops 
Signed-off-by: Andrew Thornton <art27@cantab.net>
All checks were successful
continuous-integration/drone/pr Build is passing
Details
ccabd3adb3
zeripath added 1 commit 2022-05-29 12:29:49 +00:00
Instead of changing the signature of OrderBy just add OrderByWithArgs 
Signed-off-by: Andrew Thornton <art27@cantab.net>
Some checks failed
continuous-integration/drone/pr Build is failing
Details
9c3cc96cb2
zeripath commented 2022-05-29 12:30:11 +00:00
Author
Contributor
Copy Link

Hmm... thinking on it would be reasonable instead to not change the signature of OrderBy but instead to create a new function OrderByWithArgs so I've pushed that.

Hmm... thinking on it would be reasonable instead to not change the signature of `OrderBy` but instead to create a new function `OrderByWithArgs` so I've pushed that.
zeripath added 1 commit 2022-05-30 07:48:02 +00:00
Merge branch 'master' into order-by-interface
Some checks failed
continuous-integration/drone/pr Build is failing
Details
5045ec0d3e
zeripath commented 2022-05-30 09:19:54 +00:00
Author
Contributor
Copy Link

There's a strange issue in TestTableIndices affecting MySQL8 which I think is doing some internal mapping with the duplicated indices in that test f_two_f_one and f_one_f_two. This PR cannot be responsible for the test failure but I've pushed up a fix that avoids that problem.

There's a strange issue in `TestTableIndices` affecting MySQL8 which I think is doing some internal mapping with the duplicated indices in that test f_two_f_one and f_one_f_two. This PR cannot be responsible for the test failure but I've pushed up a fix that avoids that problem.
zeripath force-pushed order-by-interface from 574ca18480 to 129d533df4 2022-05-30 09:44:21 +00:00 Compare
zeripath commented 2022-05-30 09:55:04 +00:00
Author
Contributor
Copy Link

Cool I've fixed that test.

Cool I've fixed that test.
lunny commented 2022-05-30 10:01:17 +00:00
Owner
Copy Link

I think this is a temporary fix and #2150 will resolve the problem from the ground.

I think this is a temporary fix and #2150 will resolve the problem from the ground.
zeripath added 1 commit 2022-05-30 11:14:09 +00:00
Merge branch 'master' into order-by-interface
All checks were successful
continuous-integration/drone/pr Build is passing
Details
5315a26382
lunny closed this pull request 2022-06-01 06:49:15 +00:00
All checks were successful
continuous-integration/drone/pr Build is passing
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backport/done

   
backport/v1

   
blocked
  
db
oracle

  
db
sqlserver

  
duplicate
  
feature
cache

  
frontport/done

   
frontport/main

   
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 Label
backport/done
backport/v1
blocked
db
oracle
db
sqlserver
duplicate
feature
cache
frontport/done
frontport/main
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
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: xorm/xorm#2149
No description provided.
Delete Branch "order-by-interface"

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?