LIMIT is no column but a SQL keyword, allow limit on initial sync#10842
Conversation
nickvergessen
left a comment
There was a problem hiding this comment.
Should really move to the query builder at some time
|
I will take out the additional limit code and put it into a dedicated pull-request because it needs more testing ... |
|
The first part (removing the `s) is working here fine :) I'd not use the other part (allow limit on initial sync) this way because it returns the wrong sync-token ( Support for client-requested limiting (truncating) is not required by the RFC, so maybe just throwing 507 on initial syncs with |
|
set the milestone to 15 as this PR is against master. |
|
Moved it to the query builder (just so we keep cleaning up bit by bit). If all is green we just need to rebase. |
|
CI is not happy. Seems that @georgehrke added the order by which is not present. |
|
@georgehrke ^ ? |
|
Moved to 15.0.1 |
@georgehrke What is the status? We are close to the beta 1. 17 or 18? |
Let me put it onto my ToDo list for tomorrow. |
Tomorrow was yesterday, right? 😉 |
ae1bfb9 to
5966286
Compare
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
5966286 to
9f6dd51
Compare
|
Test fresh sync with limit: => Throws an exception Test fresh sync without limit: => Returns a list of all events Test with sync-token and limit: => Behaviour didn't change compared to master Test with sync-token and without limit: => Behaviour didn't change compared to master |
|
I changed this PR as per @rfc2822's suggestion. |
fixes #9339
please test @rfc2822