CrossRef / cayenne

MOVED to https://gitlab.com/crossref/rest_api
https://gitlab.com/crossref/rest_api
MIT License
17 stars 9 forks source link

Some querying tweaks with good performance improvement #25

Closed MikeYalter closed 6 years ago

MikeYalter commented 6 years ago

Doing a query vs solr with q=(*:*) AND (field:"test") is a lot slower than q=(field:"test"). Something close to 50% speed improvement on ad hoc tests for field queries.

markwoodhall commented 6 years ago

Hi Mike,

I've been doing a little bit of testing against this branch using some of the automated tests created recently. It looks like I'm hitting a problem when trying to load a DOI using something like http://localhost:3000/works/10.13039/501100003593, this works fine on master but using this branch I get a NullPointerException with the following stack trace:

"stack": [
      "clojure.lang.Numbers.ops(Numbers.java:1013)",
      "clojure.lang.Numbers.gt(Numbers.java:229)",
      "clojure.lang.Numbers.gt(Numbers.java:3864)",
      "cayenne.api.v1.query$__GT_solr_query.invokeStatic(query.clj:331)",
      "cayenne.api.v1.query$__GT_solr_query.doInvoke(query.clj:264)",
      "clojure.lang.RestFn.invoke(RestFn.java:439)",
      "cayenne.data.work$fetch_one.invokeStatic(work.clj:174)",
      "cayenne.data.work$fetch_one.invoke(work.clj:170)",
      "cayenne.api.v1.routes$work_resource$fn__23538$fn__23539.invoke(routes.clj:222)",
      "cayenne.api.v1.routes$__GT_1$fn__23471.invoke(routes.clj:66)",
      "liberator.core$decide.invokeStatic(core.clj:98)",
      "liberator.core$decide.invoke(core.clj:91)",
      "liberator.core$exists_QMARK_.invokeStatic(core.clj:406)",
      "liberator.core$exists_QMARK_.invoke(core.clj:406)",
      "liberator.core$decide.invokeStatic(core.clj:103)",
      "liberator.core$decide.invoke(core.clj:91)",
      "liberator.core$processable_QMARK_.invokeStatic(core.clj:409)",
      "liberator.core$processable_QMARK_.invoke(core.clj:409)",
      "liberator.core$decide.invokeStatic(core.clj:103)",
      "liberator.core$decide.invoke(core.clj:91)",
      "liberator.core$encoding_available_QMARK_.invokeStatic(core.clj:413)",
      "liberator.core$encoding_available_QMARK_.invoke(core.clj:413)"

A quick glance seems to indicate (.getRows query) might be returning nil.

MikeYalter commented 6 years ago

Hi Mark, yeah, I actually had a check in place with the (and) but I must have hit undo too many times in the ide at some point.

markwoodhall commented 6 years ago

Looks like it works fine now Mike, thanks. Do you happen to know if there is any implication to running (.getRows query) twice, like we do now?

If there is, perhaps we could use something like (> (or (.getRows query) 0) 0)? If not then I will merge this.

MikeYalter commented 6 years ago

@markwoodhall there shouldn't be as it's generated from a map that we created, rather than a return from the query. But I like your way of doing the check.