You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by moshebla <gi...@git.apache.org> on 2019/01/01 15:07:57 UTC
[GitHub] lucene-solr pull request #531: SOLR-12768
GitHub user moshebla opened a pull request:
https://github.com/apache/lucene-solr/pull/531
SOLR-12768
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/moshebla/lucene-solr SOLR-12768
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/lucene-solr/pull/531.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #531
----
commit 6b147506ab7e8c7eda52dde9748fdebef8133194
Author: Moshe <mo...@...>
Date: 2019-01-01T10:29:15Z
SOLR-12678: applied David's patch
commit 83560ecf38f52f31d48b133d0c1a925fa7df8c28
Author: Moshe <mo...@...>
Date: 2019-01-01T12:37:47Z
SOLR-12768: remove last / from _NEST_PATH_FIELD filter in ChildDocTransformerFactory#processPathHierarchyQueryString
commit 75a1ad79eb5255a4b2e6807042140bbe3d09bb58
Author: Moshe <mo...@...>
Date: 2019-01-01T15:06:38Z
SOLR-12768: fix partial and full path support in ChildDocTransformer
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/531
> I wonder... hmmm... maybe the nest path should always start with a '/'? It would seem more correct since paths in general usually start with one, and it would also make implementing this case simpler.
Uploaded a new commit with the above requested change.
I also fixed TestNestedUpdateProcessor and TestChildDocTransformerHierarchy.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/531
I'll add another test-case then.
> I wonder... hmmm... maybe the nest path should always start with a '/'? It would seem more correct since paths in general usually start with one, and it would also make implementing this case simpler.
Logically, this seems like the more correct way to pursue.
Although, I wonder whether this is the correct time to start this over,
with Solr 8 just around the corner.
WDYT?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Alright?
I'm confused since I thought PathHierarchyTokenizerFactory is the agreed upon method of querying paths.
Perhaps it might not cut it for this feature though?
In any case,
Should the JIRA ticket used to further discuss this matter openly be another subtask of SOLR-12298, or a separate ticket altogether?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Perhaps we could test whether "foo" is a defined field in the current collection?
I'll try and test this on Sunday as I have got to go.
I personally prefer keeping the API as simple as possible(I'm a believer of the KISS principle).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Nonetheless we could stop trying to solve this ambiguous case further _for now_ (thus commit what you have here) since (a) this syntax is very experimental (b) it's documented nowhere (c) and wasn't developed very openly. RE openness; given (a) & (b), the non-openness of (c) is okay but if we _really_ want to make this feature known, it deserves it's own issue to discuss with the syntax ought to be publicly. A syntax to match paths is big enough that it shouldn't be buried within the scope of some other issue.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Thanks for helping out!
It's insufficient to simply prepend a `*` since it would match other labels that share the same suffix. For example: `childFilter='ingredients/name\_s:cocoa'` would get transformed into a query that might match a label `secretingredients` but we don't want it to. I think a solution is to build an OR query where one clause has a `*/` prepended to it, and the other has nothing modified and so matches the term exactly. WDYT?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
> Perhaps we could test whether "foo" is a defined field in the current collection?
Eh; that sounds too much like a "guess what the user might mean" kind of solution. Even if "foo" is in the schema, it doesn't prevent an element "foo" from being used.
> I personally prefer keeping the API as simple as possible(I'm a believer of the KISS principle).
At the expense of ambiguity? Trade-offs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
The way this field is indexed has been experimental and will remain so for a bit. 8.0 will be it's more public debut, after which changing it will be more effort than today. Nonetheless I will make this change clear in CHANGES.txt in case someone was using it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by moshebla <gi...@git.apache.org>.
Github user moshebla commented on the issue:
https://github.com/apache/lucene-solr/pull/531
> I think a solution is to build an OR query where one clause has a */ prepended to it, and the other has nothing modified and so matches the term exactly. WDYT?
Yes that seems correct, and all added unit tests pass.
I just pushed a new commit implementing this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Code changes seem good; I'll look closer and run tests.
PathHierarchyTokenizerFactory _might_ eventually be used at query time if we had a syntax to say "get me all my ancestors". But given it's conditional based on syntax, it can't easily go into the query analyzer, so I think it's better to leave the query time chain simple/direct and we do this stuff when looking at the syntax. I know this has evolved recently from where we were. In this issue I removed PathHierarchyTokenizer from the index side because I feel it's better to err on lighter-weight indexing at the expense of slower queries since I think it's very likely a very low cost for what I think are typical use-cases.
RE issue to discuss the syntax: yes a subtask of SOLR-12298. no hurry on that one
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
[GitHub] lucene-solr issue #531: SOLR-12768
Posted by dsmiley <gi...@git.apache.org>.
Github user dsmiley commented on the issue:
https://github.com/apache/lucene-solr/pull/531
Looks good. I thought you were going to test with something to show we don't screw it up? You could search for 'redients' which would have returned something before but now should not.
I wonder... hmmm... maybe the nest path should always start with a '/'? It would seem more correct since paths in general usually start with one, and it would also make implementing this case simpler.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org