[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16518497#comment-16518497 ] David Smiley commented on SOLR-11865: - To clarify, the "PR" (GitHub) is what I cannot "close"... albeit I can indirectly if I put the magic words into a commit message but it's easy to forget that and I forgot. I can change the workflow state of Jira (which is not referred to as a PR) and I did. Issues go to "Resolved" state upon a completion (not closed). Closure happens when the issue is released to the next version, performed upon a release in bulk (without issue/Jira notification) by the release manager. They shouldn't be closed beforehand, which you did but I simply put it back to Resolved state just now – no big deal – I don't think there's any consequence. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Assignee: David Smiley >Priority: Minor > Labels: QueryComponent > Fix For: 7.5 > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16517247#comment-16517247 ] Bruno Roustant commented on SOLR-11865: --- Thanks for your incredible help [~dsmiley]! Closing this PR. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Assignee: David Smiley >Priority: Minor > Labels: QueryComponent > Fix For: 7.5 > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507042#comment-16507042 ] ASF subversion and git services commented on SOLR-11865: Commit 62d1fc3eef6c4c4bf278e592deb0521602f1f248 in lucene-solr's branch refs/heads/branch_7x from broustant [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=62d1fc3 ] SOLR-11865: QueryElevationComponent changes * new useConfiguredElevatedOrder setting * more extensible (customizable via subclass) ** ElevationProvider ** handleInitializationException with cause enum * use BytesRef for uniqueKey ID pervasively instead of String. * ElevatorComparatorSource now reuses getBoostedDocs logic * setSort will short-circuit if there are no elevated Ids * extensive refactoring and affects some interrelated components Closes #390 (cherry picked from commit a06256c) > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16507037#comment-16507037 ] ASF subversion and git services commented on SOLR-11865: Commit a06256ccee8742eaf47ac1f2c12ec8e68a70a163 in lucene-solr's branch refs/heads/master from broustant [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=a06256c ] SOLR-11865: QueryElevationComponent changes * new useConfiguredElevatedOrder setting * more extensible (customizable via subclass) ** ElevationProvider ** handleInitializationException with cause enum * use BytesRef for uniqueKey ID pervasively instead of String. * ElevatorComparatorSource now reuses getBoostedDocs logic * setSort will short-circuit if there are no elevated Ids * extensive refactoring and affects some interrelated components > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16503459#comment-16503459 ] Markus Jelsma commented on SOLR-11865: -- I am not sure this will work for me right away, but maybe i can get things to work via new API's. So i am fine with this for now. Thanks! > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16503428#comment-16503428 ] David Smiley commented on SOLR-11865: - Updated patch with more visibility. I reverted some of analyzeQuery logic to behave as before. Interestingly this added space was reflected in a test but I had not caught it before. I removed the static from this method (to be like before) and made MapElevationProvider non-static so it could access the outer analyzeQuery instance method (which can be overridden, unlike a static method). > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16503360#comment-16503360 ] David Smiley commented on SOLR-11865: - Okay Markus... though I hesitate to make *every* member protected/public so I'll upload something and you can tell me if it meets your purposes. [~bruno.roustant] while looking at analyzeQuery/getAnalyzedQuery I noticed a change in behavior. First I was wondering what we changed, and then I saw what seems to me a pointless method extraction splitQueryTermsWithAnalyzer so I inlined it (it's not like analyzeQuery was so long as to warrant that). Then I thought I liked the older code here better -- no need to build up the query terms into an ArrayList; the StringBuilder was totally fine. Then I looked closer at the "QUERY_EXACT_JOINER" and saw it joins by a space. Well the current code does *not* do this. I think this will result in a change of behavior. The current QueryElevationComponent behavior will look at two queries, "foo bar" and "foobar" and come up with the same result for both. If there are two configured elevated queries this way, I think this will result in an error; which I don't care about to retain either way. More importantly, at search time, a query for "foo bar" could match a configured elevated query "foobar". Your change here will *not* do this. Right? So I think we can keep the current code/behavior and make protected so you can subclass as you need to. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16503164#comment-16503164 ] Markus Jelsma commented on SOLR-11865: -- [~dsmiley] can you change those private members to protected in QueryElevationComponent? All those members such as initArgs and queryAnalyzer but also the methods should be extendible. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16502002#comment-16502002 ] David Smiley commented on SOLR-11865: - Here's our final patch. My CHANGES.txt will be as follows: New Features: * The QueryElevationComponent now has a useConfiguredElevatedOrder setting. When multiple docs are elevated, this specifies wether their relative order should be the order in the configuration file or if not then should they be subject to whatever the sort criteria is. Other: * Extensive refactoring of internals of the QueryElevationComponent to allow for better customization. Affects tie-ins with CollapseQParser & marker doc transformers that honor QEC's adjustments. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch, SOLR-11865.patch > > Time Spent: 0.5h > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16496788#comment-16496788 ] Bruno Roustant commented on SOLR-11865: --- You're right MapElevationProvider.buildElevationMap should merge in this case (which indeed should not happen since they have been merged earlier). I have created the GitHub PR ([https://github.com/apache/lucene-solr/pull/390),] to be enhanced with all your improvements. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > Time Spent: 20m > Remaining Estimate: 0h > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16479764#comment-16479764 ] David Smiley commented on SOLR-11865: - BTW random comment; it seems inconsistent that MapElevationProvider.buildElevationMap will throw an exception of the elevation is duplicated, yet earlier we merge elevations. Is there a rhyme/reason to the disparity? It could be merged here again, as is done earlier. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16479751#comment-16479751 ] David Smiley commented on SOLR-11865: - Bruno, can you please use a GitHub PR (referencing this issue in the title so that it auto-links) to push your commits/patches? It's way easier to do back & forth code review using GitHub. I have a new patch but I'd rather apply it to a feature-branch/PR where you (and anyone else) can see the deltas more easily. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16476032#comment-16476032 ] Bruno Roustant commented on SOLR-11865: --- Great! I agree with all your points [~dsmiley]. Indeed the String IDs in Elevation would be clearer as BytesRefs. And I vote to apply the key String => indexed form as early as possible, if the code remains small. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16464442#comment-16464442
]
David Smiley commented on SOLR-11865:
-
Patch still in progress but want to mention some things.
* New name {{useConfiguredOrderForElevations}}. Documentation language: "When
multiple docs are elevated, should their relative order be the order in the
configuration file or should they be subject to whatever the sort criteria is?
True by default."
* Found a way to entirely skip using ElevationComparatorSource if the
Elevation obj has no elevations (maybe just has exclusions)
* I was looking in detail at ElevationComparatorSource that led me to some
observations that I'd like your input on:
** BytesRef[] termValues is the "value half of the map". It's the BytesRef
version of the ID values aligned with ordSet (doc ID) slots. But the reader of
these (docVal()) has to do additional work to look it up in
elevation.priorities to get an int. I think this could be replaced with an
int[] populated with the pertinent int priorities when doSetNextReader is
called (which is where ordSet & termValues is init'ed right now). This int[]
would be named simply priorities.
** Elevation.elevatedIds could be a Map that maps directly to
the priority from the uniqueKey val (thus removing the need for a separate
"priorities" map), and then in doSetNextReader we can iterate on the Map.Entry
and needn't do another lookup.
** I wonder if the String IDs in Elevation, both elevated and excluded, ought
to be BytesRefs to clarify that they are raw indexed form IDs? (consider when
uniqueKey is a long) The current String form is suggestive that they are the
surface form IDs, yet they aren't since they've already been mapped with
FieldType.readableToIndexed. Or alternatively keep the surface form IDs and
translate them at a later time. I think we might as well do them eagerly as it
saves work during search, even if it's easy work, and again it clarifies the
type.
*** FieldType.readableToIndexed(String) ought to be deprecated in lieu of
readableToIndexed(CharSequence, BytesRefBuilder).
** I guess it's debatable where to actually apply the key String => indexed
form (String of BytesRef)... we're doing it in Elevation's constructor with a
passed in UnaryOperator thingy but it could just as easily be done very late
in, say, ElevationComparatorSource.doSetNextReader, or perhaps very early right
after we read it from the XML. I suppose it's fine as-is.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch,
> 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16456587#comment-16456587
]
David Smiley commented on SOLR-11865:
-
_I deleted my previous comment as I did it from an old/wrong account
accidentally._
I said:
{quote}Maybe we should make it a query param, which is more convenient &
useful. The spots that load the config that examine this boolean could track
the order (e.g. using LinkedHashSet) even if ultimately the order might not be
needed? It seems like not a big deal to track the order potentially needlessly
but you might have a better sense of this.
{quote}
You said:
{quote}Actually the TrieSubsetMatcher introduced by the next patch does not
support keepElevationPriority.
{quote}
Well I think it's okay to have another parameter that is ignored if another
parameter is set. This would still be the case even if it wasn't a runtime
parameter! (it's just a matter of where it's specified)
In general, Solr has been trying to use more runtime parameters where possible
& appropriate to avoid the requirement to modify configs. It's good for
experimentation, and makes tests easier to write (no new config file to put in
the tests).
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch,
> 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16456470#comment-16456470 ] Bruno Roustant commented on SOLR-11865: --- Actually the TrieSubsetMatcher introduced by the next patch does not support keepElevationPriority. If keepElevationPriority=true, this matcher is replaced by another, which keeps the order but which is less efficient. And this is done at component initialization time, in the inform() method (in loadElevationProvider()). So I think it cannot be a query param because it is fixed in the data structure at initialization time. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16452932#comment-16452932 ] David Smiley commented on SOLR-11865: - Maybe we should make it a query param, which is more convenient & useful. The spots that load the config that examine this boolean could track the order (e.g. using LinkedHashSet) even if ultimately the order might not be needed? It seems like not a big deal to track the order potentially needlessly but you might have a better sense of this. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16452868#comment-16452868
]
David Smiley commented on SOLR-11865:
-
Ok if keepElevationPriority works that way, I think there could be a clearer
name. Perhaps {{allowElevatedDocsToBeResorted}} or {{sortElevatedDocs}} ?
(defaults to false; current behavior),
or {{useConfiguredElevatedOrder}} (default to true)?
I'm going to add a test for this behavior. It seems like a useful feature,
especially with forceElevation.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch,
> 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16450065#comment-16450065 ] Bruno Roustant commented on SOLR-11865: --- Sorry for the delay. Yes, if you can take it from here, that would be awesome! * Getters for defaults: you're right, there is no need. Please remove them. * keepElevationPriority as a constant in QEC: good point. * keepElevationPriority meaning: Actually the comment is not right, maybe the sorting has changed since the time I wrote this comment. I don't think it is linked anymore to forceElevation since the ElevationComparatorSource can be added as a SortField even if forceElevation=false when one sort by score. The point is - with keepElevationPriority=true, the behavior is unchanged, the elevated documents (on top) are sorted by the order of the elevation rules and elevated ids in the config file. - with keepElevationPriority=false, the behavior changes, the elevated documents (still on top) are in any order, and they may be re-ordered by other sort fields (this will allow the use of the efficient but unsorted TrieSubsetMatcher in the other patch). > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16427637#comment-16427637
]
David Smiley commented on SOLR-11865:
-
This is looking very good Bruno. You even hid InitializationException. I
think I can take it from here but would like some input. I can throw up
another patch. I've already tweaked some formatting locally (e.g. bad or wrong
indentation).
* Why make it possible for a subclass to change the _default_ values of
settings? I'm looking at the getters you added referring to some constants and
it seems needless. These are just defaults after all; can't you simply be
explicit if the default doesn't suit you?
* RE keepElevationPriority, I saw you added it to QueryElevationParams but
realized you're not actually using it as a _parameter_, you're using it as a
config file setting name (which we don't call parameters in Solr as it's
ambiguous). Therefore it goes to a constant in QEC.
** I want to ensure I understand this setting better. I did read the docs you
put on the constant definition. So it requires forceElevation. If this is
configured to false, will the sort order of the elevated documents be not only
at the top but then sorted by the sort parameter coming into Solr? And if true
it's in config-order? Maybe this could be named forceElevationWithConfigOrder?
This way it's name suggests a more clear relationship with forceElevation.
BTW I'm going to add a bit of docs to the ref guide (file
{{the-query-elevation-component.adoc}}) here.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch,
> 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16426847#comment-16426847
]
Lucene/Solr QA commented on SOLR-11865:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:red}-1{color} | {color:red} patch {color} | {color:red} 0m 4s{color}
| {color:red} SOLR-11865 does not apply to master. Rebase required? Wrong
Branch? See
https://wiki.apache.org/solr/HowToContribute#Creating_the_patch_file for help.
{color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | SOLR-11865 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12917652/SOLR-11865.patch |
| Console output |
https://builds.apache.org/job/PreCommit-SOLR-Build/37/console |
| Powered by | Apache Yetus 0.7.0 http://yetus.apache.org |
This message was automatically generated.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch,
> 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16426589#comment-16426589 ] Bruno Roustant commented on SOLR-11865: --- New delta patch with the modification mentioned. Eventually I'll squash the commits to produce a single patch that should be supported by "Yetus" (currently I simply use git format-patch and it produces three separate patch files for three commits). > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, > 0003-Remove-exception-handlers-and-refactor-getBoostDocs.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16421062#comment-16421062
]
Lucene/Solr QA commented on SOLR-11865:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:red}-1{color} | {color:red} patch {color} | {color:red} 0m 11s{color}
| {color:red} SOLR-11865 does not apply to master. Rebase required? Wrong
Branch? See
https://wiki.apache.org/solr/HowToContribute#Creating_the_patch_file for help.
{color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | SOLR-11865 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12917033/SOLR-11865.patch |
| Console output |
https://builds.apache.org/job/PreCommit-SOLR-Build/30/console |
| Powered by | Apache Yetus 0.7.0 http://yetus.apache.org |
This message was automatically generated.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch, SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420920#comment-16420920 ] David Smiley commented on SOLR-11865: - Thanks by the way for entertaining some further improvements to the code that aren't essential. I'm aware some of my feedback is to code you didn't write. It's spring-cleaning time :) * splitQueryTermsWithAnalyzer: Could be improved to use try-with-resources style since TokenStream is Closeable * RE == false vs "!", "And that's a warning highlighted by IntelliJ by the way": I know; IntelliJ has a ton of things it warns about in its default configuration. It goes a bit far IMO; a default config should be very conservative. I've tweaked my warning settings a lot (some on, some off). * RE exceptions: I get that you want to expose more insight into what went wrong. I'm just wondering if the stock QueryElevationComponent could be a bit simpler complexity-wise and still accommodate your needs via another mechanism. I like trying to simplify where possible. What if we do away with this LoadingExceptionHandler as a separate class and instead have protected methods directly on QueryElevationComponent? And that's it. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420858#comment-16420858
]
David Smiley commented on SOLR-11865:
-
Thanks for the update; it's looking nicer. The delta patch was helpful
(thanks!) but "Yetus" didn't know what to make of it. The SOLR-.patch file
should be a standard diff or git formap-patch. The one I see right now is
definitely not that; it's only 2 lines.
(getBoostDocs):
bq. 6- Use localBoosts.addAll(boosted.keySet()); at line ~661 instead of manual
looping.
I believe the "localBoosts" var is a copy intentionally because the algorithm
that follows will remove items to reduce the lookups in 1/2. See
{{iter.remove()}}. In your recent change, you're manipulating the incoming
argument from the parameter (not good).
I propose the following rewrite of this method. Whoever first wrote it didn't
know about {{SolrIndexSearcher.lookupId(...)}}.
{code:java}
public static IntIntHashMap getBoostDocs(SolrIndexSearcher indexSearcher,
Mapboosted, Map context) throws IOException {
IntIntHashMap boostDocs = null;
if(boosted != null) {
//First see if it's already in the request context. Could have been put
there
//by another caller.
if(context != null) {
boostDocs = (IntIntHashMap) context.get(BOOSTED_DOCIDS);
}
if(boostDocs != null) {
return boostDocs;
}
//Not in the context yet so load it.
boostDocs = new IntIntHashMap(boosted.size()); // docId to boost
for (Map.Entry keyAndBoostPair : boosted.entrySet()) {
final BytesRef uniqueKey = keyAndBoostPair.getKey();
long segAndId = indexSearcher.lookupId(uniqueKey); // higher 32 bits ==
segment ID, low 32 bits == doc ID
if (segAndId == -1) { // not found
continue;
}
int seg = (int) (segAndId >> 32);
int localDocId = (int) segAndId;
final IndexReaderContext indexReaderContext =
indexSearcher.getTopReaderContext().children().get(seg);
int docId = indexReaderContext.docBaseInParent + localDocId;
boostDocs.put(docId, keyAndBoostPair.getValue());
}
}
if(context != null) {
//noinspection unchecked
context.put(BOOSTED_DOCIDS, boostDocs);
}
return boostDocs;
}
{code}
I'll respond to the other parts in another comment.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch, SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420620#comment-16420620 ] Bruno Roustant commented on SOLR-11865: --- [~dsmiley] I uploaded a new patch. Is it better now? > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > 0002-Refactor-QueryElevationComponent-after-review.patch, SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420604#comment-16420604
]
Lucene/Solr QA commented on SOLR-11865:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:red}-1{color} | {color:red} patch {color} | {color:red} 0m 4s{color}
| {color:red} SOLR-11865 does not apply to master. Rebase required? Wrong
Branch? See
https://wiki.apache.org/solr/HowToContribute#Creating_the_patch_file for help.
{color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Issue | SOLR-11865 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12917017/0002-Refactor-QueryElevationComponent-after-review.patch
|
| Console output |
https://builds.apache.org/job/PreCommit-SOLR-Build/29/console |
| Powered by | Apache Yetus 0.7.0 http://yetus.apache.org |
This message was automatically generated.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> 0002-Refactor-QueryElevationComponent-after-review.patch, SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420329#comment-16420329 ] Bruno Roustant commented on SOLR-11865: --- 11- subsetMatch flag in ElevatingQuery. Yes, the idea is to support some queries with subset match, and other without. This will be supported by the next ElevationProvider in the next patch. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420322#comment-16420322 ] Bruno Roustant commented on SOLR-11865: --- 10- seen.contains(id) == false. I didn't know this Lucene practice. It explains why I see this strange construct. "I recommend against modifying existing lines" - that's what I tried (see points 3,5,6 above) and I thought this "!seen.contains(id)" was tiny and harmless. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420319#comment-16420319 ] Bruno Roustant commented on SOLR-11865: --- 9- Make the constructor of ElevatingQuery protected. Done. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420317#comment-16420317 ] Bruno Roustant commented on SOLR-11865: --- 8- Use a UnaryOperator instead of IndexedValueProvider. Good point. It is still clear with less code. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420313#comment-16420313 ] Bruno Roustant commented on SOLR-11865: --- 7- In parseExcludedMarkerFieldName and parseEditorialMarkerFieldName. Removed the if () block. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420311#comment-16420311
]
Bruno Roustant commented on SOLR-11865:
---
6- Use {{localBoosts.addAll(boosted.keySet());}} at line ~661 instead of manual
looping.
Again, I didn't change that (and I didn't want to touch existing code without
reason).
I fixed by directly removing localBoosts which was an exact copy of
boosts.keySet() (boots parameter is a map).
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420308#comment-16420308 ] Bruno Roustant commented on SOLR-11865: --- 5- Change comparator docVal (~line 1318) to use getOrDefault. I didn't change that. Fixed. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420301#comment-16420301 ] Bruno Roustant commented on SOLR-11865: --- 3- The indentation around line ~671 (contents of the for loop) is messed up. I didn't change that part. I'll try to fix the indentation. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420302#comment-16420302 ] Bruno Roustant commented on SOLR-11865: --- 4- No "Can be overridden by extending this class". Sure. Removed. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420300#comment-16420300 ] Bruno Roustant commented on SOLR-11865: --- 2- ElevationProvider should be immutable and simplified: Good point. createElevationProvider() accepts the elevationBuilderMap. getElevationForQuery() does not throw IOException. ElevationProvider.size is used by tests to verify the number of parsed rules. I added @VisibleForTesting annotation. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420272#comment-16420272 ] Bruno Roustant commented on SOLR-11865: --- 1- InitializationExceptionHandler & LoadingExceptionHandler: At Salesforce (i.e. in a multi-tenant context) we allow each organization admin to update the list of elevation rules dynamically. When some rules are updated, the core corresponding to the organization is updated to reload the elevation rules XML. It is important to note that the organization admin - the person who defines the elevation rules - is not a Solr admin expert. He needs to get clear feedback on any error that may prevent the rules to be loaded. The XML rules are more considered as dynamic config rather than static config. In its original version, the QueryElevationComponent simply throws an exception. In this new version, it differentiates the error cause and lets an extending class (e.g. specific Salesforce extension) override the loading exception and take appropriate actions (logging, warning, etc) instead of simply throwing the Solr exception. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[ https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395493#comment-16395493 ] David Smiley commented on SOLR-11865: - BTW one thing that I'm not sure about is if it might make sense to move the subsetMatch flag from the ElevatingQuery to Elevation; and then we don't need an ElevatingQuery (just use the query String as before). But I'm not sure. The current design allows to elevate a query with and without subsetMatch with separate rules... I think? If that's pertinent then nevermind. > Refactor QueryElevationComponent to prepare query subset matching > - > > Key: SOLR-11865 > URL: https://issues.apache.org/jira/browse/SOLR-11865 > Project: Solr > Issue Type: Improvement > Security Level: Public(Default Security Level. Issues are Public) > Components: SearchComponents - other >Affects Versions: master (8.0) >Reporter: Bruno Roustant >Priority: Minor > Labels: QueryComponent > Fix For: master (8.0) > > Attachments: > 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch, > SOLR-11865.patch > > > The goal is to prepare a second improvement to support query terms subset > matching or query elevation rules. > Before that, we need to refactor the QueryElevationComponent. We make it > extendible. We introduce the ElevationProvider interface which will be > implemented later in a second patch to support subset matching. The current > full-query match policy becomes a default simple MapElevationProvider. > - Add overridable methods to handle exceptions during the component > initialization. > - Add overridable methods to provide the default values for config properties. > - No functional change beyond refactoring. > - Adapt unit test. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-11865) Refactor QueryElevationComponent to prepare query subset matching
[
https://issues.apache.org/jira/browse/SOLR-11865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395483#comment-16395483
]
David Smiley commented on SOLR-11865:
-
Thanks Bruno.
It seems there is _some_ new/changed behavior (and that's fine):
* can define the same query more than once and it'll get merged; no longer an
exception
* new {{keepElevationPriority}} parameter
Comments:
* I'm a little skeptical about the need for InitializationExceptionHandler,
LoadingExceptionHandler, and related complexities. Maybe you can provide some
insight as to why this is needed?
* IMO it's unfortunate that ElevationProvider is mutable and has the
makeImmutable method. How about createElevationProvider accept the
elevationBuilderMap? And does ElevationProvider.size need to be there either?
And does getElevationForQuery need to throw an IOException? With such
simplifications, we could then simply use JDK Function. Not
that using the JDK one is a big deal (and it's debatable too) but my point is
more about simplifying.
* the indentation around line ~671 (contents of the for loop) is messed up; it
made me misinterpret the intent of the the logic
* My preference is to not have javadoc comments like "Can be overridden by
extending this class." because "protected" access implies this, thus it's
redundant.
* suggest change comparator docVal (~line 1318) to use getOrDefault
* suggest to use {{localBoosts.addAll(boosted.keySet());}} at line ~661 instead
of manual looping (IntelliJ helpfully pointed this out)
* in parseExcludedMarkerFieldName and parseEditorialMarkerFieldName I see
initArgs.get being called with a default value, yet it's followed up with a
check for null to then use the default value. This seems quite redundant since
the two-arg SolrParams.get already does that. I don't like the empty string
check (this is for a config file, not a request parameter where a better
argument could be made for it) -- I'd much prefer something tighter/simpler.
It appears SOLR-2037 introduced this but it was a minor detail that wasn't
discussed in the comments. Removing this would technically be a back-compat
break but it's minor enough and so easy for someone to fix that I think it's
fine.
* Instead of IndexedValueProvider, which we don't even expose, lets just use a
UnaryOperator? and then use a Java 8 method reference when needed
instead of declaring QueryElevationComponent.indexedValueProvider ?
* Maybe make the constructor of ElevatingQuery protected so it could be
subclassed. Likewise for Elevation.
BTW this code pattern {{seen.contains(id) == false}} is often written as-such
deliberately instead of {{!seen.contains(id)}} because it reads clearer (and
takes no additional lines of code). Bugs have been missed then found because
of that style. There is no code standard for it in Lucene or Solr but I
recommend against modifying existing lines that made one choice or the other as
part this cleanup.
> Refactor QueryElevationComponent to prepare query subset matching
> -
>
> Key: SOLR-11865
> URL: https://issues.apache.org/jira/browse/SOLR-11865
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SearchComponents - other
>Affects Versions: master (8.0)
>Reporter: Bruno Roustant
>Priority: Minor
> Labels: QueryComponent
> Fix For: master (8.0)
>
> Attachments:
> 0001-Refactor-QueryElevationComponent-to-introduce-Elevat.patch,
> SOLR-11865.patch
>
>
> The goal is to prepare a second improvement to support query terms subset
> matching or query elevation rules.
> Before that, we need to refactor the QueryElevationComponent. We make it
> extendible. We introduce the ElevationProvider interface which will be
> implemented later in a second patch to support subset matching. The current
> full-query match policy becomes a default simple MapElevationProvider.
> - Add overridable methods to handle exceptions during the component
> initialization.
> - Add overridable methods to provide the default values for config properties.
> - No functional change beyond refactoring.
> - Adapt unit test.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
