Index map annotations (Fix #12318) #3383

joshmoore · 2015-01-23T12:41:36Z

In order for the name-value pairs attach to objects
to be searchable by Lucene, a handler for the map
annotations needed to be added.

To discuss/do:

Which other map columns to index?
Whether or not "map.key" is useful?
Add documentation to the developers/Modules/Search.html page (Map index documentation omero-documentation#1113)

To test:

Add a map annotation to an object and search for either the key or the value.
Additionally, searching for "key:value" should also return the object.

--no-rebase

In order for the name-value pairs attach to objects to be searchable by Lucene, a handler for the map annotations needed to be added.

mtbc · 2015-01-27T08:58:15Z

components/server/src/ome/services/fulltext/FullTextBridge.java

+        if (nvs != null && nvs.size() > 0) {
+            for (NamedValue nv : nvs) {
+                if (nv != null) {
+logger().error("{}={}", nv.getName(), nv.getValue());


"in progress" things to be removed before final merge, I guess

joshmoore · 2015-01-27T09:00:57Z

Thanks, @mtbc. Pushed.

mtbc · 2015-01-27T09:03:41Z

components/server/src/ome/services/fulltext/FullTextBridge.java

+            for (NamedValue nv : nvs) {
+                if (nv != null) {
+logger().error("{}={}", nv.getName(), nv.getValue());
+                    add(document, nv.getName(), nv.getValue(), opts);


I wonder if we want a standard prefix before map keys. (Even also using the map name, if it has a name?) So, they don't collide with other search field names.

Happy to entertain concrete suggestions. My thinking was that even a collision would still return proper search results. A prefix will require users to know to add it. Shrugs Both perhaps?

dominikl · 2015-01-28T09:57:12Z

Search works like expected. I just wonder what the "map.key" is about.

joshmoore · 2015-01-28T10:52:42Z

Using [ NamedValue("foo", "bar") ] as an example, without map,key

foo:bar would return a result and
bar (i.e. the default in _combined) would return a result

but foo alone would not. With map.key, the following all return:

foo:bar
bar
foo
map.key:foo

Is this useful?

dominikl · 2015-01-28T11:29:06Z

Ah ok, yes, I think it makes sense; one would expect to find something by just searching for a name/key only.

joshmoore · 2015-01-28T13:27:06Z

Would "has_key:foo" be better/clearer?

joshmoore · 2015-02-11T07:54:42Z

@dominikl @pwalczysko @jburel @will-moore : any thoughts on this?

pwalczysko · 2015-02-11T10:04:38Z

@joshmoore : Sorry, not sure about 2 things

does the user type this in the search box (e.g. has_key:foo ) ?
not sure what the difference between has_key:foo and map.key:foo really is (seems a bit the same ?)
searching for foo:bar (or rather in more realistic terms temperature:37 is the main case to cater for in my opinion -> we shouild not make the user to have to know the whole syntax with the colon, the search for temperature 37 or similar should give him something as well

will-moore · 2015-02-11T10:04:44Z

If I search for foo=bar would I get key:value of foo: blah in my search results? If so, is it guaranteed to come AFTER foo: bar in the results?
What's the difference between searching for foo:bar and foo=bar?

will-moore · 2015-02-11T10:06:05Z

Do we need any changes on the client side for this PR? Any changes in how we parse queries?

joshmoore · 2015-02-11T10:12:57Z

Do we need any changes on the client side for this PR? Any changes in how we parse queries?

I don't think so. The only thing you would need to decide is if you ever want to work with has_key.

If I search for foo=bar would I get key:value of foo: blah in my search results? If so, is it guaranteed to come AFTER foo: bar in the results?

Do you mean foo=bar or foo:bar? I haven't done anything to support the former. If you're saying the client will parse out the "=" and therefore this will actually search for "foo bar" then yes, you could also get results for "foo blah" and there's no guarantee as to the ordering, though in general, I'd assume more matches come earlier.

What's the difference between searching for foo:bar and foo=bar?

The former is proper Lucene syntax. The second (if it works) is an odd by-product of our string parsing.

does the user type this in the search box (e.g. has_key:foo ) ?

I'm not sure if that's allowed by the clients, but certainly on the CLI you could type this to find all images that have a certain key. (At the moment, map.key:foo though). Alternatively, just foo would also return those images.

not sure what the difference between has_key:foo and map.key:foo really is (seems a bit the same ?)

Yes, it's just the question of which name is most understandable.

searching for foo:bar (or rather in more realistic terms temperature:37 is the main case to cater for in my opinion -> we shouild not make the user to have to know the whole syntax with the colon, the search for temperature 37 or similar should give him something as well

I believe this to be the case, yes.

pwalczysko · 2015-02-11T10:39:23Z

@joshmoore : Thanks. has_key:foo sounds more understandable.

joshmoore · 2015-02-11T10:48:08Z

Pushed.

dominikl · 2015-02-13T10:26:57Z

If I understood this right, the name "map.key" or "has_key" doesn't matter on the client side (just an issue on the backend), so there's nothing specific to test for that in the clients, right?
If so, this PR is good to merge. Checked the search functionality again; search results are like I would expect. And I can confirm, if I search for "foo:bar" (or "foo=bar", doesn't matter because it's reduced to "foo bar" in both cases), it will find results where key=foo and value=bar, as well as results where just key or value=foo respectively where just key or value=bar; but results which contain both terms come first.

joshmoore · 2015-02-13T10:28:45Z

Just to be clear: so there's currently no way to make use of "X:" from the clients at the momonet? i.e. it's parsed away?

dominikl · 2015-02-13T10:46:54Z

Yes, at the moment all non-alphanummeric characters (except _ are replaced with a space). Ok that actually means I didn't understand this right, "has_key:xxx" is an expression the user should be able to type in order to search for a specific key only?

mtbc · 2015-02-16T08:53:40Z

https://ci.openmicroscopy.org/view/Failing/job/OMERO-5.1-merge-integration-python/lastCompletedBuild/testReport/test.integration.test_search/TestSearch/test_map_annotations/

The expectation is that if "has_key:foo" is passed that this would only find fields with exactly that key. Though the LuceneQueryBuilder removes the ":", the queries still seem to work as expected.

joshmoore · 2015-02-18T08:49:40Z

For me all the tests are now green. I pushed a few more to looking into #3383 (comment) (namely joshmoore@9b22776) which all seem to be fine. (Minimal) documentation has been opened. Assuming we don't want to start indexing more in this PR, I'd say it's good to go once the next run of the integration tests are green unless there are any questions/objections on the client-side.

joshmoore · 2015-02-18T08:50:28Z

NB: re: "Which other map columns to index?" -- the only other 2 objects which would be useful to index onto the image would be the GenericLightSource and ImagingEnvironment. At the moment, though, we haven't done anything with those objects, so likely that would be more effort than we want to expend.

joshmoore · 2015-02-18T12:16:54Z

Comment fixed. As I said, my assumption is that all these tests are green. If it's better to have this in m4, I'm happy to handle any test adjustments in subsequent PRs.

Index map annotations (Fix #12318)

joshmoore added 2 commits January 23, 2015 13:38

Index map annotations (Fix #12318)

09fda64

In order for the name-value pairs attach to objects to be searchable by Lucene, a handler for the map annotations needed to be added.

Passing test_map_annotations after minor cleanup

87ce619

jburel added the develop label Jan 23, 2015

mtbc reviewed Jan 27, 2015
View reviewed changes

Cleanup FullTextBridge

83dd361

joshmoore force-pushed the map-indexing branch from 2740a65 to 83dd361 Compare January 27, 2015 09:15

FullTextBridge: change map.key to has_key

f83f5ee

joshmoore added 2 commits February 17, 2015 17:17

Fix use of map.key in itest

18b76a4

Add passing has_key tests

9b22776

The expectation is that if "has_key:foo" is passed that this would only find fields with exactly that key. Though the LuceneQueryBuilder removes the ":", the queries still seem to work as expected.

Fix comment typo

7d02b16

sbesson added a commit that referenced this pull request Feb 18, 2015

Merge pull request #3383 from joshmoore/map-indexing

e501d4e

Index map annotations (Fix #12318)

sbesson merged commit e501d4e into ome:develop Feb 18, 2015

sbesson added this to the 5.1.0-m4 milestone Feb 20, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index map annotations (Fix #12318) #3383

Index map annotations (Fix #12318) #3383

joshmoore commented Jan 23, 2015

mtbc Jan 27, 2015

joshmoore commented Jan 27, 2015

mtbc Jan 27, 2015

joshmoore Jan 27, 2015

dominikl commented Jan 28, 2015

joshmoore commented Jan 28, 2015

dominikl commented Jan 28, 2015

joshmoore commented Jan 28, 2015

joshmoore commented Feb 11, 2015

pwalczysko commented Feb 11, 2015

will-moore commented Feb 11, 2015

will-moore commented Feb 11, 2015

joshmoore commented Feb 11, 2015

pwalczysko commented Feb 11, 2015

joshmoore commented Feb 11, 2015

dominikl commented Feb 13, 2015

joshmoore commented Feb 13, 2015

dominikl commented Feb 13, 2015

mtbc commented Feb 16, 2015

joshmoore commented Feb 18, 2015

joshmoore commented Feb 18, 2015

joshmoore commented Feb 18, 2015

Index map annotations (Fix #12318) #3383

Index map annotations (Fix #12318) #3383

Conversation

joshmoore commented Jan 23, 2015

mtbc Jan 27, 2015

Choose a reason for hiding this comment

joshmoore commented Jan 27, 2015

mtbc Jan 27, 2015

Choose a reason for hiding this comment

joshmoore Jan 27, 2015

Choose a reason for hiding this comment

dominikl commented Jan 28, 2015

joshmoore commented Jan 28, 2015

dominikl commented Jan 28, 2015

joshmoore commented Jan 28, 2015

joshmoore commented Feb 11, 2015

pwalczysko commented Feb 11, 2015

will-moore commented Feb 11, 2015

will-moore commented Feb 11, 2015

joshmoore commented Feb 11, 2015

pwalczysko commented Feb 11, 2015

joshmoore commented Feb 11, 2015

dominikl commented Feb 13, 2015

joshmoore commented Feb 13, 2015

dominikl commented Feb 13, 2015

mtbc commented Feb 16, 2015

joshmoore commented Feb 18, 2015

joshmoore commented Feb 18, 2015

joshmoore commented Feb 18, 2015