Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9
    • Component/s: core/queryparser
    • Labels:
      None
    • Environment:

      N/A

    • Lucene Fields:
      New

      Description

      From "New flexible query parser" thread by Micheal Busch

      in my team at IBM we have used a different query parser than Lucene's in
      our products for quite a while. Recently we spent a significant amount
      of time in refactoring the code and designing a very generic
      architecture, so that this query parser can be easily used for different
      products with varying query syntaxes.

      This work was originally driven by Andreas Neumann (who, however, left
      our team); most of the code was written by Luis Alves, who has been a
      bit active in Lucene in the past, and Adriano Campos, who joined our
      team at IBM half a year ago. Adriano is Apache committer and PMC member
      on the Tuscany project and getting familiar with Lucene now too.

      We think this code is much more flexible and extensible than the current
      Lucene query parser, and would therefore like to contribute it to
      Lucene. I'd like to give a very brief architecture overview here,
      Adriano and Luis can then answer more detailed questions as they're much
      more familiar with the code than I am.
      The goal was it to separate syntax and semantics of a query. E.g. 'a AND
      b', '+a +b', 'AND(a,b)' could be different syntaxes for the same query.
      We distinguish the semantics of the different query components, e.g.
      whether and how to tokenize/lemmatize/normalize the different terms or
      which Query objects to create for the terms. We wanted to be able to
      write a parser with a new syntax, while reusing the underlying
      semantics, as quickly as possible.
      In fact, Adriano is currently working on a 100% Lucene-syntax compatible
      implementation to make it easy for people who are using Lucene's query
      parser to switch.

      The query parser has three layers and its core is what we call the
      QueryNodeTree. It is a tree that initially represents the syntax of the
      original query, e.g. for 'a AND b':
      AND
      / \
      A B

      The three layers are:
      1. QueryParser
      2. QueryNodeProcessor
      3. QueryBuilder

      1. The upper layer is the parsing layer which simply transforms the
      query text string into a QueryNodeTree. Currently our implementations of
      this layer use javacc.
      2. The query node processors do most of the work. It is in fact a
      configurable chain of processors. Each processors can walk the tree and
      modify nodes or even the tree's structure. That makes it possible to
      e.g. do query optimization before the query is executed or to tokenize
      terms.
      3. The third layer is also a configurable chain of builders, which
      transform the QueryNodeTree into Lucene Query objects.

      Furthermore the query parser uses flexible configuration objects, which
      are based on AttributeSource/Attribute. It also uses message classes that
      allow to attach resource bundles. This makes it possible to translate
      messages, which is an important feature of a query parser.

      This design allows us to develop different query syntaxes very quickly.
      Adriano wrote the Lucene-compatible syntax in a matter of hours, and the
      underlying processors and builders in a few days. We now have a 100%
      compatible Lucene query parser, which means the syntax is identical and
      all query parser test cases pass on the new one too using a wrapper.

      Recent posts show that there is demand for query syntax improvements,
      e.g improved range query syntax or operator precedence. There are
      already different QP implementations in Lucene+contrib, however I think
      we did not keep them all up to date and in sync. This is not too
      surprising, because usually when fixes and changes are made to the main
      query parser, people don't make the corresponding changes in the contrib
      parsers. (I'm guilty here too)
      With this new architecture it will be much easier to maintain different
      query syntaxes, as the actual code for the first layer is not very much.
      All syntaxes would benefit from patches and improvements we make to the
      underlying layers, which will make supporting different syntaxes much
      more manageable.

      1. lucene_1567_adriano_crestani_07_13_2009.patch
        882 kB
        Adriano Crestani
      2. lucene_trunk_FlexQueryParser_2009July09_v4.patch
        738 kB
        Luis Alves
      3. lucene_trunk_FlexQueryParser_2009July10_v5.patch
        738 kB
        Luis Alves
      4. lucene_trunk_FlexQueryParser_2009july15_v6.patch
        848 kB
        Luis Alves
      5. lucene_trunk_FlexQueryParser_2009july16_v7.patch
        870 kB
        Adriano Crestani
      6. lucene_trunk_FlexQueryParser_2009july23_v8.patch
        837 kB
        Adriano Crestani
      7. lucene_trunk_FlexQueryParser_2009july27_v9.patch
        862 kB
        Luis Alves
      8. lucene_trunk_FlexQueryParser_2009july28_v10.patch
        900 kB
        Adriano Crestani
      9. lucene_trunk_FlexQueryParser_2009july30_v12.patch
        907 kB
        Adriano Crestani
      10. lucene_trunk_FlexQueryParser_2009july31_v14.patch
        809 kB
        Luis Alves
      11. lucene_trunk_FlexQueryParser_2009March24.patch
        894 kB
        Luis Alves
      12. lucene_trunk_FlexQueryParser_2009March26_v3.patch
        660 kB
        Luis Alves
      13. lucene-1567.patch
        855 kB
        Michael Busch
      14. new_query_parser_src.tar
        610 kB
        Michael Busch
      15. QueryParser_restructure_meetup_june2009_v2.pdf
        76 kB
        Luis Alves
      16. wiki_switching_to_the_new_query_parser.txt
        2 kB
        Adriano Crestani

        Issue Links

          Activity

          Hide
          Luis Alves added a comment -

          Should the Flexible Query Parser patch be committed to the main,
          as a replacement for the old queryparser?

          The current implementation is using Java 1.5 syntax.
          Is that OK, if we commit it to the trunk.

          Show
          Luis Alves added a comment - Should the Flexible Query Parser patch be committed to the main, as a replacement for the old queryparser? The current implementation is using Java 1.5 syntax. Is that OK, if we commit it to the trunk.
          Hide
          Adriano Crestani added a comment -

          It's probably not ok, since lucene build script will probably fail because of that. We are working on a patch which we will upload to this JIRA soon, it will only be for the community to review the new query parser code and not to be committed against the trunk. I think somebody could create a sandbox and commit the code, it would be easier for other to review the new query parser.

          I think the right question is if we should include this new parser in the release 2.9, if yes, then we definitely need to change the code to be java 1.4 compatible. Anyway, before taking this decision, the code must be available for the community : )

          Best Regards,

          Show
          Adriano Crestani added a comment - It's probably not ok, since lucene build script will probably fail because of that. We are working on a patch which we will upload to this JIRA soon, it will only be for the community to review the new query parser code and not to be committed against the trunk. I think somebody could create a sandbox and commit the code, it would be easier for other to review the new query parser. I think the right question is if we should include this new parser in the release 2.9, if yes, then we definitely need to change the code to be java 1.4 compatible. Anyway, before taking this decision, the code must be available for the community : ) Best Regards,
          Hide
          Paul Elschot added a comment -

          You may want to take a look here:
          http://wiki.apache.org/lucene-java/Java_1.5_Migration
          Iirc somewhere in the threads referenced there it was mentioned that contrib modules can go ahead to 1.5 already now.

          Show
          Paul Elschot added a comment - You may want to take a look here: http://wiki.apache.org/lucene-java/Java_1.5_Migration Iirc somewhere in the threads referenced there it was mentioned that contrib modules can go ahead to 1.5 already now.
          Hide
          Michael Busch added a comment -

          I think it would be good to attach the 1.5 implementation here for now as a patch so that we can review the code.
          We can then decide when/how/where to commit it.

          Show
          Michael Busch added a comment - I think it would be good to attach the 1.5 implementation here for now as a patch so that we can review the code. We can then decide when/how/where to commit it.
          Hide
          Grant Ingersoll added a comment - - edited

          First step towards Software Grant is to have all people who worked on the code file a CLA. I would suggest IBM file a CCLA for this, especially b/c it sounds like one of the people who worked on it is no longer there. I will ask for clarification if it is really needed. In the meantime, if Luis can file his CLA that would be great: http://www.apache.org/licenses/icla.txt

          Show
          Grant Ingersoll added a comment - - edited First step towards Software Grant is to have all people who worked on the code file a CLA. I would suggest IBM file a CCLA for this, especially b/c it sounds like one of the people who worked on it is no longer there. I will ask for clarification if it is really needed. In the meantime, if Luis can file his CLA that would be great: http://www.apache.org/licenses/icla.txt
          Hide
          Grant Ingersoll added a comment -

          Never mind on the CCLA, as I think the software grant covers it: http://www.apache.org/licenses/software-grant.txt

          Show
          Grant Ingersoll added a comment - Never mind on the CCLA, as I think the software grant covers it: http://www.apache.org/licenses/software-grant.txt
          Hide
          Grant Ingersoll added a comment -

          OK, I have started the IP Clearance in incubation. Please send in the software grant ASAP and make sure you CC me on it (gsingers@a.o)

          Show
          Grant Ingersoll added a comment - OK, I have started the IP Clearance in incubation. Please send in the software grant ASAP and make sure you CC me on it (gsingers@a.o)
          Hide
          Luis Alves added a comment -

          This is first initial patch, for people to review and play with
          it was done against the trunk as of 2009 March 24 into the main,

          We tried to document the code as much as possible, Adriano and me will try answer any
          questions as soon as we can.

          Samples using the new API's
          org.apache.lucene.queryParser.spans.TestSpanQueryParserSimpleSample
          org.apache.lucene.queryParser.spans.TestSpanQueryParser

          Old QueryParser testcases running against
          org.apache.lucene.queryParser.lucene2.QueryParserWrapper
          org.apache.lucene.queryParser.lucene2.MultiFieldQueryParserWrapper

          the Compatible Wrappers, using the new Flexible Query Parser as backend

          org.apache.lucene.queryParser.lucene2.TestMultiAnalyzer
          org.apache.lucene.queryParser.lucene2.TestMultiFieldQueryParser
          org.apache.lucene.queryParser.lucene2.TestQueryParser

          The "build" and "test-core" targets are working fine.

          The build.xml 'test" target is failing in the xml-query-parser, I still need to verify why.
          I'm also working on "javadocs" warnings in the new classes

          Show
          Luis Alves added a comment - This is first initial patch, for people to review and play with it was done against the trunk as of 2009 March 24 into the main, We tried to document the code as much as possible, Adriano and me will try answer any questions as soon as we can. Samples using the new API's org.apache.lucene.queryParser.spans.TestSpanQueryParserSimpleSample org.apache.lucene.queryParser.spans.TestSpanQueryParser Old QueryParser testcases running against org.apache.lucene.queryParser.lucene2.QueryParserWrapper org.apache.lucene.queryParser.lucene2.MultiFieldQueryParserWrapper the Compatible Wrappers, using the new Flexible Query Parser as backend org.apache.lucene.queryParser.lucene2.TestMultiAnalyzer org.apache.lucene.queryParser.lucene2.TestMultiFieldQueryParser org.apache.lucene.queryParser.lucene2.TestQueryParser The "build" and "test-core" targets are working fine. The build.xml 'test" target is failing in the xml-query-parser, I still need to verify why. I'm also working on "javadocs" warnings in the new classes
          Hide
          Luis Alves added a comment -

          I forgot to mention that we made some changes to common-build.xml to patch it to jdk 1.5 level for patch to work, and to include the queryparser property file needed by the new queryparser into the lucene jar.

          Show
          Luis Alves added a comment - I forgot to mention that we made some changes to common-build.xml to patch it to jdk 1.5 level for patch to work, and to include the queryparser property file needed by the new queryparser into the lucene jar.
          Hide
          Mark Miller added a comment -

          I have not had a chance to look too deeply yet (I've just jumped around the code for 15 or 20 minutes), but first impression is great. A really nice step forward for the queryparser. Thanks guys.

          Show
          Mark Miller added a comment - I have not had a chance to look too deeply yet (I've just jumped around the code for 15 or 20 minutes), but first impression is great. A really nice step forward for the queryparser. Thanks guys.
          Hide
          Mark Miller added a comment -

          From org.apache.lucene.queryParser.lucene2.config.package.html

          This configuration
          handler reproduces almost everything that could be set on the old query parser.

          Does this mean there is something missing? Or that some settings are handled in another manner?

          Show
          Mark Miller added a comment - From org.apache.lucene.queryParser.lucene2.config.package.html This configuration handler reproduces almost everything that could be set on the old query parser. Does this mean there is something missing? Or that some settings are handled in another manner?
          Hide
          Adriano Crestani added a comment -

          From org.apache.lucene.queryParser.lucene2.config.package.html

          This configuration
          handler reproduces almost everything that could be set on the old query parser.

          Does this mean there is something missing? Or that some settings are handled in another manner?

          The only QueryParser configuration that is not implemented on the new query parser configuration is QueryParser.setFuzzyMinSim(float) and QueryParser.setFuzzyPrefixLength(int). They are kind of hardcoded for now, always using the default values for these configs. But it doesn't mean they cannot be implemented on the new query parser...I think the main reason why they are not implemented yet is that Lucene TestQueryParser is not testing them, so I did not give much attention to these 2 settings. I will try to implement them for the next patch ; )

          Show
          Adriano Crestani added a comment - From org.apache.lucene.queryParser.lucene2.config.package.html This configuration handler reproduces almost everything that could be set on the old query parser. Does this mean there is something missing? Or that some settings are handled in another manner? The only QueryParser configuration that is not implemented on the new query parser configuration is QueryParser.setFuzzyMinSim(float) and QueryParser.setFuzzyPrefixLength(int). They are kind of hardcoded for now, always using the default values for these configs. But it doesn't mean they cannot be implemented on the new query parser...I think the main reason why they are not implemented yet is that Lucene TestQueryParser is not testing them, so I did not give much attention to these 2 settings. I will try to implement them for the next patch ; )
          Hide
          Luis Alves added a comment -

          Here is an updated version of the patch with minor fixes. This version does not delete the old lucene queryparser.

          build.xml default, javadocs, test-core all run fine.

          Show
          Luis Alves added a comment - Here is an updated version of the patch with minor fixes. This version does not delete the old lucene queryparser. build.xml default, javadocs, test-core all run fine.
          Hide
          Luis Alves added a comment -

          HI Grant and Micheal
          I faxed the CLA today.

          Show
          Luis Alves added a comment - HI Grant and Micheal I faxed the CLA today.
          Hide
          Luis Alves added a comment -

          I cleaned up all the javadocs on this one.

          Show
          Luis Alves added a comment - I cleaned up all the javadocs on this one.
          Hide
          Mark Miller added a comment -

          Having gone over this a bit, I think its a great step forward over the current parser. I really think it should end up replacing it. It might work well with our possible plan of steps towards modularization. We could deprecate the core QueryParser and add this as part of a new module (the trick will be turning contrib into modules in practice as opposed to just saying they are modules now - this could be a start though)

          Show
          Mark Miller added a comment - Having gone over this a bit, I think its a great step forward over the current parser. I really think it should end up replacing it. It might work well with our possible plan of steps towards modularization. We could deprecate the core QueryParser and add this as part of a new module (the trick will be turning contrib into modules in practice as opposed to just saying they are modules now - this could be a start though)
          Hide
          Grant Ingersoll added a comment -

          OK, I see the CLA is registered. Now we need the Software Grant.

          Show
          Grant Ingersoll added a comment - OK, I see the CLA is registered. Now we need the Software Grant.
          Hide
          Michael Busch added a comment -

          Now we need the Software Grant.

          Working on it. The terms "paper work", "IBM" and "quick" don't usually appear in the same sentence

          Show
          Michael Busch added a comment - Now we need the Software Grant. Working on it. The terms "paper work", "IBM" and "quick" don't usually appear in the same sentence
          Hide
          Bertrand Delacretaz added a comment -

          Grant, the ip-clearance document that you created under incubator-public in svn had not been added to the site-publish folder, I just did that in revision 769253. If that's not correct, please remove both xml and html versions of the lucene-query-parser file there.

          Show
          Bertrand Delacretaz added a comment - Grant, the ip-clearance document that you created under incubator-public in svn had not been added to the site-publish folder, I just did that in revision 769253. If that's not correct, please remove both xml and html versions of the lucene-query-parser file there.
          Hide
          Ali Oral added a comment - - edited

          The flexible query parser looks very promising. I can see that the ProximityQueryNode was implemented but not integrated completely. Can the ProximityQuerNode be used as a replacement for SrndQueryParser? If yes do you think will it be possible to mix proximity query with the other types of queries like the one below:

          field1: ((term1* or term2* or term3*) WITHIN/5 "phrase term1* term2"~2 WITHIN/50 (term3 or term4~3 or term5*))

          Show
          Ali Oral added a comment - - edited The flexible query parser looks very promising. I can see that the ProximityQueryNode was implemented but not integrated completely. Can the ProximityQuerNode be used as a replacement for SrndQueryParser? If yes do you think will it be possible to mix proximity query with the other types of queries like the one below: field1: ((term1* or term2* or term3*) WITHIN/5 "phrase term1* term2"~2 WITHIN/50 (term3 or term4~3 or term5*))
          Hide
          Grant Ingersoll added a comment -

          The software Grant has been received and filed. I will update the paperwork and work to finish this out next week, such that we can then work to commit it.

          Show
          Grant Ingersoll added a comment - The software Grant has been received and filed. I will update the paperwork and work to finish this out next week, such that we can then work to commit it.
          Hide
          Luis Alves added a comment -

          This is a deck of slides we prepared for meetup in san francisco

          Show
          Luis Alves added a comment - This is a deck of slides we prepared for meetup in san francisco
          Hide
          Michael Busch added a comment -

          Since the modularization discussion on java-dev hasn't really had any conclusions yet, we have the options to a) make the patch jre 1.4 compatible, or b) commit it as a contrib module and move to core with 3.0.

          Luis and Adriano: how much work would a) be?

          Show
          Michael Busch added a comment - Since the modularization discussion on java-dev hasn't really had any conclusions yet, we have the options to a) make the patch jre 1.4 compatible, or b) commit it as a contrib module and move to core with 3.0. Luis and Adriano: how much work would a) be?
          Hide
          Adriano Crestani added a comment -

          Hi Michael,

          I think the biggest work will be to refactor the code to not use generics anymore, which is used all over the place. It might take an afternoon. Let me know, if everybody agree on changing the code to be Java 1.4 compatible, so I can do the changes ASAP.

          Show
          Adriano Crestani added a comment - Hi Michael, I think the biggest work will be to refactor the code to not use generics anymore, which is used all over the place. It might take an afternoon. Let me know, if everybody agree on changing the code to be Java 1.4 compatible, so I can do the changes ASAP.
          Hide
          Michael Busch added a comment -

          Is it mostly internal stuff you need to change to compile with 1.4, or do also a lot of public APIs use generics?

          Show
          Michael Busch added a comment - Is it mostly internal stuff you need to change to compile with 1.4, or do also a lot of public APIs use generics?
          Hide
          Adriano Crestani added a comment -

          It's mostly internal stuffs, the only api that uses generics is QueryNode tha returns List<QueryNode> and receives it as param, I actually don't think it's a big deal

          Show
          Adriano Crestani added a comment - It's mostly internal stuffs, the only api that uses generics is QueryNode tha returns List<QueryNode> and receives it as param, I actually don't think it's a big deal
          Hide
          Luis Alves added a comment -

          I actually think we should give the parser to contrib on 2.9 using jdk 1.5 syntax
          and move it to main on 3.0 using jdk1.5 syntax.

          I don't think it's a small change and this change will affect the interfaces and future
          versions of the parser (to be 1.4 compatible).

          I would see nothing wrong with having a jdk 1.4 version if we were 100% compatible with the old queryparser,
          but since that is not the case, I don't think it is worth it. (the wrapper we built does not support the case where users extend the old queryparser class and overwrite methods to add new functionality)

          If everyone else thinks making the queryparser interfaces 1.4 compatible is a must, I will be OK with it.
          But only if we actually move the new queryparser to main on 2.9 and break the compatibility with the old lucene Queryparser class, for users that are extending this class.

          The new queryparser supports 100% on the syntax, and 100% of the lucene Junits. But does not support users that extended the QueryParser class and overwrote some methods.

          Show
          Luis Alves added a comment - I actually think we should give the parser to contrib on 2.9 using jdk 1.5 syntax and move it to main on 3.0 using jdk1.5 syntax. I don't think it's a small change and this change will affect the interfaces and future versions of the parser (to be 1.4 compatible). I would see nothing wrong with having a jdk 1.4 version if we were 100% compatible with the old queryparser, but since that is not the case, I don't think it is worth it. (the wrapper we built does not support the case where users extend the old queryparser class and overwrite methods to add new functionality) If everyone else thinks making the queryparser interfaces 1.4 compatible is a must, I will be OK with it. But only if we actually move the new queryparser to main on 2.9 and break the compatibility with the old lucene Queryparser class, for users that are extending this class. The new queryparser supports 100% on the syntax, and 100% of the lucene Junits. But does not support users that extended the QueryParser class and overwrote some methods.
          Hide
          Adriano Crestani added a comment -

          I went through the new QP and listed what exactly needs to be changed:

          QueryNode class has 2 methods: set(List<QueryNode>), add(List<QueryNode>) and List<QueryNode> getChildren(). All the generics would be removed. I don't see any back compatibility problem if we add generics in future, we could hardcode the type checking if we release with 1.4 and any user impl of this class will need to do the same and follow the documentation.

          ModifierQueryNode has an enum called Modifier with values MOD_NOT, MOD_NONE and MOD_REQ. An enum can be almost completely reproduced on 1.4 using:

          ...
          final public static class Modifier implements Serializable {

          final public static Modifier MOD_NOT = new Modifier();

          final public static Modifier MOD_NOT = new Modifier();

          final public static Modifier MOD_NOT = new Modifier();

          private Modifier()

          { // empty constructor }

          // we might add some Enum methods, like name(), etc...

          }
          ...

          The only back compatibility problem I see when we change the Modifier to enum again is if on the version 1.4 the user checks for Modifier.class.isEnum()...does anybody see any other back-compatibility issue?

          The last thing that will need to be changed is on the QueryBuilder and LuceneQueryBuilder. The QueryBuilder.build() returns an Object and when LuceneQueryBuilder implements it, it specializes the return to Query, which will start throwing Object instead if we change to 1.4. On this case I don't see any back-compatibility issue also.

          Regarding the new QP framework, I don't see any problem about back compatibility, because Lucene will only be Java 1.5 on version 3.0, and back compatibility may be broken. But...

          I would see nothing wrong with having a jdk 1.4 version if we were 100% compatible with the old queryparser,
          but since that is not the case, I don't think it is worth it. (the wrapper we built does not support the case where users extend the old queryparser class and overwrite methods to add new functionality)

          I agree with Luis, if we only release the new QP framework 2.9, we will definitely brake the back-compatiblity of the old QP, so, why not release the old and the new QP together on 2.9?

          Suggestions?

          Best Regards,
          Adriano Crestani Campos
          Adriano Crestani Campos

          Show
          Adriano Crestani added a comment - I went through the new QP and listed what exactly needs to be changed: QueryNode class has 2 methods: set(List<QueryNode>), add(List<QueryNode>) and List<QueryNode> getChildren(). All the generics would be removed. I don't see any back compatibility problem if we add generics in future, we could hardcode the type checking if we release with 1.4 and any user impl of this class will need to do the same and follow the documentation. ModifierQueryNode has an enum called Modifier with values MOD_NOT, MOD_NONE and MOD_REQ. An enum can be almost completely reproduced on 1.4 using: ... final public static class Modifier implements Serializable { final public static Modifier MOD_NOT = new Modifier(); final public static Modifier MOD_NOT = new Modifier(); final public static Modifier MOD_NOT = new Modifier(); private Modifier() { // empty constructor } // we might add some Enum methods, like name(), etc... } ... The only back compatibility problem I see when we change the Modifier to enum again is if on the version 1.4 the user checks for Modifier.class.isEnum()...does anybody see any other back-compatibility issue? The last thing that will need to be changed is on the QueryBuilder and LuceneQueryBuilder. The QueryBuilder.build() returns an Object and when LuceneQueryBuilder implements it, it specializes the return to Query, which will start throwing Object instead if we change to 1.4. On this case I don't see any back-compatibility issue also. Regarding the new QP framework, I don't see any problem about back compatibility, because Lucene will only be Java 1.5 on version 3.0, and back compatibility may be broken. But... I would see nothing wrong with having a jdk 1.4 version if we were 100% compatible with the old queryparser, but since that is not the case, I don't think it is worth it. (the wrapper we built does not support the case where users extend the old queryparser class and overwrite methods to add new functionality) I agree with Luis, if we only release the new QP framework 2.9, we will definitely brake the back-compatiblity of the old QP, so, why not release the old and the new QP together on 2.9? Suggestions? Best Regards, Adriano Crestani Campos Adriano Crestani Campos
          Hide
          Luis Alves added a comment -

          There will be a couple of more changes need:
          We also have to change "List change <QueryNode> getchildren();" and "public Map<CharSequence, Object> getTags();"
          We also have change QueryNodeImpl, we will have to patch all QueryNode classes implementations and perform forced casts.
          and users implementing QueryNode's will also have to do that.

          It's about 30 changes, not that a big change, I agree. But if we release both parsers I see no need to change it.

          > I agree with Luis, if we only release the new QP framework 2.9, we will definitely brake the back-compatiblity of the old QP,
          > so, why not release the old and the new QP together on 2.9?

          Some extras:
          If we chose to release both parsers, we should deprecate the old one,
          allowing people to migrate to the new one with release 2.9. and drop the old queryparser classes on 3.0.
          (we can keep the wrappers in 2.9 throwing exceptions in all methods to remind people to move to the new framework
          we probably can also keep the wrapper in 3.0, if we think is still necessary).

          Show
          Luis Alves added a comment - There will be a couple of more changes need: We also have to change "List change <QueryNode> getchildren();" and "public Map<CharSequence, Object> getTags();" We also have change QueryNodeImpl, we will have to patch all QueryNode classes implementations and perform forced casts. and users implementing QueryNode's will also have to do that. It's about 30 changes, not that a big change, I agree. But if we release both parsers I see no need to change it. > I agree with Luis, if we only release the new QP framework 2.9, we will definitely brake the back-compatiblity of the old QP, > so, why not release the old and the new QP together on 2.9? Some extras: If we chose to release both parsers, we should deprecate the old one, allowing people to migrate to the new one with release 2.9. and drop the old queryparser classes on 3.0. (we can keep the wrappers in 2.9 throwing exceptions in all methods to remind people to move to the new framework we probably can also keep the wrapper in 3.0, if we think is still necessary).
          Hide
          Adriano Crestani added a comment -

          > We also have to change "List change <QueryNode> getchildren();" and "public Map<CharSequence, Object> getTags();"

          I already mentioned about getChildren(), I forgot about getTags(), thanks for reminding me : )...I looked into the patch attached, it does not contain that yet

          > We also have change QueryNodeImpl, we will have to patch all QueryNode classes implementations and perform forced casts.
          > and users implementing QueryNode's will also have to do that.

          Yes, these are internal changes, they shouldn't affect anything, even after released with 2.9. If some user extends these classes, they will need to do the type checking and follow the documentation as I said before. Also, there would be no impact when they migrate to 3.0 when we re-add the generics, they will just keep performing the type checking on the List objects and verifying if they are QueryNodes or not.

          > Some extras:
          > If we chose to release both parsers, we should deprecate the old one,
          > allowing people to migrate to the new one with release 2.9. and drop the old queryparser classes on 3.0.
          > (we can keep the wrappers in 2.9 throwing exceptions in all methods to remind people to move to the new framework
          > we probably can also keep the wrapper in 3.0, if we think is still necessary).

          Completely agree

          Show
          Adriano Crestani added a comment - > We also have to change "List change <QueryNode> getchildren();" and "public Map<CharSequence, Object> getTags();" I already mentioned about getChildren(), I forgot about getTags(), thanks for reminding me : )...I looked into the patch attached, it does not contain that yet > We also have change QueryNodeImpl, we will have to patch all QueryNode classes implementations and perform forced casts. > and users implementing QueryNode's will also have to do that. Yes, these are internal changes, they shouldn't affect anything, even after released with 2.9. If some user extends these classes, they will need to do the type checking and follow the documentation as I said before. Also, there would be no impact when they migrate to 3.0 when we re-add the generics, they will just keep performing the type checking on the List objects and verifying if they are QueryNodes or not. > Some extras: > If we chose to release both parsers, we should deprecate the old one, > allowing people to migrate to the new one with release 2.9. and drop the old queryparser classes on 3.0. > (we can keep the wrappers in 2.9 throwing exceptions in all methods to remind people to move to the new framework > we probably can also keep the wrapper in 3.0, if we think is still necessary). Completely agree
          Hide
          Michael Busch added a comment -

          Adriano and Luis:

          I think a few things need to be changed/improved here in order to commit this for 2.9:

          • JDK 1.4 compatibility
          • Backwards-compatibility, which means the patch should deprecate the old QueryParser, not remove it
          • A class/interface that is as easy to use as the old QueryParser for people who simply want to parse a query string; those users shouldn't have to know about what a processor or builder chain is
          • Better documentation about how to easily switch over from the old to the new QueryParser

          We're talking on java-dev about releasing 2.9 fairly soon; how long would these changes take? I'd like to get this in 2.9 because it seems to be a popular issue.

          Show
          Michael Busch added a comment - Adriano and Luis: I think a few things need to be changed/improved here in order to commit this for 2.9: JDK 1.4 compatibility Backwards-compatibility, which means the patch should deprecate the old QueryParser, not remove it A class/interface that is as easy to use as the old QueryParser for people who simply want to parse a query string; those users shouldn't have to know about what a processor or builder chain is Better documentation about how to easily switch over from the old to the new QueryParser We're talking on java-dev about releasing 2.9 fairly soon; how long would these changes take? I'd like to get this in 2.9 because it seems to be a popular issue.
          Hide
          Grant Ingersoll added a comment - - edited

          I need an MD5/SHA1 hash (http://incubator.apache.org/ip-clearance/ip-clearance-template.html) for the exact code listed in the software grant. Also include the version number of the software used to create the hash.

          Please also upload that code as a tarball on this issue. No need to worry about the patches for now.

          See https://issues.apache.org/jira/browse/INCUBATOR-77 for example.

          Show
          Grant Ingersoll added a comment - - edited I need an MD5/SHA1 hash ( http://incubator.apache.org/ip-clearance/ip-clearance-template.html ) for the exact code listed in the software grant. Also include the version number of the software used to create the hash. Please also upload that code as a tarball on this issue. No need to worry about the patches for now. See https://issues.apache.org/jira/browse/INCUBATOR-77 for example.
          Hide
          Grant Ingersoll added a comment -

          From the IP Clearance, consider yourself reminded:

          Remind active committers that they are responsible for
          ensuring that a Corporate CLA is recorded if such is
          required to authorize their contributions under their
          individual CLA.

          Show
          Grant Ingersoll added a comment - From the IP Clearance, consider yourself reminded: Remind active committers that they are responsible for ensuring that a Corporate CLA is recorded if such is required to authorize their contributions under their individual CLA.
          Hide
          Michael Busch added a comment -

          But we still need to update the code before we can commit. From which patch do you need the MD5/SHA1 hash from?

          Show
          Michael Busch added a comment - But we still need to update the code before we can commit. From which patch do you need the MD5/SHA1 hash from?
          Hide
          Grant Ingersoll added a comment -

          OK, only outstanding items for clearance are:
          1. tarball and hash
          2. Vote on Incubator for clearance.

          Show
          Grant Ingersoll added a comment - OK, only outstanding items for clearance are: 1. tarball and hash 2. Vote on Incubator for clearance.
          Hide
          Adriano Crestani added a comment -

          Hi Michael,

          I expect it takes one week at max!

          Show
          Adriano Crestani added a comment - Hi Michael, I expect it takes one week at max!
          Hide
          Michael Busch added a comment -

          Ok GO!

          Show
          Michael Busch added a comment - Ok GO!
          Hide
          Grant Ingersoll added a comment -

          Commit is separate from IP Clearance and you can't commit until the clearance is accepted.

          I just need the tarball for the code that was referenced in the software grant along with a hash on it. In the grant, you have a file directory listing describing the code. Take that file listing, tar it up and run md5 on it.

          Show
          Grant Ingersoll added a comment - Commit is separate from IP Clearance and you can't commit until the clearance is accepted. I just need the tarball for the code that was referenced in the software grant along with a hash on it. In the grant, you have a file directory listing describing the code. Take that file listing, tar it up and run md5 on it.
          Hide
          Michael Busch added a comment -

          OK that should be easy. We'll do that asap. Thanks for explaining, Grant.

          Show
          Michael Busch added a comment - OK that should be easy. We'll do that asap. Thanks for explaining, Grant.
          Hide
          Michael Busch added a comment -

          MD5 (new_query_parser_src.tar) = b678596e3dea63e8e66e035d6dc7f45e

          On Jul 4, 2009, at 5:17 PM, Michael Busch wrote:

          Hi Grant,

          attached is the tar file that includes the files that were listed in the software grant. These files contain all the IP of this new feature that was developed internally in IBM. However, the final patch that will be committed will look a bit different, due to discussions with the other committers, which of course take now place on the public mailinglist.

          On 7/5/09 8:15 PM, Grant Ingersoll wrote:

          Please attach to the issue. No worries on the other part, just need the bits there for me to say they exist and align w/ the Grant. What we commit can be patched.

          Show
          Michael Busch added a comment - MD5 (new_query_parser_src.tar) = b678596e3dea63e8e66e035d6dc7f45e On Jul 4, 2009, at 5:17 PM, Michael Busch wrote: Hi Grant, attached is the tar file that includes the files that were listed in the software grant. These files contain all the IP of this new feature that was developed internally in IBM. However, the final patch that will be committed will look a bit different, due to discussions with the other committers, which of course take now place on the public mailinglist. On 7/5/09 8:15 PM, Grant Ingersoll wrote: Please attach to the issue. No worries on the other part, just need the bits there for me to say they exist and align w/ the Grant. What we commit can be patched.
          Hide
          Mark Miller added a comment -

          I wonder if all of this was really necessary. Months ago, while doing some searching, I saw that at least one other Apache project (might have been on the legal email list?), asked about a large code contribution from a company, and the response was that if a guy at the company had a CLA on file (was a committer), and was part of the process, he could commit the large code contribution without all of this paperwork mumbo jumbo.

          Of course, best to be thorough and complete, but I think we may not have to jump through these same hoops in the future.

          Not that that means much, as I say that with no authority or complete knowledge about it. But if someone wanted to research further ...

          Show
          Mark Miller added a comment - I wonder if all of this was really necessary. Months ago, while doing some searching, I saw that at least one other Apache project (might have been on the legal email list?), asked about a large code contribution from a company, and the response was that if a guy at the company had a CLA on file (was a committer), and was part of the process, he could commit the large code contribution without all of this paperwork mumbo jumbo. Of course, best to be thorough and complete, but I think we may not have to jump through these same hoops in the future. Not that that means much, as I say that with no authority or complete knowledge about it. But if someone wanted to research further ...
          Hide
          Grant Ingersoll added a comment -

          I saw that at least one other Apache project

          Just because someone else does it wrong... It's pretty clear in this case that the Grant is necessary.

          Show
          Grant Ingersoll added a comment - I saw that at least one other Apache project Just because someone else does it wrong... It's pretty clear in this case that the Grant is necessary.
          Hide
          Mark Miller added a comment -

          Someone else didnt do it wrong - they asked and got an answer from someone from Apache that seemed to know what they were talking about, and seemed to have the authority/knowledge to give the answer they gave.

          I'm not saying something one way or another - just throwing what I saw out there. I'm sure you have more info on the subject than I do.

          Show
          Mark Miller added a comment - Someone else didnt do it wrong - they asked and got an answer from someone from Apache that seemed to know what they were talking about, and seemed to have the authority/knowledge to give the answer they gave. I'm not saying something one way or another - just throwing what I saw out there. I'm sure you have more info on the subject than I do.
          Hide
          Mark Miller added a comment -

          Hmmm - if you look at the strict letter of the law in Intellectual Property Clearance, then LocalLucene and Trie and a lot of other stuff also needed this clearance ... I may have just missed the process on those though.

          Show
          Mark Miller added a comment - Hmmm - if you look at the strict letter of the law in Intellectual Property Clearance, then LocalLucene and Trie and a lot of other stuff also needed this clearance ... I may have just missed the process on those though.
          Hide
          Grant Ingersoll added a comment -

          LocalLucene did. Not sure about Trie. Anyway, this issue is not the place for this discussion.

          Show
          Grant Ingersoll added a comment - LocalLucene did. Not sure about Trie. Anyway, this issue is not the place for this discussion.
          Hide
          Mark Miller added a comment -

          Anyway, this issue is not the place for this discussion.

          Seems like a couple comments about this here is appropriate to me. Your just being prickly man. I was pointing something out that has relevance to this issue and relevance to committers when dealing with future similar issues.

          My comment about LocalLucene and Trie were not an accusation, but an attempt to clarify what requires this and what doesn't. As a committer, its important that this information is clear to me. As the PMC head, I'd think youd be more helpful with the matter.

          Show
          Mark Miller added a comment - Anyway, this issue is not the place for this discussion. Seems like a couple comments about this here is appropriate to me. Your just being prickly man. I was pointing something out that has relevance to this issue and relevance to committers when dealing with future similar issues. My comment about LocalLucene and Trie were not an accusation, but an attempt to clarify what requires this and what doesn't. As a committer, its important that this information is clear to me. As the PMC head, I'd think youd be more helpful with the matter.
          Hide
          Uwe Schindler added a comment -

          Not sure about Trie

          Trie was not property of a company, it was my private idea (and even if I work at the University of Bremen, which sponsors me, it is not owned by the University. Scientific research in Germany is the scientist's responsibility). And the code was already Apache 2.0 licensed, so there was no problem to donate it. And now I am committer and already signed the CLA. If there is still a problem, I would open another issue about that.

          Show
          Uwe Schindler added a comment - Not sure about Trie Trie was not property of a company, it was my private idea (and even if I work at the University of Bremen, which sponsors me, it is not owned by the University. Scientific research in Germany is the scientist's responsibility). And the code was already Apache 2.0 licensed, so there was no problem to donate it. And now I am committer and already signed the CLA. If there is still a problem, I would open another issue about that.
          Hide
          Mark Miller added a comment -

          I'll just keep my response out of JIRA to avoid taking over that issue:

          According to http://incubator.apache.org/ip-clearance/index.html, it
          doesn't matter if it was your companys code or if you are a committer or
          if you have a CLA. If it was developed outside of Apache svn/mailing
          lists and was then donated, it says it needs the grant.

          • Mark

          • Mark

          http://www.lucidimagination.com

          Show
          Mark Miller added a comment - I'll just keep my response out of JIRA to avoid taking over that issue: According to http://incubator.apache.org/ip-clearance/index.html , it doesn't matter if it was your companys code or if you are a committer or if you have a CLA. If it was developed outside of Apache svn/mailing lists and was then donated, it says it needs the grant. Mark – Mark http://www.lucidimagination.com
          Hide
          Grant Ingersoll added a comment -

          And the code was already Apache 2.0 licensed, so there was no problem to donate it.

          This does not matter, nor does the license. I was unaware that it lived in public someplace else. If the code lives somewhere else in public, then it needs to go through Soft. Grant, AIUI. Having it licensed as ASL just makes the paperwork a formality. At any rate, as I said, the discussion of Trie, LocalLucene and when some generic piece of code needs a grant has nothing to do with this particular issue, so please, if you want to continue this conversation, then start one on java-dev.

          Show
          Grant Ingersoll added a comment - And the code was already Apache 2.0 licensed, so there was no problem to donate it. This does not matter, nor does the license. I was unaware that it lived in public someplace else. If the code lives somewhere else in public, then it needs to go through Soft. Grant, AIUI. Having it licensed as ASL just makes the paperwork a formality. At any rate, as I said, the discussion of Trie, LocalLucene and when some generic piece of code needs a grant has nothing to do with this particular issue, so please, if you want to continue this conversation, then start one on java-dev.
          Hide
          Luis Alves added a comment -

          Since all legal work is finally finished now, it is time for an updated patch with the latest fixes and improvements.

          Below are the changes compared to the previous patch:
          • moved the new queryparser to contrib
          • deprecated old QueryParser classes in the core
          • the new queryparser in contrib uses jdk 1.5
          • patch compiles against current trunk
          • rewrote the lucene testcases to use the new API's
          • created wrapper testcases that uses wrapper classes
          • created classes to overwrite the old QueryParser in the util folder, and make Lucene use the flexible query parser engine, without having to change your code.

          I verified that all testcases are working, and that all contrib modules still compile fine.

          Adriano when you have some time, can you write an interface for simple usage of the new QueryParser, and a simple implementation of the interface, that creates a textparser, creates a processor pipeline, and instantiates the lucene builders?

          And please add a simple junit that demonstrates the usage of that interface and ideally some documentation into the package.html of the new contrib package that will help users who want to use the queryparser to get started.

          Show
          Luis Alves added a comment - Since all legal work is finally finished now, it is time for an updated patch with the latest fixes and improvements. Below are the changes compared to the previous patch: • moved the new queryparser to contrib • deprecated old QueryParser classes in the core • the new queryparser in contrib uses jdk 1.5 • patch compiles against current trunk • rewrote the lucene testcases to use the new API's • created wrapper testcases that uses wrapper classes • created classes to overwrite the old QueryParser in the util folder, and make Lucene use the flexible query parser engine, without having to change your code. I verified that all testcases are working, and that all contrib modules still compile fine. Adriano when you have some time, can you write an interface for simple usage of the new QueryParser, and a simple implementation of the interface, that creates a textparser, creates a processor pipeline, and instantiates the lucene builders? And please add a simple junit that demonstrates the usage of that interface and ideally some documentation into the package.html of the new contrib package that will help users who want to use the queryparser to get started.
          Hide
          Luis Alves added a comment -

          patch compiles against current trunk

          Show
          Luis Alves added a comment - patch compiles against current trunk
          Hide
          David Sitsky added a comment -

          I will be out of the office on Friday, 10th of July.


          Cheers,
          David

          Nuix Pty Ltd
          Suite 79, 89 Jones St, Ultimo NSW 2007, Australia Ph: +61 2 9280 0699
          Web: http://www.nuix.com Fax: +61 2 9212 6902

          Show
          David Sitsky added a comment - I will be out of the office on Friday, 10th of July. – Cheers, David Nuix Pty Ltd Suite 79, 89 Jones St, Ultimo NSW 2007, Australia Ph: +61 2 9280 0699 Web: http://www.nuix.com Fax: +61 2 9212 6902
          Hide
          Michael Busch added a comment -

          Luis, I think you need to modify the main build.xml, because the query parser contrib uses java 1.5. For an example look into the build.xml from Lucene 2.2.x. It had a contrib called gdata, which used JRE 1.5. (This was removed after 2.2, so you won't find it in the current build.xml anymore).

          Currently the build will fail if the user runs JRE 1.4, but it should rather skip the new query parser contrib. You can use this property, which is definied in common-build.xml:

          <condition property="build-1-5-contrib">
            <equals arg1="1.5" arg2="${ant.java.version}" />
          </condition>
          
          Show
          Michael Busch added a comment - Luis, I think you need to modify the main build.xml, because the query parser contrib uses java 1.5. For an example look into the build.xml from Lucene 2.2.x. It had a contrib called gdata, which used JRE 1.5. (This was removed after 2.2, so you won't find it in the current build.xml anymore). Currently the build will fail if the user runs JRE 1.4, but it should rather skip the new query parser contrib. You can use this property, which is definied in common-build.xml: <condition property= "build-1-5-contrib" > <equals arg1= "1.5" arg2= "${ant.java.version}" /> </condition>
          Hide
          Adriano Crestani added a comment -

          Hi Luis,

          I have been improving the code documentation lately, I will merge my diff with your new patch and submit the changes soon. I also could merge with the trunk, it depends when last Luis' patch will be committed.

          Adriano when you have some time, can you write an interface for simple usage of the new QueryParser, and a simple implementation of the interface, that creates a textparser, creates a processor pipeline, and instantiates the Lucene builders?

          Good idea Luis! I was thinking about a class that would allow query parser implementors to "bundle" their processor, text parser and builder in it, so the user could simply use it, nobody needs to know how it's implemented. I think the class should contain a method parse(String defaultField, String queryString) that returns whatever that query parser creates from it, in Lucene's case, a Query object. Also, some sets and gets to access the internal processor, builder and text parser, if the user wishes to. I'm gonna work more on the design and submit a patch soon containing it.

          And please add a simple junit that demonstrates the usage of that interface and ideally some documentation into the package.html of the new contrib package that will help users who want to use the queryparser to get started.

          I was also thinking about a wiki page that would guide Lucene users to migrate to the new query parser using this new interface.

          More suggestions?

          Show
          Adriano Crestani added a comment - Hi Luis, I have been improving the code documentation lately, I will merge my diff with your new patch and submit the changes soon. I also could merge with the trunk, it depends when last Luis' patch will be committed. Adriano when you have some time, can you write an interface for simple usage of the new QueryParser, and a simple implementation of the interface, that creates a textparser, creates a processor pipeline, and instantiates the Lucene builders? Good idea Luis! I was thinking about a class that would allow query parser implementors to "bundle" their processor, text parser and builder in it, so the user could simply use it, nobody needs to know how it's implemented. I think the class should contain a method parse(String defaultField, String queryString) that returns whatever that query parser creates from it, in Lucene's case, a Query object. Also, some sets and gets to access the internal processor, builder and text parser, if the user wishes to. I'm gonna work more on the design and submit a patch soon containing it. And please add a simple junit that demonstrates the usage of that interface and ideally some documentation into the package.html of the new contrib package that will help users who want to use the queryparser to get started. I was also thinking about a wiki page that would guide Lucene users to migrate to the new query parser using this new interface. More suggestions?
          Hide
          Luis Alves added a comment -

          fix for jdk 1.4, on build.xml

          Show
          Luis Alves added a comment - fix for jdk 1.4, on build.xml
          Hide
          Luis Alves added a comment -

          Hi Michael,

          > For an example look into the build.xml from Lucene 2.2.x.
          The ant file on this Lucene 2.2 module does not follow the lucene convention and it uses a complex implementation.
          So I fixed the problem in a different way:
          I renamed the contrib/queryparser/build.xml to build15.xml,
          and I fixed the contrib-crawl to include build15.xml when a jdk15 is present.

          I tested default, build-contrib, javadocs-contrib all work fine.

          I just uploaded the patch v5 with this fix.

          Show
          Luis Alves added a comment - Hi Michael, > For an example look into the build.xml from Lucene 2.2.x. The ant file on this Lucene 2.2 module does not follow the lucene convention and it uses a complex implementation. So I fixed the problem in a different way: I renamed the contrib/queryparser/build.xml to build15.xml, and I fixed the contrib-crawl to include build15.xml when a jdk15 is present. I tested default, build-contrib, javadocs-contrib all work fine. I just uploaded the patch v5 with this fix.
          Hide
          Adriano Crestani added a comment -

          Hey guys,

          Here is a patch containing some changes I did on top of last Luis' patch ( lucene_trunk_FlexQueryParser_2009July10_v5.patch):

          • javadoc reviewed and improved
          • 2 new classes: QueryParserHelper and LuceneQueryParserHelper, they make it easier to use the new query parser
          • added the ability to set the prefix length for fuzzy queries, it was still missing in the new query parser
          • resolved some TODOs
          • AnalyzerQueryNodeProcessor is now using only the new TokenStream API...is it required to be compatible with the old API even if it is in contrib?
          • I duplicated the test cases so they run using the query parser API directly, the query parser helpers and the query parser wrappers, this way we test the three ways the user can actually use the query parser.

          I think that is everything. I will keep reviewing and improving the documentation, I think there might be some broken javadoc links yet.

          I also would like to rename the package and everythiing else that does reference to "lucene2" to "lucene". I think it does not make sense to have a package name tied to a version. So, the package org.apache.lucene.queryParser.lucene2 would be renamed to org.apache.lucene.queryParser.lucene. I know it's kind of weird, because there are 2 "lucene" in the package declararion, but I think it's better than "lucene2". Anyway, suggestions about this are welcome ... if nobody replies I will feel free to rename it and submit a new patch soon.

          I will also work on writing a documentation for Lucene wiki that explains how to easily migrate from the old query parser to the new one, but I will only add it to the wiki when the code is committed to the trunk, it doesn't make sense a wiki documentation about something that is not even committed, agreed?

          Suggestions?

          Regards,
          Adriano Crestani Campos

          Show
          Adriano Crestani added a comment - Hey guys, Here is a patch containing some changes I did on top of last Luis' patch ( lucene_trunk_FlexQueryParser_2009July10_v5.patch): javadoc reviewed and improved 2 new classes: QueryParserHelper and LuceneQueryParserHelper, they make it easier to use the new query parser added the ability to set the prefix length for fuzzy queries, it was still missing in the new query parser resolved some TODOs AnalyzerQueryNodeProcessor is now using only the new TokenStream API...is it required to be compatible with the old API even if it is in contrib? I duplicated the test cases so they run using the query parser API directly, the query parser helpers and the query parser wrappers, this way we test the three ways the user can actually use the query parser. I think that is everything. I will keep reviewing and improving the documentation, I think there might be some broken javadoc links yet. I also would like to rename the package and everythiing else that does reference to "lucene2" to "lucene". I think it does not make sense to have a package name tied to a version. So, the package org.apache.lucene.queryParser.lucene2 would be renamed to org.apache.lucene.queryParser.lucene. I know it's kind of weird, because there are 2 "lucene" in the package declararion, but I think it's better than "lucene2". Anyway, suggestions about this are welcome ... if nobody replies I will feel free to rename it and submit a new patch soon. I will also work on writing a documentation for Lucene wiki that explains how to easily migrate from the old query parser to the new one, but I will only add it to the wiki when the code is committed to the trunk, it doesn't make sense a wiki documentation about something that is not even committed, agreed? Suggestions? Regards, Adriano Crestani Campos
          Hide
          Adriano Crestani added a comment -

          Ah, I also couldn't run "ant build-contrib" using Java 1.4, it fails, I even tried a clean trunk and it did not work. Were you able to run it using 1.4 Luis?

          I already opened a thread on the ML about this: http://markmail.org/thread/3fyldf7t423fhwbm

          Show
          Adriano Crestani added a comment - Ah, I also couldn't run "ant build-contrib" using Java 1.4, it fails, I even tried a clean trunk and it did not work. Were you able to run it using 1.4 Luis? I already opened a thread on the ML about this: http://markmail.org/thread/3fyldf7t423fhwbm
          Hide
          Adriano Crestani added a comment -

          Ah, I also couldn't run "ant build-contrib" using Java 1.4, it fails, I even tried a clean trunk and it did not work. Were you able to run it using 1.4 Luis?

          I already opened a thread on the ML about this: http://markmail.org/thread/3fyldf7t423fhwbm

          Mark Miller just replied to the thread and based on his response there is no need for contrib projects to be able to compile using JDK 1.4. So, Luis, could you rollback your changes you did on the build files?

          Thanks,
          Adriano Crestani Campos

          Show
          Adriano Crestani added a comment - Ah, I also couldn't run "ant build-contrib" using Java 1.4, it fails, I even tried a clean trunk and it did not work. Were you able to run it using 1.4 Luis? I already opened a thread on the ML about this: http://markmail.org/thread/3fyldf7t423fhwbm Mark Miller just replied to the thread and based on his response there is no need for contrib projects to be able to compile using JDK 1.4. So, Luis, could you rollback your changes you did on the build files? Thanks, Adriano Crestani Campos
          Hide
          Mark Miller added a comment -

          Hang on a sec - it sounds like the target was 1.4 because this was going to replace a 1.4 core piece of functionality.

          I don't know that all of the details are fully straightened out though.

          1. I'm not pro moving the QueryParser to contrib myself, unless we actually move forward on that 'modules' thread - if not, it doesn't appear very helpful to me.

          2. If we move this to contrib, perhaps it can be 1.5? But then in 3.0, can we have 1.5 already? Or is that 3.1? If its 3.1, than if we remove the deprecated query parser in 3.0, you won't have a java 1.4 replacement to move to (if course we could keep the old QueryParser till 4.0 ... ). I'm not clear that we can't add new functionality to 3.0 though. I know Mike has mentioned it, but I can't find where it says that - I just see that we can remove deprecations, not that we can't also add new features. I may be missing something though?

          We should get things fully straightened out before you spend too much time switching between 1.4 and 1.5 though.

          Show
          Mark Miller added a comment - Hang on a sec - it sounds like the target was 1.4 because this was going to replace a 1.4 core piece of functionality. I don't know that all of the details are fully straightened out though. 1. I'm not pro moving the QueryParser to contrib myself, unless we actually move forward on that 'modules' thread - if not, it doesn't appear very helpful to me. 2. If we move this to contrib, perhaps it can be 1.5? But then in 3.0, can we have 1.5 already? Or is that 3.1? If its 3.1, than if we remove the deprecated query parser in 3.0, you won't have a java 1.4 replacement to move to (if course we could keep the old QueryParser till 4.0 ... ). I'm not clear that we can't add new functionality to 3.0 though. I know Mike has mentioned it, but I can't find where it says that - I just see that we can remove deprecations, not that we can't also add new features. I may be missing something though? We should get things fully straightened out before you spend too much time switching between 1.4 and 1.5 though.
          Hide
          Michael Busch added a comment -

          Mark,

          it seems like the best thing to do here is to add this as a 1.5 contrib for now and deprecate the core query parser. Then in 3.0 we would move the new one into core and remove the old one entirely. Since it will remain in the same package users won't have to change their code, just while they use 2.9 they have to put an extra jar in their classpath.

          Looking at the latest patch, that's what it does (new one to contrib while deprecating old one).

          Show
          Michael Busch added a comment - Mark, it seems like the best thing to do here is to add this as a 1.5 contrib for now and deprecate the core query parser. Then in 3.0 we would move the new one into core and remove the old one entirely. Since it will remain in the same package users won't have to change their code, just while they use 2.9 they have to put an extra jar in their classpath. Looking at the latest patch, that's what it does (new one to contrib while deprecating old one).
          Hide
          Luis Alves added a comment -

          Adriano,

          I will rollback the build.xml changes tomorrow, and use the convention that the "spatial" and "fast-vector-highlighter" modules use.

          On the package name "lucene2":
          I think during the Lucene 3.X development more parsers will be added to the QueryParser, and these parsers will also be lucene parsers and we will need different names. It is probably better to keep lucene2 on the package name, or use a name that makes a reference to the old queryparser.

          For example, in the future we could have:
          org.apache.lucene.queryParser.lucene2 <- lucene 2.X syntax
          org.apache.lucene.queryParser.lucene3 <- lucene 3.X syntax
          org.apache.lucene.queryParser.xml <- some XML syntax
          org.apache.lucene.queryParser.luceneBoolean <- boolean syntax
          org.apache.lucene.queryParser.explicit <- explict query language syntax

          I'll also help on the when wiki the code is committed to the trunk.

          Show
          Luis Alves added a comment - Adriano, I will rollback the build.xml changes tomorrow, and use the convention that the "spatial" and "fast-vector-highlighter" modules use. On the package name "lucene2": I think during the Lucene 3.X development more parsers will be added to the QueryParser, and these parsers will also be lucene parsers and we will need different names. It is probably better to keep lucene2 on the package name, or use a name that makes a reference to the old queryparser. For example, in the future we could have: org.apache.lucene.queryParser.lucene2 <- lucene 2.X syntax org.apache.lucene.queryParser.lucene3 <- lucene 3.X syntax org.apache.lucene.queryParser.xml <- some XML syntax org.apache.lucene.queryParser.luceneBoolean <- boolean syntax org.apache.lucene.queryParser.explicit <- explict query language syntax I'll also help on the when wiki the code is committed to the trunk.
          Hide
          Grant Ingersoll added a comment -

          Names that tack a "2" or some other number on the end are pretty much meaningless. I'd suggest finding something better that actually describes what the package contains. After all what is the "second" query parser?

          Show
          Grant Ingersoll added a comment - Names that tack a "2" or some other number on the end are pretty much meaningless. I'd suggest finding something better that actually describes what the package contains. After all what is the "second" query parser?
          Hide
          Mark Miller added a comment -

          Mark,

          it seems like the best thing to do here is to add this as a 1.5 contrib for now and deprecate the core query parser. Then in 3.0 we would move the new one into core and remove the old one entirely. Since it will remain in the same package users won't have to change their code, just while they use 2.9 they have to put an extra jar in their classpath.

          Looking at the latest patch, that's what it does (new one to contrib while deprecating old one).

          Right, I think that does make sense, but can we actually go to 1.5 in 3.0? Thats what my main question is around. I know the 1.5 wiki says that we can, but Mike has indicated that 3.0 would just be a quick bug fix release with deprecations removed from 2.9. I thought I'd seen him say that 3.1 would actually be the first with 1.5?

          Mike M?

          Show
          Mark Miller added a comment - Mark, it seems like the best thing to do here is to add this as a 1.5 contrib for now and deprecate the core query parser. Then in 3.0 we would move the new one into core and remove the old one entirely. Since it will remain in the same package users won't have to change their code, just while they use 2.9 they have to put an extra jar in their classpath. Looking at the latest patch, that's what it does (new one to contrib while deprecating old one). Right, I think that does make sense, but can we actually go to 1.5 in 3.0? Thats what my main question is around. I know the 1.5 wiki says that we can, but Mike has indicated that 3.0 would just be a quick bug fix release with deprecations removed from 2.9. I thought I'd seen him say that 3.1 would actually be the first with 1.5? Mike M?
          Hide
          Grant Ingersoll added a comment -
          Show
          Grant Ingersoll added a comment - 3.0 will be 1.5. See http://wiki.apache.org/lucene-java/Java_1.5_Migration
          Hide
          Michael McCandless added a comment -

          Right, 3.0 is when we can first use 1.5 code.

          But, 3.0 will be a fast "mechanical" release after 2.9. This is just
          like the 1.9 -> 2.0 fast turnaround, except because we begin
          accepting 1.5 code in 3.0 we may make certain changes (switch to
          generics in certain APIs; move the new QueryParser into core; etc.).

          However we don't plan on doing any new features, etc in 3.0; that will
          first happen in 3.1.

          Show
          Michael McCandless added a comment - Right, 3.0 is when we can first use 1.5 code. But, 3.0 will be a fast "mechanical" release after 2.9. This is just like the 1.9 -> 2.0 fast turnaround, except because we begin accepting 1.5 code in 3.0 we may make certain changes (switch to generics in certain APIs; move the new QueryParser into core; etc.). However we don't plan on doing any new features, etc in 3.0; that will first happen in 3.1.
          Hide
          Mark Miller added a comment -

          Yeah, I had seen that, I was just remembering an email or two from Mike that mentioned differently (waiting till 3.1) ... but I just found one of the threads discussing it and it looks like consensus shifted:

          http://www.lucidimagination.com/search/document/6d2b6488b4115/2_9_3_0_plan_java_1_5#6d2b6488b4115

          Show
          Mark Miller added a comment - Yeah, I had seen that, I was just remembering an email or two from Mike that mentioned differently (waiting till 3.1) ... but I just found one of the threads discussing it and it looks like consensus shifted: http://www.lucidimagination.com/search/document/6d2b6488b4115/2_9_3_0_plan_java_1_5#6d2b6488b4115
          Hide
          Michael Busch added a comment -

          except because we begin
          accepting 1.5 code in 3.0 we may make certain changes (switch to
          generics in certain APIs; move the new QueryParser into core; etc.).

          OK sounds like a plan then! The new QP code will not change, but we'll move it into core in 3.0.

          Show
          Michael Busch added a comment - except because we begin accepting 1.5 code in 3.0 we may make certain changes (switch to generics in certain APIs; move the new QueryParser into core; etc.). OK sounds like a plan then! The new QP code will not change, but we'll move it into core in 3.0.
          Hide
          Adriano Crestani added a comment -

          except because we begin
          accepting 1.5 code in 3.0 we may make certain changes (switch to
          generics in certain APIs; move the new QueryParser into core; etc.).

          OK sounds like a plan then! The new QP code will not change, but we'll move it into core in 3.0.

          Thanks for the explanation!

          Names that tack a "2" or some other number on the end are pretty much meaningless. I'd suggest finding something better that actually describes what the package contains. After all what is the "second" query parser?

          I agree with Luis, it's a good idea to have a package for each different query parser implementation. I also agree with Grant that it does not make sense to have an implementation tied to a number. So, as the "lucene2" implementation contains the default/main Lucene query parser implementation, I would suggest to rename it to "defaultLucene", "default" or "main". I will give +1 for "default".

          Regards,
          Adriano Crestani Campos

          Show
          Adriano Crestani added a comment - except because we begin accepting 1.5 code in 3.0 we may make certain changes (switch to generics in certain APIs; move the new QueryParser into core; etc.). OK sounds like a plan then! The new QP code will not change, but we'll move it into core in 3.0. Thanks for the explanation! Names that tack a "2" or some other number on the end are pretty much meaningless. I'd suggest finding something better that actually describes what the package contains. After all what is the "second" query parser? I agree with Luis, it's a good idea to have a package for each different query parser implementation. I also agree with Grant that it does not make sense to have an implementation tied to a number. So, as the "lucene2" implementation contains the default/main Lucene query parser implementation, I would suggest to rename it to "defaultLucene", "default" or "main". I will give +1 for "default". Regards, Adriano Crestani Campos
          Hide
          Luis Alves added a comment -
          • Undo the changes on the build file to skip queryparser module if jdk 1.4 was found.
          • Include Adriano changes
          Show
          Luis Alves added a comment - Undo the changes on the build file to skip queryparser module if jdk 1.4 was found. Include Adriano changes
          Hide
          Luis Alves added a comment -

          I upload the patch that undo my changes on the build files to skip the queryparser module if jdk 1.4 was found
          This latest patch also includes Adriano's changes.

          I agree with Luis, it's a good idea to have a package for each different query parser implementation. I also agree with Grant that it does not make sense to have an implementation tied to a number. So, as the "lucene2" implementation contains the default/main Lucene query parser implementation, I would suggest to rename it to "defaultLucene", "default" or "main". I will give +1 for "default".

          I'll add 2 more suggestions
          "standard", "standardSyntax". I will give +1 for "standard".

          Show
          Luis Alves added a comment - I upload the patch that undo my changes on the build files to skip the queryparser module if jdk 1.4 was found This latest patch also includes Adriano's changes. I agree with Luis, it's a good idea to have a package for each different query parser implementation. I also agree with Grant that it does not make sense to have an implementation tied to a number. So, as the "lucene2" implementation contains the default/main Lucene query parser implementation, I would suggest to rename it to "defaultLucene", "default" or "main". I will give +1 for "default". I'll add 2 more suggestions "standard", "standardSyntax". I will give +1 for "standard".
          Hide
          Adriano Crestani added a comment -

          I upload the patch that undo my changes on the build files to skip the queryparser module if jdk 1.4 was found

          Cool!

          I'll add 2 more suggestions
          "standard", "standardSyntax". I will give +1 for "standard".

          I would like to suggest also "original" and my +1 for "original"

          Show
          Adriano Crestani added a comment - I upload the patch that undo my changes on the build files to skip the queryparser module if jdk 1.4 was found Cool! I'll add 2 more suggestions "standard", "standardSyntax". I will give +1 for "standard". I would like to suggest also "original" and my +1 for "original"
          Hide
          Luis Alves added a comment -

          I would like to suggest also "original" and my +1 for "original"

          +1 for "original"

          Show
          Luis Alves added a comment - I would like to suggest also "original" and my +1 for "original" +1 for "original"
          Hide
          Adriano Crestani added a comment -

          Here are some updates for the new query parser:

          • support to set the minimum fuzzy similarity was added to the configuration handler
          • get methods were added to the configuration handler, so the user that is used to the old query parser can easily access the configuration in the old way
          • renamed everything referencing "lucene2" to "original"
          • removed one author tag
          • improved javadoc documentation
          • added a constructor to LuceneQueryParserHelper that accepts an Analyzer as argument, I think Lucene users are used to create a query parser and also pass the analyzer

          That's it

          I have also noticed that when building using "ant build-contrib" it does not copy .properties files to the jar. The new query parser uses a property file to read the NLS messages from and I'm getting some message warnings when running the tests. Is anybody getting the same warnings?

          Show
          Adriano Crestani added a comment - Here are some updates for the new query parser: support to set the minimum fuzzy similarity was added to the configuration handler get methods were added to the configuration handler, so the user that is used to the old query parser can easily access the configuration in the old way renamed everything referencing "lucene2" to "original" removed one author tag improved javadoc documentation added a constructor to LuceneQueryParserHelper that accepts an Analyzer as argument, I think Lucene users are used to create a query parser and also pass the analyzer That's it I have also noticed that when building using "ant build-contrib" it does not copy .properties files to the jar. The new query parser uses a property file to read the NLS messages from and I'm getting some message warnings when running the tests. Is anybody getting the same warnings?
          Hide
          Grant Ingersoll added a comment -

          I need an MD5/SHA1 hash (http://incubator.apache.org/ip-clearance/ip-clearance-template.html) for the exact code listed in the software grant. Also include the version number of the software used to create the hash.

          I see the hash, but not the version number/platform of the tool used to create it. This is the last remaining item on my list. Then I can submit the IP-Clearance to incubator for lazy consensus vote and then this can be committed.

          Show
          Grant Ingersoll added a comment - I need an MD5/SHA1 hash ( http://incubator.apache.org/ip-clearance/ip-clearance-template.html ) for the exact code listed in the software grant. Also include the version number of the software used to create the hash. I see the hash, but not the version number/platform of the tool used to create it. This is the last remaining item on my list. Then I can submit the IP-Clearance to incubator for lazy consensus vote and then this can be committed.
          Hide
          Michael Busch added a comment -

          I see the hash, but not the version number/platform of the tool used to create it.

          md5sum version is included in coreutils 6.10-6ubuntu1 The GNU core utilities
          OS: Ubuntu Jaunty 2.6.28-13-server i686 GNU/Linux

          Show
          Michael Busch added a comment - I see the hash, but not the version number/platform of the tool used to create it. md5sum version is included in coreutils 6.10-6ubuntu1 The GNU core utilities OS: Ubuntu Jaunty 2.6.28-13-server i686 GNU/Linux
          Hide
          Adriano Crestani added a comment -

          This is a mini-tutorial that will help users to switch over to the new query parser. I think it's pretty simple and helpful.

          Just take a look, I will appreciate any suggestion.

          I plan to add it to Lucene wiki after the query parser code is committed.

          Show
          Adriano Crestani added a comment - This is a mini-tutorial that will help users to switch over to the new query parser. I think it's pretty simple and helpful. Just take a look, I will appreciate any suggestion. I plan to add it to Lucene wiki after the query parser code is committed.
          Hide
          Michael McCandless added a comment -

          How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser? In the latest patch I see that these subclasses are deprecated, saying "please use the new flexible queryParser instead", but I think that's not enough, ie don't we need to make corresponding subclasses of the new queryParser for these?

          Show
          Michael McCandless added a comment - How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser? In the latest patch I see that these subclasses are deprecated, saying "please use the new flexible queryParser instead", but I think that's not enough, ie don't we need to make corresponding subclasses of the new queryParser for these?
          Hide
          Michael McCandless added a comment -

          How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser?

          Here's the answer to 1/2 of my question: the current patch already contains a MultiFieldQueryParserWrapper. It too is deprecated, which means in the move to core that we'll do for 3.0, it'll be removed. But the migration path forward seems quite simple: use a MultiFieldQueryNodeProcessor to rewrite all queries so they are against multiple fields (I think?). If that's right, can you update the deprecated javadocs explaining that this is the way forward? In general when we deprecate the old, we need to point the way to the new.

          Also, can you fix all indentation to 2-space in the next patch iteration?

          I'm not sure exactly how, but it'd be great if this new QueryParser had better integration with NumericRangeQuery. But that's a nice-to-have for 2.9.

          Show
          Michael McCandless added a comment - How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser? Here's the answer to 1/2 of my question: the current patch already contains a MultiFieldQueryParserWrapper. It too is deprecated, which means in the move to core that we'll do for 3.0, it'll be removed. But the migration path forward seems quite simple: use a MultiFieldQueryNodeProcessor to rewrite all queries so they are against multiple fields (I think?). If that's right, can you update the deprecated javadocs explaining that this is the way forward? In general when we deprecate the old, we need to point the way to the new. Also, can you fix all indentation to 2-space in the next patch iteration? I'm not sure exactly how, but it'd be great if this new QueryParser had better integration with NumericRangeQuery. But that's a nice-to-have for 2.9.
          Hide
          Grant Ingersoll added a comment -

          OK, I have submitted the IP Clearance to Incubator. It now has 3 days to percolate under a lazy consensus review. After that, assuming it passes, this can be committed.

          Show
          Grant Ingersoll added a comment - OK, I have submitted the IP Clearance to Incubator. It now has 3 days to percolate under a lazy consensus review. After that, assuming it passes, this can be committed.
          Hide
          Adriano Crestani added a comment -

          How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser? In the latest patch I see that these subclasses are deprecated, saying "please use the new flexible queryParser instead", but I think that's not enough, ie don't we need to make corresponding subclasses of the new queryParser for these?

          MultiFieldQueryParser has one equivalent class: MultiFieldQueryParserHelper. Lucene users that use the old one can easily switch to the MultiFieldQueryParserHelper.

          ComplexPhraseQueryParser, this was recently added to Lucene and I did not have time to work on that yet. I started to get more info and check what is the exactly syntax/features it supports, but I'm having some trouble on finding documentation.

          In general when we deprecate the old, we need to point the way to the new.

          Sure, I will work on that and add some javadocs pointing to the new classes.

          Also, can you fix all indentation to 2-space in the next patch iteration?

          Wow, I'm scaried now, do you know an easy and automated way to do that? It will take forever to do manually.

          Show
          Adriano Crestani added a comment - How are we going to migrate the subclasses we have to the current queryParser (ComplexPhraseQueryParser and MultiFieldQueryParser) to the new queryParser? In the latest patch I see that these subclasses are deprecated, saying "please use the new flexible queryParser instead", but I think that's not enough, ie don't we need to make corresponding subclasses of the new queryParser for these? MultiFieldQueryParser has one equivalent class: MultiFieldQueryParserHelper. Lucene users that use the old one can easily switch to the MultiFieldQueryParserHelper. ComplexPhraseQueryParser, this was recently added to Lucene and I did not have time to work on that yet. I started to get more info and check what is the exactly syntax/features it supports, but I'm having some trouble on finding documentation. In general when we deprecate the old, we need to point the way to the new. Sure, I will work on that and add some javadocs pointing to the new classes. Also, can you fix all indentation to 2-space in the next patch iteration? Wow, I'm scaried now, do you know an easy and automated way to do that? It will take forever to do manually.
          Hide
          Michael Busch added a comment -

          Wow, I'm scaried now, do you know an easy and automated way to do that? It will take forever to do manually.

          Just change your eclipse settings (Java Code Style -> Formatter) to 2-space indentation (only whitespaces) and then do Source -> Correct Indentation.

          No reason to be scared!

          Show
          Michael Busch added a comment - Wow, I'm scaried now, do you know an easy and automated way to do that? It will take forever to do manually. Just change your eclipse settings (Java Code Style -> Formatter) to 2-space indentation (only whitespaces) and then do Source -> Correct Indentation. No reason to be scared!
          Hide
          Adriano Crestani added a comment -

          Just change your eclipse settings (Java Code Style -> Formatter) to 2-space indentation (only whitespaces) and then do Source -> Correct Indentation.

          No reason to be scared!

          Thanks Michael, that was helpful

          Some more changes to the query parser patch:

          • Added more info to the @deprecated tags, referencing the new implementation
          • LuceneMultiFieldQueryParserHelper helper was merged with LuceneQueryParserHelper, as well as LuceneMultiFieldQueryConfigHandler was merged with LuceneQueryConfigHandler
          • static/utility methods declared in the old QueryParser and MultiFieldQueryParser are deprecated, so I copied them to a new class called QueryParserUtil
          • lucene.queryParser.original.parser.QueryParser was renamed to TextParser to avoid confusion, this QueryParser class does not do all the query parsing, it's just a part of the query parsing process. Users might also make confusion with the old query parser, which is also called QueryParser
          • fixed the identation (I hope), the patch now contains 2-space identation
          Show
          Adriano Crestani added a comment - Just change your eclipse settings (Java Code Style -> Formatter) to 2-space indentation (only whitespaces) and then do Source -> Correct Indentation. No reason to be scared! Thanks Michael, that was helpful Some more changes to the query parser patch: Added more info to the @deprecated tags, referencing the new implementation LuceneMultiFieldQueryParserHelper helper was merged with LuceneQueryParserHelper, as well as LuceneMultiFieldQueryConfigHandler was merged with LuceneQueryConfigHandler static/utility methods declared in the old QueryParser and MultiFieldQueryParser are deprecated, so I copied them to a new class called QueryParserUtil lucene.queryParser.original.parser.QueryParser was renamed to TextParser to avoid confusion, this QueryParser class does not do all the query parsing, it's just a part of the query parsing process. Users might also make confusion with the old query parser, which is also called QueryParser fixed the identation (I hope), the patch now contains 2-space identation
          Hide
          Michael Busch added a comment -

          The build.xml seems to be missing in the latest patch.

          Show
          Michael Busch added a comment - The build.xml seems to be missing in the latest patch.
          Hide
          Adriano Crestani added a comment -

          Hi Michael,

          I think I excluded by mistake every file (non folder) under contrib/queryparser/. So, the files contrib/queryparser/build.xml and pom.xml.template were not included in patch v8. You can copy them from patch v7, because they were not changed between v7 and v8. Or I can submit a new patch including these 2 files. Let me know what is the best option for you.

          Sorry for the mistake

          Show
          Adriano Crestani added a comment - Hi Michael, I think I excluded by mistake every file (non folder) under contrib/queryparser/. So, the files contrib/queryparser/build.xml and pom.xml.template were not included in patch v8. You can copy them from patch v7, because they were not changed between v7 and v8. Or I can submit a new patch including these 2 files. Let me know what is the best option for you. Sorry for the mistake
          Hide
          Michael Busch added a comment -

          Thanks, Adriano. I'll take the files from the v7 patch.

          I made some changes to make the patch compile with the recent changes on trunk (Attribute changes and constant score rewrite method changes).

          Now some tests are failing for me:

              [junit] Testsuite: org.apache.lucene.messages.TestNLS
              [junit] Tests run: 6, Failures: 2, Errors: 0, Time elapsed: 0.073 sec
              [junit] ------------- Standard Error -----------------
              [junit] WARN: Message with key:Q0005E_MESSAGE_NOT_IN_BUNDLE and locale: en_US not found.
              [junit] ------------- ---------------- ---------------
              [junit] Testcase: testMessageLoading_ja(org.apache.lucene.messages.TestNLS):	FAILED
              [junit] expected:<[?????]: XXX> but was:<[?????]: XXX>
              [junit] junit.framework.ComparisonFailure: expected:<[?????]: XXX> but was:<[?????]: XXX>
              [junit] 	at org.apache.lucene.messages.TestNLS.testMessageLoading_ja(TestNLS.java:36)
              [junit] Testcase: testNLSLoading_ja(org.apache.lucene.messages.TestNLS):	FAILED
              [junit] expected:<[?????????????????????????]> but was:<[?????????????????????????]>
              [junit] junit.framework.ComparisonFailure: expected:<[?????????????????????????]> but was:<[?????????????????????????]>
              [junit] 	at org.apache.lucene.messages.TestNLS.testNLSLoading_ja(TestNLS.java:54)
              [junit] Test org.apache.lucene.messages.TestNLS FAILED
          
          Show
          Michael Busch added a comment - Thanks, Adriano. I'll take the files from the v7 patch. I made some changes to make the patch compile with the recent changes on trunk (Attribute changes and constant score rewrite method changes). Now some tests are failing for me: [junit] Testsuite: org.apache.lucene.messages.TestNLS [junit] Tests run: 6, Failures: 2, Errors: 0, Time elapsed: 0.073 sec [junit] ------------- Standard Error ----------------- [junit] WARN: Message with key:Q0005E_MESSAGE_NOT_IN_BUNDLE and locale: en_US not found. [junit] ------------- ---------------- --------------- [junit] Testcase: testMessageLoading_ja(org.apache.lucene.messages.TestNLS): FAILED [junit] expected:<[?????]: XXX> but was:<[?????]: XXX> [junit] junit.framework.ComparisonFailure: expected:<[?????]: XXX> but was:<[?????]: XXX> [junit] at org.apache.lucene.messages.TestNLS.testMessageLoading_ja(TestNLS.java:36) [junit] Testcase: testNLSLoading_ja(org.apache.lucene.messages.TestNLS): FAILED [junit] expected:<[?????????????????????????]> but was:<[?????????????????????????]> [junit] junit.framework.ComparisonFailure: expected:<[?????????????????????????]> but was:<[?????????????????????????]> [junit] at org.apache.lucene.messages.TestNLS.testNLSLoading_ja(TestNLS.java:54) [junit] Test org.apache.lucene.messages.TestNLS FAILED
          Hide
          Michael Busch added a comment -

          Sorry, my fault. The encoding was not set to UTF-8 in my eclipse project when I applied the patch.
          I changed it to UTF-8, reapplied, and now it works fine.

          Show
          Michael Busch added a comment - Sorry, my fault. The encoding was not set to UTF-8 in my eclipse project when I applied the patch. I changed it to UTF-8, reapplied, and now it works fine.
          Hide
          Michael Busch added a comment -

          Fixed the patch so that it compiles against current trunk:

          • Attributes needed to be changed after LUCENE-1693 was committed
          • Needed to change constantScoreRewrite to RewriteMethod (LUCENE-1644)

          I also added back the build.xml and pom.xml.template from the v7 patch.

          All tests pass now again.

          Show
          Michael Busch added a comment - Fixed the patch so that it compiles against current trunk: Attributes needed to be changed after LUCENE-1693 was committed Needed to change constantScoreRewrite to RewriteMethod ( LUCENE-1644 ) I also added back the build.xml and pom.xml.template from the v7 patch. All tests pass now again.
          Hide
          Michael Busch added a comment -

          OK, I have submitted the IP Clearance to Incubator. It now has 3 days to percolate under a lazy consensus review. After that, assuming it passes, this can be committed.

          Did it pass the review, Grant?

          If yes then I think we can commit this soon to contrib. We can still make small improvements, which will be easier when it's committed. This patch is really large now, and it's hard to track changes.

          Also, even though in 3.0 we can't add new features, we can certainly improve documentation and add more testcases aftert 2.9 and before 3.0. I haven't reviewed everything yet, but if feels like it could use some more documentation...

          Show
          Michael Busch added a comment - OK, I have submitted the IP Clearance to Incubator. It now has 3 days to percolate under a lazy consensus review. After that, assuming it passes, this can be committed. Did it pass the review, Grant? If yes then I think we can commit this soon to contrib. We can still make small improvements, which will be easier when it's committed. This patch is really large now, and it's hard to track changes. Also, even though in 3.0 we can't add new features, we can certainly improve documentation and add more testcases aftert 2.9 and before 3.0. I haven't reviewed everything yet, but if feels like it could use some more documentation...
          Hide
          Michael Busch added a comment -

          Luis/Adriano: I find it a bit confusing now that the different main folders, such as builders, processors, etc. share the same root with 'original', which is an actual implementation.

          Could we change the packaging here? Maybe we could create contrib/queryparser/core and move builders, processors, etc. there?

          Show
          Michael Busch added a comment - Luis/Adriano: I find it a bit confusing now that the different main folders, such as builders, processors, etc. share the same root with 'original', which is an actual implementation. Could we change the packaging here? Maybe we could create contrib/queryparser/core and move builders, processors, etc. there?
          Hide
          Luis Alves added a comment -

          Hi Michael,

          Thanks for updating the patch to work with the latest code in svn.

          I like your suggestion about creating a core package,
          I'll re-factor the code to use the core package.

          Show
          Luis Alves added a comment - Hi Michael, Thanks for updating the patch to work with the latest code in svn. I like your suggestion about creating a core package, I'll re-factor the code to use the core package.
          Hide
          Grant Ingersoll added a comment -

          Did it pass the review, Grant?

          Yes.

          Show
          Grant Ingersoll added a comment - Did it pass the review, Grant? Yes.
          Hide
          Luis Alves added a comment -

          The latest patch include:

          • created core package, fixed javadocs for that.
          • fixed TextParser.jj - I assumed Adriano's re-factored this class without generating the code using javacc, the generated classes where not correct.
          • remove all javadoc references to QueryParser and QueryParserWrapper from javadocs, since these will be deprecated and removed on v3.0. I change it to use the LuceneQueryconfigHandler class. (probably more clean up is necessary on the javadocs for this part)
          • created a overview.html inside the src folder, to remove another javadoc warning. It's empty for now.
          • This patch includes, Michael changes.
          Show
          Luis Alves added a comment - The latest patch include: created core package, fixed javadocs for that. fixed TextParser.jj - I assumed Adriano's re-factored this class without generating the code using javacc, the generated classes where not correct. remove all javadoc references to QueryParser and QueryParserWrapper from javadocs, since these will be deprecated and removed on v3.0. I change it to use the LuceneQueryconfigHandler class. (probably more clean up is necessary on the javadocs for this part) created a overview.html inside the src folder, to remove another javadoc warning. It's empty for now. This patch includes, Michael changes.
          Hide
          Adriano Crestani added a comment -

          fixed TextParser.jj - I assumed Adriano's re-factored this class without generating the code using javacc, the generated classes where not correct.

          Hi Luis,

          What exactly did I forget to do? It's hard to find out differences between the patch versions while the code is not commited

          I'm not sure, I generated a new TextParser.java after I changed the .jj, it was working and tests passing.

          Could we change the packaging here? Maybe we could create contrib/queryparser/core and move builders, processors, etc. there?

          That's fine, I think this way the code will look clearer and less confusing.

          Show
          Adriano Crestani added a comment - fixed TextParser.jj - I assumed Adriano's re-factored this class without generating the code using javacc, the generated classes where not correct. Hi Luis, What exactly did I forget to do? It's hard to find out differences between the patch versions while the code is not commited I'm not sure, I generated a new TextParser.java after I changed the .jj, it was working and tests passing. Could we change the packaging here? Maybe we could create contrib/queryparser/core and move builders, processors, etc. there? That's fine, I think this way the code will look clearer and less confusing.
          Hide
          Luis Alves added a comment - - edited

          Hi Adriano,

          You just forgot to delete the query parser files generated by javacc,
          and regenerate the code from the TextParser.jj using javacc.

          This created 2 new classes called TextParserConstants and TextParserTokenManager,
          but your re-factored code was still using the QueryParser named classes.

          Not a big deal. We should always use the generated code from javacc,
          and avoid editing those generated files, this will prevent inconsistencies.

          I'll try to add a javacc target to the queryparser build file, in the near future to make that easier

          Show
          Luis Alves added a comment - - edited Hi Adriano, You just forgot to delete the query parser files generated by javacc, and regenerate the code from the TextParser.jj using javacc. This created 2 new classes called TextParserConstants and TextParserTokenManager, but your re-factored code was still using the QueryParser named classes. Not a big deal. We should always use the generated code from javacc, and avoid editing those generated files, this will prevent inconsistencies. I'll try to add a javacc target to the queryparser build file, in the near future to make that easier
          Hide
          Adriano Crestani added a comment -

          Hi Michael,

          I noticed you renamed LuceneQueryConfigHandler.setConstantScoreRewrite to setMultiTermRewriteMethod. Shouldn't ConstantScoreRewriteAttribute class (and its impl) be renamed too?

          Show
          Adriano Crestani added a comment - Hi Michael, I noticed you renamed LuceneQueryConfigHandler.setConstantScoreRewrite to setMultiTermRewriteMethod. Shouldn't ConstantScoreRewriteAttribute class (and its impl) be renamed too?
          Hide
          Adriano Crestani added a comment -

          You just forgot to delete the query parser files generated by javacc,
          and regenerate the code from the TextParser.jj using javacc.

          This created 2 new classes called TextParserConstants and TextParserTokenManager,
          but your re-factored code was still using the QueryParser named classes.

          Not a big deal. We should always use the generated code from javacc,
          and avoid editing those generated files, this will prevent inconsistencies.

          I see now. I just regenerated the code and replaced the main QueryParser.java by TextParser.java. Sorry for the mistake.

          I'll try to add a javacc target to the queryparser build file, in the near future to make that easier

          I would love that!

          Show
          Adriano Crestani added a comment - You just forgot to delete the query parser files generated by javacc, and regenerate the code from the TextParser.jj using javacc. This created 2 new classes called TextParserConstants and TextParserTokenManager, but your re-factored code was still using the QueryParser named classes. Not a big deal. We should always use the generated code from javacc, and avoid editing those generated files, this will prevent inconsistencies. I see now. I just regenerated the code and replaced the main QueryParser.java by TextParser.java. Sorry for the mistake. I'll try to add a javacc target to the queryparser build file, in the near future to make that easier I would love that!
          Hide
          Uwe Schindler added a comment -

          I'll try to add a javacc target to the queryparser build file, in the near future to make that easier

          The main Lucene build.xml/common-build.xml has support for this, maybe it is just using an already available target/macro.

          Show
          Uwe Schindler added a comment - I'll try to add a javacc target to the queryparser build file, in the near future to make that easier The main Lucene build.xml/common-build.xml has support for this, maybe it is just using an already available target/macro.
          Hide
          Luis Alves added a comment -

          The main Lucene build.xml/common-build.xml has support for this, maybe it is just using an already available target/macro.

          Thanks, I'll take a look and see if I can reuse it.

          Show
          Luis Alves added a comment - The main Lucene build.xml/common-build.xml has support for this, maybe it is just using an already available target/macro. Thanks, I'll take a look and see if I can reuse it.
          Hide
          Michael Busch added a comment -

          I noticed you renamed LuceneQueryConfigHandler.setConstantScoreRewrite to setMultiTermRewriteMethod. Shouldn't ConstantScoreRewriteAttribute class (and its impl) be renamed too?

          Oh yeah of course! I meant to do that, but then forgot...

          Are you going to submit a new version of the patch anyway? If yes, can you make this change?
          If no I can submit a new patch...

          Show
          Michael Busch added a comment - I noticed you renamed LuceneQueryConfigHandler.setConstantScoreRewrite to setMultiTermRewriteMethod. Shouldn't ConstantScoreRewriteAttribute class (and its impl) be renamed too? Oh yeah of course! I meant to do that, but then forgot... Are you going to submit a new version of the patch anyway? If yes, can you make this change? If no I can submit a new patch...
          Hide
          Adriano Crestani added a comment -
          • Just revised patch and fixed every javadocs referencing *QueryParserWrapper
          • Fixed javadoc warnings
          • Fixed javadocs (text)
          • Added getAnalyzer() to LuceneQueryParserHandler
          Show
          Adriano Crestani added a comment - Just revised patch and fixed every javadocs referencing *QueryParserWrapper Fixed javadoc warnings Fixed javadocs (text) Added getAnalyzer() to LuceneQueryParserHandler
          Hide
          Adriano Crestani added a comment -

          Are you going to submit a new version of the patch anyway? If yes, can you make this change?
          If no I can submit a new patch...

          Sorry Michael, by one minute, I did not see your comment. I will submit another patch tomorrow with these changes.

          Show
          Adriano Crestani added a comment - Are you going to submit a new version of the patch anyway? If yes, can you make this change? If no I can submit a new patch... Sorry Michael, by one minute, I did not see your comment. I will submit another patch tomorrow with these changes.
          Hide
          Michael Busch added a comment -

          I will submit another patch tomorrow with these changes.

          Sounds good. Could you also please fix the javadocs? When I'm building the javadocs I'm getting a lot of warnings about not found references.

          Otherwise I think this is ready to commit soon.

          Show
          Michael Busch added a comment - I will submit another patch tomorrow with these changes. Sounds good. Could you also please fix the javadocs? When I'm building the javadocs I'm getting a lot of warnings about not found references. Otherwise I think this is ready to commit soon.
          Hide
          Uwe Schindler added a comment -

          Just a question: Will it be possible to specify some type of "schema" for the query parser in future, to automatically create NumericRangeQuery for different numeric types? It would then be possible to index a numeric value (double,float,long,int) using NumericField and then the query parser knows, which type of field this is and so it correctly creates a NumericRangeQuery for strings like "[1.567..*]" or "(1.787..19.5]". NumericRangeQuery also supports the rewrite modes, only some type of schema support is missing.

          I ask this, because someone asked on java-user for such a feature in query parser.

          Show
          Uwe Schindler added a comment - Just a question: Will it be possible to specify some type of "schema" for the query parser in future, to automatically create NumericRangeQuery for different numeric types? It would then be possible to index a numeric value (double,float,long,int) using NumericField and then the query parser knows, which type of field this is and so it correctly creates a NumericRangeQuery for strings like " [1.567..*] " or "(1.787..19.5]". NumericRangeQuery also supports the rewrite modes, only some type of schema support is missing. I ask this, because someone asked on java-user for such a feature in query parser.
          Hide
          Michael Busch added a comment -

          Could you also please fix the javadocs? When I'm building the javadocs I'm getting a lot of warnings about not found references.

          The warnings occur because you put links to the new contrib queryparser into the core queryparser. That doesn't work as the contribs are not in the classpath of the core, so I think we should remove those links and change them just to plain text.

          Also, please make sure to add to the main build.xml appropriate entries for the javadocs, otherwise the "All" javadocs will not contain the contrib QP classes.

          There are also some TODOs in the docs; especially in top-level places, such as the package.html of your new package, we should not have TODOs in the docs. Please fix that soon, 2.9 is coming quickly.

          Show
          Michael Busch added a comment - Could you also please fix the javadocs? When I'm building the javadocs I'm getting a lot of warnings about not found references. The warnings occur because you put links to the new contrib queryparser into the core queryparser. That doesn't work as the contribs are not in the classpath of the core, so I think we should remove those links and change them just to plain text. Also, please make sure to add to the main build.xml appropriate entries for the javadocs, otherwise the "All" javadocs will not contain the contrib QP classes. There are also some TODOs in the docs; especially in top-level places, such as the package.html of your new package, we should not have TODOs in the docs. Please fix that soon, 2.9 is coming quickly.
          Hide
          Luis Alves added a comment -

          Hi Uwe,

          Will it be possible to specify some type of "schema" for the query parser in future, to automatically create NumericRangeQuery for different numeric types? It would then be possible to index a numeric value (double,float,long,int) using NumericField and then the query parser knows, which type of field this is and so it correctly creates a NumericRangeQuery for strings like "[1.567..*]" or "(1.787..19.5]". NumericRangeQuery also supports the rewrite modes, only some type of schema support is missing.

          I think this is doable.
          I don't think there is a way to extract if a field is numeric from the index, so
          the user will have to configure the FieldConfig objects in the ConfigHandler.
          But if this is done, it will not be that difficult to implement the rest.

          Can you create a new "jira issue" with the description of the feature,
          so we can discuss the details there.
          I'll try to implement that once we agree on all the details.

          Show
          Luis Alves added a comment - Hi Uwe, Will it be possible to specify some type of "schema" for the query parser in future, to automatically create NumericRangeQuery for different numeric types? It would then be possible to index a numeric value (double,float,long,int) using NumericField and then the query parser knows, which type of field this is and so it correctly creates a NumericRangeQuery for strings like " [1.567..*] " or "(1.787..19.5]". NumericRangeQuery also supports the rewrite modes, only some type of schema support is missing. I think this is doable. I don't think there is a way to extract if a field is numeric from the index, so the user will have to configure the FieldConfig objects in the ConfigHandler. But if this is done, it will not be that difficult to implement the rest. Can you create a new "jira issue" with the description of the feature, so we can discuss the details there. I'll try to implement that once we agree on all the details.
          Hide
          Adriano Crestani added a comment -

          The warnings occur because you put links to the new contrib queryparser into the core queryparser. That doesn't work as the contribs are not in the classpath of the core, so I think we should remove those links and change them just to plain text.

          Also, please make sure to add to the main build.xml appropriate entries for the javadocs, otherwise the "All" javadocs will not contain the contrib QP classes.

          There are also some TODOs in the docs; especially in top-level places, such as the package.html of your new package, we should not have TODOs in the docs. Please fix that soon, 2.9 is coming quickly.

          Done!

          I also fixed and added a some other javadocs that were missing and renamed ConstantScoreRewriteAttribute (and its impl) to MultiTermRewriteMethodAttribute.

          I think the only thing remaining is to add a package.html to org.apache.queryParser.messages package with a good description about it. Luis has a good knowledge about this package, if you have time, can you add this file to that package? Thanks

          Show
          Adriano Crestani added a comment - The warnings occur because you put links to the new contrib queryparser into the core queryparser. That doesn't work as the contribs are not in the classpath of the core, so I think we should remove those links and change them just to plain text. Also, please make sure to add to the main build.xml appropriate entries for the javadocs, otherwise the "All" javadocs will not contain the contrib QP classes. There are also some TODOs in the docs; especially in top-level places, such as the package.html of your new package, we should not have TODOs in the docs. Please fix that soon, 2.9 is coming quickly. Done! I also fixed and added a some other javadocs that were missing and renamed ConstantScoreRewriteAttribute (and its impl) to MultiTermRewriteMethodAttribute. I think the only thing remaining is to add a package.html to org.apache.queryParser.messages package with a good description about it. Luis has a good knowledge about this package, if you have time, can you add this file to that package? Thanks
          Hide
          Luis Alves added a comment -

          Hi Adriano

          There is something wrong with your patch v11, the size almost doubled.
          I did a diff with v10 and it looks like all the files show up twice on v11 patch.
          Can you resubmit it again, I'll add the docs for messages after you resubmit it.

          Show
          Luis Alves added a comment - Hi Adriano There is something wrong with your patch v11, the size almost doubled. I did a diff with v10 and it looks like all the files show up twice on v11 patch. Can you resubmit it again, I'll add the docs for messages after you resubmit it.
          Hide
          Adriano Crestani added a comment -

          There is something wrong with your patch v11, the size almost doubled.
          I did a diff with v10 and it looks like all the files show up twice on v11 patch.
          Can you resubmit it again, I'll add the docs for messages after you resubmit it.

          Hi Luis,

          Yes, you are right, I think I know why it happened, my mistake. Thanks for reporting that. Here is the new patch

          Show
          Adriano Crestani added a comment - There is something wrong with your patch v11, the size almost doubled. I did a diff with v10 and it looks like all the files show up twice on v11 patch. Can you resubmit it again, I'll add the docs for messages after you resubmit it. Hi Luis, Yes, you are right, I think I know why it happened, my mistake. Thanks for reporting that. Here is the new patch
          Hide
          Uwe Schindler added a comment -

          Can you create a new "jira issue" with the description of the feature,
          so we can discuss the details there.
          I'll try to implement that once we agree on all the details.

          Will do! Thanks.

          Show
          Uwe Schindler added a comment - Can you create a new "jira issue" with the description of the feature, so we can discuss the details there. I'll try to implement that once we agree on all the details. Will do! Thanks.
          Hide
          Michael Busch added a comment -

          Good work!

          All tests (core, contrib, bw-comp) pass, the warnings are gone, and the "All" javadocs section contains the new packages now too.

          I will commit this in a couple of days if nobody objects.

          If there are small outstanding javadoc improvements or so, we should open a separate JIRA issue... this patch is so big that changes are hard to track.

          Show
          Michael Busch added a comment - Good work! All tests (core, contrib, bw-comp) pass, the warnings are gone, and the "All" javadocs section contains the new packages now too. I will commit this in a couple of days if nobody objects. If there are small outstanding javadoc improvements or so, we should open a separate JIRA issue... this patch is so big that changes are hard to track.
          Hide
          Luis Alves added a comment -

          Michael,

          I will submit a new patch in a few hours, I just need to finished the testing.

          It includes:

          • javadocs for the messages package
          • rename all classes that started with Lucene to Original.
          • refactor the OriginalConfigHandler and OriginalQueryParserHelper
          • remove the testcases used the ConfigHandler directly, since it was a duplication of the ones that use the QPHelper.
          • new FieldBostMapFCListener, to handle boost maps in a cleaner way
          • rename method in FieldConfigListener to buildFieldConfig
          • new DateResolutionFCListener to handle dateresolution in a cleaner way
          • A fix to a problem I found in BoostQueryNodeProcessor
          Show
          Luis Alves added a comment - Michael, I will submit a new patch in a few hours, I just need to finished the testing. It includes: javadocs for the messages package rename all classes that started with Lucene to Original. refactor the OriginalConfigHandler and OriginalQueryParserHelper remove the testcases used the ConfigHandler directly, since it was a duplication of the ones that use the QPHelper. new FieldBostMapFCListener, to handle boost maps in a cleaner way rename method in FieldConfigListener to buildFieldConfig new DateResolutionFCListener to handle dateresolution in a cleaner way A fix to a problem I found in BoostQueryNodeProcessor
          Hide
          Luis Alves added a comment -

          It includes:

          • javadocs for the messages package
          • rename all classes that started with Lucene to Original.
          • refactor the OriginalConfigHandler and OriginalQueryParserHelper
          • remove the testcases using the ConfigHandler directly, since it was a duplication of the ones that use the QPHelper.
          • new FieldBostMapFCListener, to handle boost maps in a cleaner way
          • rename method in FieldConfigListener to buildFieldConfig
          • new DateResolutionFCListener to handle dateresolution in a cleaner way
          • A fix to a problem I found in BoostQueryNodeProcessor
          • rename Parser to SyntaxParser
          • rename TextParser to OriginalSyntaxParser

          I think this is all I changed, but it might have some other minor changes.

          Show
          Luis Alves added a comment - It includes: javadocs for the messages package rename all classes that started with Lucene to Original. refactor the OriginalConfigHandler and OriginalQueryParserHelper remove the testcases using the ConfigHandler directly, since it was a duplication of the ones that use the QPHelper. new FieldBostMapFCListener, to handle boost maps in a cleaner way rename method in FieldConfigListener to buildFieldConfig new DateResolutionFCListener to handle dateresolution in a cleaner way A fix to a problem I found in BoostQueryNodeProcessor rename Parser to SyntaxParser rename TextParser to OriginalSyntaxParser I think this is all I changed, but it might have some other minor changes.
          Hide
          Luis Alves added a comment -

          Hi Michael,

          I'm done with my changes, let me know if you find any wrong with the new patch.

          Show
          Luis Alves added a comment - Hi Michael, I'm done with my changes, let me know if you find any wrong with the new patch.
          Hide
          Luis Alves added a comment - - edited

          I created a block dependency on LUCENE-1486.
          The "new flexible query parser" blocks LUCENE-1486.

          Show
          Luis Alves added a comment - - edited I created a block dependency on LUCENE-1486 . The "new flexible query parser" blocks LUCENE-1486 .
          Hide
          Michael Busch added a comment -

          Depend on the "new flexible query parser" work.

          OK, didn't know there was another patch coming.... I guess I'll redo my verification then...

          Show
          Michael Busch added a comment - Depend on the "new flexible query parser" work. OK, didn't know there was another patch coming.... I guess I'll redo my verification then...
          Hide
          Luis Alves added a comment - - edited

          Hi Michael,

          OK, didn't know there was another patch coming.... I guess I'll redo my verification then...

          I added that comment when I created a block dependency on LUCENE-1486.

          I'm still learning JIRA .

          I didn't know the comment was going to get posted in this thread,
          I was assuming LUCENE-1486 would get the comment.

          Show
          Luis Alves added a comment - - edited Hi Michael, OK, didn't know there was another patch coming.... I guess I'll redo my verification then... I added that comment when I created a block dependency on LUCENE-1486 . I'm still learning JIRA . I didn't know the comment was going to get posted in this thread, I was assuming LUCENE-1486 would get the comment.
          Hide
          Michael Busch added a comment -

          No worries.

          I just tested the latest patch. All tests (core, contrib, tag) pass and javadocs look good.

          I'll commit this in a day or two!

          Show
          Michael Busch added a comment - No worries. I just tested the latest patch. All tests (core, contrib, tag) pass and javadocs look good. I'll commit this in a day or two!
          Hide
          Michael Busch added a comment -

          Committed revision 800191.

          Thank you very much, Adriano and Luis, for all your hard work!

          Show
          Michael Busch added a comment - Committed revision 800191. Thank you very much, Adriano and Luis, for all your hard work!

            People

            • Assignee:
              Michael Busch
              Reporter:
              Luis Alves
            • Votes:
              3 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development