search

koopjs / winnow

Deprecated
Apache License 2.0
90 stars 18 forks source link

Bugfix/84 #85

Closed efbenson closed 6 years ago

efbenson commented 6 years ago

changes how the query code works to keep existing manipulation, but allowing for limit/offset/order to be done by alasql

efbenson commented 6 years ago

Ill take a look at those build issues in the morning

rgwozdz commented 6 years ago

@efbenson tests are failing because the new code doesn't conform to standard.js

rgwozdz commented 6 years ago

When your first issue was logged I started looking into a post-query iterative sort. Sorry, I didn't realize you had already started an effort. My WIP PR is here: https://github.com/koopjs/winnow/pull/86

efbenson commented 6 years ago

Problem with post ordering outside the alasql is that you will have to disregard limit and process the whole feature set to make sure you order correctly. This is something brought up by it client as they use koop in AGOL and need to sort layers that are paginated.

rgwozdz commented 6 years ago

At least right now, the limit isn't used in the SQL, because as you mentioned the SQL is being applied on a per feature basis. So I think one way forward would be to apply the sort (if options.orderByFields is defined) prior to these loops https://github.com/koopjs/winnow/blob/master/src/executeQuery.js#L36-L41, https://github.com/koopjs/winnow/blob/master/src/executeQuery.js#L55-L59. If the feature set is big, that could be an expensive sort, but I'd big curious if it's any more expensive than what alasql is doing under the hood. @dmfenton any additional thoughts?

rgwozdz commented 6 years ago

After more thought on this, we might the approach of capturing the essentials of the esriFy function and pass it in as part of the pick function in the SQL statement.

rgwozdz commented 6 years ago

Moved the esriFy to the SQL and then was able to let SQL handle ORDER, LIMIT, OFFSET in https://github.com/koopjs/winnow/pull/90. Released in v1.6.1