[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-583518344 I have applied the changes to solve the feedback points and consequentially added additional tests to cover some missing scenario. We should be almost ready to go :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-583474019 hi @romseygeek , @dsmiley , first of all, thank you again for your patience and very useful insights. I have incorporated Alan's changes and cleaned everything up. My un-resolved questions: - boostAttribute doesn’t use BytesRef but directly float, is it a concern? We are expected to use it at query time, so we could actually see a query time minimal benefit in not encoding/decoding? - Alan expressed concerns over SpanBoostQuery, mentioning they are sort of broken, what should we do in that regard? right now the create span query seems to work as expected with boosted synonyms(see the related test), I suspect if SpanBoostQuery are broken , they should get resolved in another ticket? - from an original comment in the test code org.apache.solr.search.TestSolrQueryParser#testSynonymQueryStyle: "confirm autoGeneratePhraseQueries always builds OR queries" I changed that, was there any reason for that behaviour? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-580681314 I would love to leverage the momentum and the fresh memory on this project to finalise the contribution, I didn't get any feedback on the TokenStream / AttributeSource controversy in the last few days... If i don't get any additional feedback on that by mid next week, I'll take [~dsmiley] consideration and change the implementation toward the use of the TokenStream directly. Super happy to receive additional feedback and considerations by [~romseygeek], maybe the AttributeSource approach was misunderstood? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-579326000 No strong opinion on that, it was actually the first time I used the AttributeSource so I am happy to switch to TokenStream if it is more memory efficient. The change shouldn't be too heavy. I will just wait for confirmation, and when we are all aligned I'll proceed with the implementation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-579200844 I followed the refactor comments from both @diegoceccarelli and @romseygeek . The PR seems much cleaner right now both Lucene and Solr side. Copious tests are present and should cover the various situations. Few questions remain: - from a test I read a comment from @dsmiley saying: "confirm autoGeneratePhraseQueries always builds OR queries" from org.apache.solr.search.TestSolrQueryParser#testSynonymQueryStyle - what can we do for the SpanBoostQuery, I was completely not aware they are going to be deprecated Let me know This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene-solr] alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload
alessandrobenedetti commented on issue #357: [SOLR-12238] Synonym Queries boost by payload URL: https://github.com/apache/lucene-solr/pull/357#issuecomment-578403615 I have been doing merge conflicts, a huge cleaning up and re-design. Most of the logic is now in Lucene Query Parser, this avoids passing back and forth token streams. It seems to me a much cleaner approach, happy to discuss TO DO: This is what I'll do next week - Add additional tests on Lucene Query Builder (that is green at the moment) - discuss about Paylod decoding and possible use of org.apache.lucene.analysis.payloads.PayloadHelper - discuss why "/*confirm autoGeneratePhraseQueries always builds OR queries*/ from org.apache.solr.search.TestSolrQueryParser#testSynonymQueryStyle This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
