Details

    • Lucene Fields:
      New, Patch Available

      Description

      For my needs I've updated Lucene so that it uses Java 5 constructs. I know Java 5 migration had been planned for 2.1 someday in the past, but don't know when it is planned now. This patch against the trunk includes :

      • most obvious generics usage (there are tons of usages of sets, ... Those which are commonly used have been generified)
      • PriorityQueue generification
      • replacement of indexed for loops with for each constructs
      • removal of unnececessary unboxing

      The code is to my opinion much more readable with those features (you actually know what is stored in collections reading the code, without the need to lookup for field definitions everytime) and it simplifies many algorithms.

      Note that this patch also includes an interface for the Query class. This has been done for my company's needs for building custom Query classes which add some behaviour to the base Lucene queries. It prevents multiple unnnecessary casts. I know this introduction is not wanted by the team, but it really makes our developments easier to maintain. If you don't want to use this, replace all /Queriable/ calls with standard /Query/.

      1. instantiated_fieldable.patch
        4 kB
        Karl Wettin
      2. LUCENE-1257_analysis.patch
        8 kB
        Robert Muir
      3. LUCENE-1257_BooleanFilter_Generics.patch
        4 kB
        Karthik K
      4. LUCENE-1257_contrib_ant.patch
        2 kB
        Karthik K
      5. LUCENE-1257_contrib_benchmark_2.patch
        22 kB
        Karthik K
      6. LUCENE-1257_contrib_benchmark.patch
        56 kB
        Karthik K
      7. LUCENE-1257_contrib_highlighting.patch
        26 kB
        Karthik K
      8. LUCENE-1257_contrib_memory.patch
        26 kB
        Karthik K
      9. LUCENE-1257_contrib_misc.patch
        18 kB
        Karthik K
      10. LUCENE-1257_contrib_smartcn.patch
        15 kB
        Robert Muir
      11. LUCENE-1257_heavy.patch
        69 kB
        Uwe Schindler
      12. LUCENE-1257_heavy.patch
        59 kB
        Robert Muir
      13. LUCENE-1257_javacc_upgrade.patch
        0.8 kB
        Karthik K
      14. LUCENE-1257_lucil.patch
        11 kB
        Simon Willnauer
      15. LUCENE-1257_lucli.patch
        8 kB
        Karthik K
      16. LUCENE-1257_messages.patch
        19 kB
        Robert Muir
      17. LUCENE-1257_more_unnecessary_casts.patch
        3 kB
        Karthik K
      18. LUCENE-1257_MultiFieldQueryParser.patch
        3 kB
        Karthik K
      19. LUCENE-1257_o_a_l_demo.patch
        1 kB
        Karthik K
      20. LUCENE-1257_o_a_l_index_test.patch
        55 kB
        Karthik K
      21. LUCENE-1257_o_a_l_index_test.patch
        55 kB
        Karthik K
      22. LUCENE-1257_o_a_l_search_spans.patch
        16 kB
        Karthik K
      23. LUCENE-1257_o_a_l_search.patch
        28 kB
        Karthik K
      24. LUCENE-1257_o.a.l.queryParser.patch
        8 kB
        Karthik K
      25. LUCENE-1257_o.a.l.store.patch
        12 kB
        Karthik K
      26. LUCENE-1257_org_apache_lucene_index.patch
        59 kB
        Uwe Schindler
      27. LUCENE-1257_org_apache_lucene_index.patch
        58 kB
        Karthik K
      28. LUCENE-1257_precendence_parser.patch
        9 kB
        Karthik K
      29. LUCENE-1257_queryParser_jj.patch
        6 kB
        Karthik K
      30. LUCENE-1257_swing_wikipedia_wordnet_xmlqp.patch
        24 kB
        Robert Muir
      31. LUCENE-1257_unnecessary_casts.patch
        21 kB
        Karthik K
      32. LUCENE-1257_unnnecessary_casts_2.patch
        22 kB
        Karthik K
      33. LUCENE-1257-BooleanQuery.patch
        5 kB
        Karthik K
      34. LUCENE-1257-BooleanScorer_2.patch
        4 kB
        Karthik K
      35. LUCENE-1257-BufferedDeletes_DocumentsWriter.patch
        5 kB
        Karthik K
      36. LUCENE-1257-CheckIndex.patch
        6 kB
        Karthik K
      37. LUCENE-1257-CloseableThreadLocal.patch
        4 kB
        Karthik K
      38. LUCENE-1257-CompoundFileReaderWriter.patch
        3 kB
        Karthik K
      39. LUCENE-1257-ConcurrentMergeScheduler.patch
        2 kB
        Karthik K
      40. LUCENE-1257-DirectoryReader.patch
        8 kB
        Karthik K
      41. LUCENE-1257-DisjunctionMaxQuery-more_type_safety.patch
        2 kB
        Karthik K
      42. LUCENE-1257-DocFieldProcessorPerThread.patch
        4 kB
        Karthik K
      43. LUCENE-1257-Document.patch
        5 kB
        Uwe Schindler
      44. LUCENE-1257-FieldCacheImpl.patch
        8 kB
        Karthik K
      45. LUCENE-1257-FieldCacheRangeFilter.patch
        16 kB
        Uwe Schindler
      46. LUCENE-1257-IndexDeleter.patch
        13 kB
        Karthik K
      47. LUCENE-1257-IndexDeletionPolicy_IndexFileDeleter.patch
        5 kB
        Karthik K
      48. LUCENE-1257-iw.patch
        19 kB
        Karthik K
      49. LUCENE-1257-MTQWF.patch
        5 kB
        Uwe Schindler
      50. LUCENE-1257-NormalizeCharMap.patch
        1 kB
        Karthik K
      51. LUCENE-1257-o.a.l.util.patch
        10 kB
        Karthik K
      52. LUCENE-1257-org_apache_lucene_document.patch
        29 kB
        Karthik K
      53. LUCENE-1257-org_apache_lucene_document.patch
        29 kB
        Uwe Schindler
      54. LUCENE-1257-org_apache_lucene_document.patch
        24 kB
        Karthik K
      55. LUCENE-1257-SegmentInfos.patch
        6 kB
        Karthik K
      56. LUCENE-1257-StringBuffer.patch
        101 kB
        Uwe Schindler
      57. LUCENE-1257-StringBuffer.patch
        100 kB
        Uwe Schindler
      58. LUCENE-1257-StringBuffer.patch
        134 kB
        Uwe Schindler
      59. lucene1257surround1.patch
        7 kB
        Uwe Schindler
      60. lucene1257surround1.patch
        6 kB
        Paul Elschot
      61. LUCENE-1257-TopDocsCollector.patch
        8 kB
        Karthik K
      62. LUCENE-1257-WordListLoader.patch
        3 kB
        Karthik K
      63. shinglematrixfilter_generified.patch
        12 kB
        Karl Wettin

        Issue Links

          Activity

          Hide
          Cédric Champeau added a comment -

          Patch against the trunk

          Show
          Cédric Champeau added a comment - Patch against the trunk
          Hide
          Uwe Schindler added a comment -

          This patch is bit outdated.

          I will try to fix a lot of these things. Simple are:

          • remove new NumberSubType(value) and replace by value (autoboxing), which is faster, because allocation overhead removed
          • replace StringBuffer by StringBuilder globally (it is never used in more than one thread)

          Addition of generics is little more work, I started in other issues (NumericRangeQuery, AttributeSource).

          Show
          Uwe Schindler added a comment - This patch is bit outdated. I will try to fix a lot of these things. Simple are: remove new NumberSubType(value) and replace by value (autoboxing), which is faster, because allocation overhead removed replace StringBuffer by StringBuilder globally (it is never used in more than one thread) Addition of generics is little more work, I started in other issues (NumericRangeQuery, AttributeSource).
          Hide
          Uwe Schindler added a comment -

          A first patch that replaces StringBuffer -> StringBuilder almost everywhere (created using find/grep/sed). Some classes were left out (auto-generated classes by JavaCC).

          There are still some special cases in contrib: some public/protected methods use StringBuffer as param/return type. These should be reverted - or break backwards in 3.0? Any comments?

          Show
          Uwe Schindler added a comment - A first patch that replaces StringBuffer -> StringBuilder almost everywhere (created using find/grep/sed). Some classes were left out (auto-generated classes by JavaCC). There are still some special cases in contrib: some public/protected methods use StringBuffer as param/return type. These should be reverted - or break backwards in 3.0? Any comments?
          Hide
          Uwe Schindler added a comment -

          Updated patch. It removes some replacements in snowball and other contribs, where StringBuffer appears in public API.

          All other occurences should be correctly replaced (mostly toString() impls).

          I will commit this soon, too, as the patch may get outdated very fast.

          Show
          Uwe Schindler added a comment - Updated patch. It removes some replacements in snowball and other contribs, where StringBuffer appears in public API. All other occurences should be correctly replaced (mostly toString() impls). I will commit this soon, too, as the patch may get outdated very fast.
          Hide
          Uwe Schindler added a comment -

          Small fix in highlighter public API.

          Show
          Uwe Schindler added a comment - Small fix in highlighter public API.
          Hide
          Uwe Schindler added a comment - - edited

          StringBuffer changes committed revision: 821185

          I hope nobody has merge problems now... I keep this issue open as umbrella for further Java 1.5 changes.

          Show
          Uwe Schindler added a comment - - edited StringBuffer changes committed revision: 821185 I hope nobody has merge problems now... I keep this issue open as umbrella for further Java 1.5 changes.
          Hide
          Mark Miller added a comment -

          There are still some special cases in contrib: some public/protected methods use StringBuffer as param/return type. These should be reverted - or break backwards in 3.0? Any comments?

          I vote to move to StringBuilder anyway if its in Contrib. Though probably not with Snowball, since we don't really write/maintain that code.

          Show
          Mark Miller added a comment - There are still some special cases in contrib: some public/protected methods use StringBuffer as param/return type. These should be reverted - or break backwards in 3.0? Any comments? I vote to move to StringBuilder anyway if its in Contrib. Though probably not with Snowball, since we don't really write/maintain that code.
          Hide
          Uwe Schindler added a comment -

          I already committed tha non-public-API changes. Maybe we add the other StringBuilders in contrib on separate issues. For highlighter I modified the public API of TextFragment to use CharSequence and deprecated the StringBuffer ctor. Maybe we can do this like this elsewhere, too.

          Show
          Uwe Schindler added a comment - I already committed tha non-public-API changes. Maybe we add the other StringBuilders in contrib on separate issues. For highlighter I modified the public API of TextFragment to use CharSequence and deprecated the StringBuffer ctor. Maybe we can do this like this elsewhere, too.
          Hide
          Karl Wettin added a comment -

          I vote to move to StringBuilder anyway if its in Contrib. Though probably not with Snowball, since we don't really write/maintain that code.

          Actually I patched the Snowball stemmer code to get ridth of the use of reflection. So what we use is an altered version of their code. I tried to get Dr Porter to commit those changes for years but it's still the same. Based on this I think we could just keep going with our own stuff in there as long we keep a record of what we have done in case we want to merge with their trunk.

          Show
          Karl Wettin added a comment - I vote to move to StringBuilder anyway if its in Contrib. Though probably not with Snowball, since we don't really write/maintain that code. Actually I patched the Snowball stemmer code to get ridth of the use of reflection. So what we use is an altered version of their code. I tried to get Dr Porter to commit those changes for years but it's still the same. Based on this I think we could just keep going with our own stuff in there as long we keep a record of what we have done in case we want to merge with their trunk.
          Hide
          Uwe Schindler added a comment - - edited

          Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>.

          Show
          Uwe Schindler added a comment - - edited Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>.
          Hide
          Mark Miller added a comment - - edited

          Actually I patched the Snowball stemmer code to get ridth of the use of reflection. So what we use is an altered version of their code. I tried to get Dr Porter to commit those changes for years but it's still the same. Based on this I think we could just keep going with our own stuff in there as long we keep a record of what we have done in case we want to merge with their trunk.

          Okay - lets do it then. Testing the English version, its actually almost twice as fast with a StringBuilder.

          I bet it would be almost twice as fast or better again if we could get rid of that nasty reflection ...

          edit

          Wait ... do you mean you got rid of some of the reflection or did we lose your changes? I'm seeing some nasty slow reflection in there still ...

          edit

          err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ...

          Show
          Mark Miller added a comment - - edited Actually I patched the Snowball stemmer code to get ridth of the use of reflection. So what we use is an altered version of their code. I tried to get Dr Porter to commit those changes for years but it's still the same. Based on this I think we could just keep going with our own stuff in there as long we keep a record of what we have done in case we want to merge with their trunk. Okay - lets do it then. Testing the English version, its actually almost twice as fast with a StringBuilder. I bet it would be almost twice as fast or better again if we could get rid of that nasty reflection ... edit Wait ... do you mean you got rid of some of the reflection or did we lose your changes? I'm seeing some nasty slow reflection in there still ... edit err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ...
          Hide
          Uwe Schindler added a comment -

          Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>.

          Committed revision: 821277

          Show
          Uwe Schindler added a comment - Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>. Committed revision: 821277
          Hide
          Timo Nentwig added a comment -

          Somewhat off-topic but in the course of porting to Java 5 I would suggest IndexWriter etc. to implement java.io.Closeable.

          Show
          Timo Nentwig added a comment - Somewhat off-topic but in the course of porting to Java 5 I would suggest IndexWriter etc. to implement java.io.Closeable.
          Hide
          Uwe Schindler added a comment -

          Good idea!

          Show
          Uwe Schindler added a comment - Good idea!
          Hide
          Paul Elschot added a comment -

          For the record, there is still some usage of StringBuffer in contrib/surround. Fortunately not in public API, only internal, and in some protected method parameters.

          Show
          Paul Elschot added a comment - For the record, there is still some usage of StringBuffer in contrib/surround. Fortunately not in public API, only internal, and in some protected method parameters.
          Hide
          Uwe Schindler added a comment -

          I know, protected is also public from the API side (one could subclass and use one of the protected methods), because of that I left it out for the beginning. This is why you should always think about making as most as possible (package-)private.

          Show
          Uwe Schindler added a comment - I know, protected is also public from the API side (one could subclass and use one of the protected methods), because of that I left it out for the beginning. This is why you should always think about making as most as possible (package-)private.
          Hide
          Paul Elschot added a comment -

          StringBuffer to StringBuilder patch for conrib/surround

          Show
          Paul Elschot added a comment - StringBuffer to StringBuilder patch for conrib/surround
          Hide
          Paul Elschot added a comment -

          I would not expect that the protected methods in contrib/surround that are affected by this patch are actually overridden by other code. So formally this is an API change, but I don't think it is worth mentioning in other places than here.

          Show
          Paul Elschot added a comment - I would not expect that the protected methods in contrib/surround that are affected by this patch are actually overridden by other code. So formally this is an API change, but I don't think it is worth mentioning in other places than here.
          Hide
          Uwe Schindler added a comment -

          Lets add a changes.txt entry in contrib about a BW break.

          Show
          Uwe Schindler added a comment - Lets add a changes.txt entry in contrib about a BW break.
          Hide
          Uwe Schindler added a comment -

          patch with changes.txt

          Show
          Uwe Schindler added a comment - patch with changes.txt
          Hide
          Karl Wettin added a comment -

          Wait ... do you mean you got rid of some of the reflection or did we lose your changes? I'm seeing some nasty slow reflection in there still ...

          My changes was to the abstract Snowball stemmer class. I simply added an abstract method and got rid of the reflection in the Lucene filter.

          One could argue that we should update the Snowball compiler rather than updating the Java code it renders. But honestly I think we should just update the rendered code and then report any improvement found to the Snowball ml and keep track of it in the package readme.

          err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ...

          It could still be rather expensive per stem at query time. I vote for getting rid of it if we can. I'll throw an eye at it.

          Show
          Karl Wettin added a comment - Wait ... do you mean you got rid of some of the reflection or did we lose your changes? I'm seeing some nasty slow reflection in there still ... My changes was to the abstract Snowball stemmer class. I simply added an abstract method and got rid of the reflection in the Lucene filter. One could argue that we should update the Snowball compiler rather than updating the Java code it renders. But honestly I think we should just update the rendered code and then report any improvement found to the Snowball ml and keep track of it in the package readme. err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ... It could still be rather expensive per stem at query time. I vote for getting rid of it if we can. I'll throw an eye at it.
          Hide
          Karl Wettin added a comment -

          Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>.

          This broke InstantiatedIndex in the trunk. Patch and commit is on the way.

          Show
          Karl Wettin added a comment - Generification of Document. It makes now clear what getFields() returns really. This was very bad documented. Now its a List<Fieldable>. This broke InstantiatedIndex in the trunk. Patch and commit is on the way.
          Hide
          Uwe Schindler added a comment -

          how that?

          Show
          Uwe Schindler added a comment - how that?
          Hide
          Karl Wettin added a comment -

          Generified ShingleMatrixFilter

          Show
          Karl Wettin added a comment - Generified ShingleMatrixFilter
          Hide
          Karl Wettin added a comment -

          Generified ShingleMatrixFilter

          Committed in rev 821311

          Show
          Karl Wettin added a comment - Generified ShingleMatrixFilter Committed in rev 821311
          Hide
          Karl Wettin added a comment -

          how that?

          It asserted that a Document contained a List<Field> rather than List<Fieldable> in ctor(IndexReader) , which I actually think is true at that point using that code.

          Show
          Karl Wettin added a comment - how that? It asserted that a Document contained a List<Field> rather than List<Fieldable> in ctor(IndexReader) , which I actually think is true at that point using that code.
          Hide
          Karl Wettin added a comment -

          Fix for InstantiadexIndex compile error caused by code committed in revision 821277
          List<Fieldable> rather than List<Field>

          Show
          Karl Wettin added a comment - Fix for InstantiadexIndex compile error caused by code committed in revision 821277 List<Fieldable> rather than List<Field>
          Hide
          Karl Wettin added a comment -

          Fix for InstantiadexIndex compile error caused by code committed in revision 821277

          Committed in rev 821315

          Show
          Karl Wettin added a comment - Fix for InstantiadexIndex compile error caused by code committed in revision 821277 Committed in rev 821315
          Hide
          Uwe Schindler added a comment -

          Ah ok, the list is Fieldable. That was exactly the problem with Document, it was not documented, what was inside

          Just commit it.

          Show
          Uwe Schindler added a comment - Ah ok, the list is Fieldable. That was exactly the problem with Document, it was not documented, what was inside Just commit it.
          Hide
          Robert Muir added a comment -

          changes o.a.l.messages (back) to varargs syntax to match the java 5 MessageFormat and removes ugly Object[] {} wrappers.

          Show
          Robert Muir added a comment - changes o.a.l.messages (back) to varargs syntax to match the java 5 MessageFormat and removes ugly Object[] {} wrappers.
          Hide
          Uwe Schindler added a comment -

          Committed Revision: 821443 (Change some occurrences of StringBuffer in public/protected APIs of contrib/surround to StringBuilder)

          Show
          Uwe Schindler added a comment - Committed Revision: 821443 (Change some occurrences of StringBuffer in public/protected APIs of contrib/surround to StringBuilder)
          Hide
          Uwe Schindler added a comment -

          Committed revision: 821447 (o.a.l.messages vararg conversion)

          Show
          Uwe Schindler added a comment - Committed revision: 821447 (o.a.l.messages vararg conversion)
          Hide
          Paul Elschot added a comment -

          Uwe, thanks for adding the changes.txt entry and committing the contrib/surround StringBuffer to StringBuilder patch.

          Show
          Paul Elschot added a comment - Uwe, thanks for adding the changes.txt entry and committing the contrib/surround StringBuffer to StringBuilder patch.
          Hide
          Karl Wettin added a comment -

          err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ...

          Mark, are you referring to the reflection in Among? Those are pretty tough to get rid of.

          I think we should replace the StringBuffers in the stemmers if nobody else minds. But I think we should do that in another issue. I also found a bit of ASL headers in some of the classes. Suppose they have been added automatically at some point. These classes are all BSD.

          Show
          Karl Wettin added a comment - err... looks like perhaps its only hit once though and then reused.. maybe not so nasty. My first time looking at this code, so I'm sure you can clear it up ... Mark, are you referring to the reflection in Among? Those are pretty tough to get rid of. I think we should replace the StringBuffers in the stemmers if nobody else minds. But I think we should do that in another issue. I also found a bit of ASL headers in some of the classes. Suppose they have been added automatically at some point. These classes are all BSD.
          Hide
          Earwin Burrfoot added a comment -

          Not sure if that's the right issue, but still.

          Amongst other stuff Java5 introduced for-each loops, which can replace lots of for/while constructs with more readable ones.
          Should I post the patch with all such loops converted?

          Show
          Earwin Burrfoot added a comment - Not sure if that's the right issue, but still. Amongst other stuff Java5 introduced for-each loops, which can replace lots of for/while constructs with more readable ones. Should I post the patch with all such loops converted?
          Hide
          Earwin Burrfoot added a comment -

          Also, we can remove tons of boxing/unboxing, auto(un)boxing calls the very same valueOf(), nnnValue() methods for us.

          Show
          Earwin Burrfoot added a comment - Also, we can remove tons of boxing/unboxing, auto( un )boxing calls the very same valueOf(), nnnValue() methods for us.
          Hide
          Timo Nentwig added a comment -

          eclipse has a code cleanup tool that can easily do all this automatically. It can also make all possible fields/variables final, something I would like to recommend as well.

          Show
          Timo Nentwig added a comment - eclipse has a code cleanup tool that can easily do all this automatically. It can also make all possible fields/variables final, something I would like to recommend as well.
          Hide
          Karthik K added a comment -
          • DisjunctionMaxQuery.java - some of the casts are not necessary now that the members are made type-safe.
          Show
          Karthik K added a comment - DisjunctionMaxQuery.java - some of the casts are not necessary now that the members are made type-safe.
          Hide
          Uwe Schindler added a comment -

          Committed revision: 826035

          Show
          Uwe Schindler added a comment - Committed revision: 826035
          Hide
          Karthik K added a comment -
          • BooleanFilter ( internal data structures conformed to generics )
          Show
          Karthik K added a comment - BooleanFilter ( internal data structures conformed to generics )
          Hide
          Karthik K added a comment -

          Generify signatures.

          Show
          Karthik K added a comment - Generify signatures.
          Hide
          Uwe Schindler added a comment -

          I am working on it, I already fixed the missing SegmentInfo clone cast. Was there any other change in the SegmentInfo patch?

          Show
          Uwe Schindler added a comment - I am working on it, I already fixed the missing SegmentInfo clone cast. Was there any other change in the SegmentInfo patch?
          Hide
          Uwe Schindler added a comment -

          again we have the date sorting bug in JIRA because of am/pm. Can someone change JIRA to use ISO 8601 datstamps in its filelist and everywhere? krrrrrrrrrr.

          Kay kay: please wait one hour with posting more patches, otherwise I will loose track.

          Show
          Uwe Schindler added a comment - again we have the date sorting bug in JIRA because of am/pm. Can someone change JIRA to use ISO 8601 datstamps in its filelist and everywhere? krrrrrrrrrr. Kay kay: please wait one hour with posting more patches, otherwise I will loose track.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257-SegmentInfos.patch 2009-10-17 12:59 AM Kay Kay 6 kB
          LUCENE-1257-BufferedDeletes_DocumentsWriter.patch 2009-10-17 12:44 AM Kay Kay 5 kB
          LUCENE-1257-NormalizeCharMap.patch 2009-10-17 12:15 AM Kay Kay 1 kB
          LUCENE-1257-WordListLoader.patch 2009-10-17 12:11 AM Kay Kay 3 kB
          LUCENE-1257-BooleanScorer_2.patch 2009-10-17 01:39 AM Kay Kay 4 kB
          LUCENE-1257-BooleanQuery.patch 2009-10-17 01:31 AM Kay Kay 5 kB
          LUCENE-1257_BooleanFilter_Generics.patch 2009-10-16 11:52 PM Kay Kay 4 kB

          I also made BooleanQuery Iterable<BooleanClause> for easy usage in advanced for loops. Also added some @Override there.

          Revision: 826213 - Thanks Kay Kay!

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 -SegmentInfos.patch 2009-10-17 12:59 AM Kay Kay 6 kB LUCENE-1257 -BufferedDeletes_DocumentsWriter.patch 2009-10-17 12:44 AM Kay Kay 5 kB LUCENE-1257 -NormalizeCharMap.patch 2009-10-17 12:15 AM Kay Kay 1 kB LUCENE-1257 -WordListLoader.patch 2009-10-17 12:11 AM Kay Kay 3 kB LUCENE-1257 -BooleanScorer_2.patch 2009-10-17 01:39 AM Kay Kay 4 kB LUCENE-1257 -BooleanQuery.patch 2009-10-17 01:31 AM Kay Kay 5 kB LUCENE-1257 _BooleanFilter_Generics.patch 2009-10-16 11:52 PM Kay Kay 4 kB I also made BooleanQuery Iterable<BooleanClause> for easy usage in advanced for loops. Also added some @Override there. Revision: 826213 - Thanks Kay Kay!
          Hide
          Karthik K added a comment -

          Thanks Uwe for helping with the patches.

          Show
          Karthik K added a comment - Thanks Uwe for helping with the patches.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257-DocFieldProcessorPerThread.patch 2009-10-17 09:40 AM Kay Kay 4 kB
          LUCENE-1257-DirectoryReader.patch 2009-10-17 09:12 AM Kay Kay 8 kB
          LUCENE-1257-ConcurrentMergeScheduler.patch 2009-10-17 08:56 AM Kay Kay 2 kB
          LUCENE-1257-CompoundFileReaderWriter.patch 2009-10-17 08:52 AM Kay Kay 3 kB
          LUCENE-1257-CheckIndex.patch 2009-10-17 08:48 AM Kay Kay 6 kB

          I also added some more generics to FilterIndexReader, MultiReader, ParallelReader (commitUserData, getFieldNames).

          Revision: 826290 - Thanks Kay Kay.

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 -DocFieldProcessorPerThread.patch 2009-10-17 09:40 AM Kay Kay 4 kB LUCENE-1257 -DirectoryReader.patch 2009-10-17 09:12 AM Kay Kay 8 kB LUCENE-1257 -ConcurrentMergeScheduler.patch 2009-10-17 08:56 AM Kay Kay 2 kB LUCENE-1257 -CompoundFileReaderWriter.patch 2009-10-17 08:52 AM Kay Kay 3 kB LUCENE-1257 -CheckIndex.patch 2009-10-17 08:48 AM Kay Kay 6 kB I also added some more generics to FilterIndexReader, MultiReader, ParallelReader (commitUserData, getFieldNames). Revision: 826290 - Thanks Kay Kay.
          Hide
          Uwe Schindler added a comment -

          Kay Kay: You can also provide only one combined patch instead of separate ones.

          Show
          Uwe Schindler added a comment - Kay Kay: You can also provide only one combined patch instead of separate ones.
          Hide
          Karthik K added a comment -

          Patch across a good number of files in org.apache.lucene.index ( generify )

          Show
          Karthik K added a comment - Patch across a good number of files in org.apache.lucene.index ( generify )
          Hide
          Uwe Schindler added a comment -

          Whitespace-cleanup of your patch. See also http://www.lucidimagination.com/search/document/62fe00098351dbe3/whitespace_inside_generics_parameters for how Lucene should format generics parameters.

          I will apply when all tests are run. Thank you for the hard work, that not really fun to convert all this

          Show
          Uwe Schindler added a comment - Whitespace-cleanup of your patch. See also http://www.lucidimagination.com/search/document/62fe00098351dbe3/whitespace_inside_generics_parameters for how Lucene should format generics parameters. I will apply when all tests are run. Thank you for the hard work, that not really fun to convert all this
          Hide
          Uwe Schindler added a comment -

          Committed LUCENE-1257_org_apache_lucene_index.patch
          At revision: 826389

          Show
          Uwe Schindler added a comment - Committed LUCENE-1257 _org_apache_lucene_index.patch At revision: 826389
          Hide
          Karthik K added a comment -

          More set of patches in org.apache.lucene.document .

          Show
          Karthik K added a comment - More set of patches in org.apache.lucene.document .
          Hide
          Uwe Schindler added a comment -

          More set of patches in org.apache.lucene.document

          You mean not only document?

          I looked through it: The o.a.l.util.cache is not ideal: The whole classes should take a <K,V> parameter for the cache and not be fixed to <Object,Object>. This was on my list to fix, so I would leave out the whole cache package in this patch and fix it myself.

          Show
          Uwe Schindler added a comment - More set of patches in org.apache.lucene.document You mean not only document? I looked through it: The o.a.l.util.cache is not ideal: The whole classes should take a <K,V> parameter for the cache and not be fixed to <Object,Object>. This was on my list to fix, so I would leave out the whole cache package in this patch and fix it myself.
          Hide
          Karthik K added a comment -
          • Mostly with IndexWriter
          Show
          Karthik K added a comment - Mostly with IndexWriter
          Hide
          Uwe Schindler added a comment -

          Attached is a new patch with a correct generified o.a.l.util.cache package. Now only the usage of this SimpleLRUCache should also be generified.

          Kay Kay: Do you want to do this or should I do it?

          Show
          Uwe Schindler added a comment - Attached is a new patch with a correct generified o.a.l.util.cache package. Now only the usage of this SimpleLRUCache should also be generified. Kay Kay: Do you want to do this or should I do it?
          Hide
          Karthik K added a comment -

          Uwe: Please feel free to go ahead and do it and commit it, that I can pick up the diff.

          Show
          Karthik K added a comment - Uwe: Please feel free to go ahead and do it and commit it, that I can pick up the diff.
          Hide
          Karthik K added a comment -

          IndexDelete (Policy) and stuff

          Show
          Karthik K added a comment - IndexDelete (Policy) and stuff
          Hide
          Karthik K added a comment -

          o.a.l.search package - generified

          Show
          Karthik K added a comment - o.a.l.search package - generified
          Hide
          Karthik K added a comment -

          o.a.l.search.spans - generics added

          Show
          Karthik K added a comment - o.a.l.search.spans - generics added
          Hide
          Karthik K added a comment -

          Attached is a new patch with a correct generified o.a.l.util.cache package. Now only the usage of this SimpleLRUCache should also be generified.

          Kay Kay: Do you want to do this or should I do it?

          Revised patch attached that address issue with SimpleLRUCache as well

          Show
          Karthik K added a comment - Attached is a new patch with a correct generified o.a.l.util.cache package. Now only the usage of this SimpleLRUCache should also be generified. Kay Kay: Do you want to do this or should I do it? Revised patch attached that address issue with SimpleLRUCache as well
          Hide
          Karthik K added a comment -
          • MapOfSets
          • FieldCacheSanityChecker
          Show
          Karthik K added a comment - MapOfSets FieldCacheSanityChecker
          Hide
          Uwe Schindler added a comment -

          Looks good!

          Committed (with some modifications and additions from my last patch):
          LUCENE-1257-o.a.l.util.patch 2009-10-18 01:15 PM Kay Kay 10 kB
          LUCENE-1257-org_apache_lucene_document.patch 2009-10-18 12:42 PM Kay Kay 29 kB
          LUCENE-1257_o.a.l.store.patch 2009-10-18 11:04 AM Kay Kay 12 kB
          LUCENE-1257_o_a_l_search_spans.patch 2009-10-18 10:52 AM Kay Kay 16 kB
          LUCENE-1257_o_a_l_search.patch 2009-10-18 10:39 AM Kay Kay 28 kB
          LUCENE-1257-IndexDeleter.patch 2009-10-18 09:21 AM Kay Kay 13 kB
          LUCENE-1257-iw.patch 2009-10-18 08:49 AM Kay Kay 19 kB

          Thanks Kay Kay!

          At revision: 826527

          Show
          Uwe Schindler added a comment - Looks good! Committed (with some modifications and additions from my last patch): LUCENE-1257 -o.a.l.util.patch 2009-10-18 01:15 PM Kay Kay 10 kB LUCENE-1257 -org_apache_lucene_document.patch 2009-10-18 12:42 PM Kay Kay 29 kB LUCENE-1257 _o.a.l.store.patch 2009-10-18 11:04 AM Kay Kay 12 kB LUCENE-1257 _o_a_l_search_spans.patch 2009-10-18 10:52 AM Kay Kay 16 kB LUCENE-1257 _o_a_l_search.patch 2009-10-18 10:39 AM Kay Kay 28 kB LUCENE-1257 -IndexDeleter.patch 2009-10-18 09:21 AM Kay Kay 13 kB LUCENE-1257 -iw.patch 2009-10-18 08:49 AM Kay Kay 19 kB Thanks Kay Kay! At revision: 826527
          Hide
          Karthik K added a comment -

          o.a.lucene.index in src/test

          Show
          Karthik K added a comment - o.a.lucene.index in src/test
          Hide
          Karthik K added a comment -

          Patch resubmitted confirming to generics code guidelines

          Show
          Karthik K added a comment - Patch resubmitted confirming to generics code guidelines
          Hide
          Uwe Schindler added a comment -

          I think test code rewrite is not so important now. I think first I have to remove the rest of the deprecations and fix the test bug (see java-dev mail).

          I ran the compilation with unchecked warnings turned on, only 38 warnings for core. I think the rest in core should be doable now JUHU!

          Show
          Uwe Schindler added a comment - I think test code rewrite is not so important now. I think first I have to remove the rest of the deprecations and fix the test bug (see java-dev mail). I ran the compilation with unchecked warnings turned on, only 38 warnings for core. I think the rest in core should be doable now JUHU!
          Hide
          Karthik K added a comment -

          IndexFileDeleter
          IndexDeletePolicy

          Show
          Karthik K added a comment - IndexFileDeleter IndexDeletePolicy
          Hide
          Karthik K added a comment -

          I think test code rewrite is not so important now. I think first I have to remove the rest of the deprecations and fix the test bug (see java-dev mail).

          I ran the compilation with unchecked warnings turned on, only 38 warnings for core. I think the rest in core should be doable now JUHU!

          Ok - Makes sense. It would be nice to get it down to 0 soon..

          Show
          Karthik K added a comment - I think test code rewrite is not so important now. I think first I have to remove the rest of the deprecations and fix the test bug (see java-dev mail). I ran the compilation with unchecked warnings turned on, only 38 warnings for core. I think the rest in core should be doable now JUHU! Ok - Makes sense. It would be nice to get it down to 0 soon..
          Hide
          Uwe Schindler added a comment -

          IndexFileDeleter, IndexDeletePolicy

          Last patch makes sense, its better to use abstract base class! Committed.

          With generified ThreadLocal I get now 73 warnings, some more

          Show
          Uwe Schindler added a comment - IndexFileDeleter, IndexDeletePolicy Last patch makes sense, its better to use abstract base class! Committed. With generified ThreadLocal I get now 73 warnings, some more
          Hide
          Karthik K added a comment -

          o.a.l.queryParser

          Show
          Karthik K added a comment - o.a.l.queryParser
          Hide
          Karthik K added a comment -

          Analyzer (generified )

          Other Analyzers.

          First cut version of the patch. If Analyzer<T> is ok - then other analyzers can be ported soon to this as well.

          Show
          Karthik K added a comment - Analyzer (generified ) Other Analyzers. First cut version of the patch. If Analyzer<T> is ok - then other analyzers can be ported soon to this as well.
          Hide
          Uwe Schindler added a comment - - edited

          First cut version of the patch. If Analyzer<T> is ok - then other analyzers can be ported soon to this as well.

          That's unneeded. Analyzer always return a TokenStream and Tokenizer is a subclass of TokenStream. This makes no sense.

          From your patch, it seems that you want to generify the setPreviousTokenStream method. This shoudl not take Object, it should take TokenStream as param. But this has nothing to do with Java 5.

          Robert Muir knows more, he implemented this caching.

          Show
          Uwe Schindler added a comment - - edited First cut version of the patch. If Analyzer<T> is ok - then other analyzers can be ported soon to this as well. That's unneeded. Analyzer always return a TokenStream and Tokenizer is a subclass of TokenStream. This makes no sense. From your patch, it seems that you want to generify the setPreviousTokenStream method. This shoudl not take Object, it should take TokenStream as param. But this has nothing to do with Java 5. Robert Muir knows more, he implemented this caching.
          Hide
          Uwe Schindler added a comment -

          LUCENE-1257_o.a.l.queryParser.patch

          This is hard to generify, because parts of the API are provided by javacc. You can only patch the .jj file but only parts not automatically generated by javacc.

          I would keep this classes as they are and maybe update javacc's param to generate 1.5 code. I will look into this.

          Show
          Uwe Schindler added a comment - LUCENE-1257 _o.a.l.queryParser.patch This is hard to generify, because parts of the API are provided by javacc. You can only patch the .jj file but only parts not automatically generated by javacc. I would keep this classes as they are and maybe update javacc's param to generate 1.5 code. I will look into this.
          Hide
          Uwe Schindler added a comment -

          LUCENE-1257_o.a.l.queryParser.patch

          This is hard to generify, because parts of the API are provided by javacc. You can only patch the .jj file but only parts not automatically generated by javacc.

          I would keep this classes as they are and maybe update javacc's param to generate 1.5 code. I will look into this.

          I updated the parser generator task to use Java 1.5. If you want to generify the other parts of QueryParser, update the .jj file and regenerate the java files. I will do this tomorrow. Will go to bed now.

          Show
          Uwe Schindler added a comment - LUCENE-1257 _o.a.l.queryParser.patch This is hard to generify, because parts of the API are provided by javacc. You can only patch the .jj file but only parts not automatically generated by javacc. I would keep this classes as they are and maybe update javacc's param to generate 1.5 code. I will look into this. I updated the parser generator task to use Java 1.5. If you want to generify the other parts of QueryParser, update the .jj file and regenerate the java files. I will do this tomorrow. Will go to bed now.
          Hide
          Robert Muir added a comment -

          From your patch, it seems that you want to generify the setPreviousTokenStream method. This shoudl not take Object, it should take TokenStream as param. But this has nothing to do with Java 5.

          check out LUCENE-1794 for some related discussion.

          There are a few things that come to mind:

          • for reusable token streams, an analyzer needs to reset the chain: .reset(), but it also needs to call reset(Reader) which is only applicable to Tokenizer. This is why you see private classes like SavedStreams being used.
          • because .tokenStream() takes a fieldname as an argument, some analyzers have special field-specific behavior. To reuse for these analyzers can be more complex, for example QueryAutoStopWordAnalyzer sets/gets a map.
          Show
          Robert Muir added a comment - From your patch, it seems that you want to generify the setPreviousTokenStream method. This shoudl not take Object, it should take TokenStream as param. But this has nothing to do with Java 5. check out LUCENE-1794 for some related discussion. There are a few things that come to mind: for reusable token streams, an analyzer needs to reset the chain: .reset(), but it also needs to call reset(Reader) which is only applicable to Tokenizer. This is why you see private classes like SavedStreams being used. because .tokenStream() takes a fieldname as an argument, some analyzers have special field-specific behavior. To reuse for these analyzers can be more complex, for example QueryAutoStopWordAnalyzer sets/gets a map.
          Hide
          Uwe Schindler added a comment -

          Robert: Thanks!

          But we should not generify the wole analyzer class just because we have some internal helper API that may be generified. We are OK with casts from Object downto the actual type here. A type param in Analyzer is strange and confusing for outstanding developers.

          Show
          Uwe Schindler added a comment - Robert: Thanks! But we should not generify the wole analyzer class just because we have some internal helper API that may be generified. We are OK with casts from Object downto the actual type here. A type param in Analyzer is strange and confusing for outstanding developers.
          Hide
          Robert Muir added a comment -

          Uwe, I agree. I think if we want to improve this reusability in the future, we should look at doing something similar to what Shai Erera proposed, a subclass of Analyzer that does the common SavedStreams behavior.

          But I think Analyzer should still have Object, so the special cases like QueryAutoStopWord can be implemented.

          Show
          Robert Muir added a comment - Uwe, I agree. I think if we want to improve this reusability in the future, we should look at doing something similar to what Shai Erera proposed, a subclass of Analyzer that does the common SavedStreams behavior. But I think Analyzer should still have Object, so the special cases like QueryAutoStopWord can be implemented.
          Hide
          Uwe Schindler added a comment -

          So we just generify the ThreadLocal to <Object> or <?> and finito. I had that in mind.

          As noted before: the generic type param Kay Kay added has nothing to do with the Analyzer itsself, only with its internal impl. So do not make it public! The Object returning save/get method is just a very special helper. And this down-cast is not "unsafe" as long as you do not reuse the method from somewhere outside of Analyzer.

          Show
          Uwe Schindler added a comment - So we just generify the ThreadLocal to <Object> or <?> and finito. I had that in mind. As noted before: the generic type param Kay Kay added has nothing to do with the Analyzer itsself, only with its internal impl. So do not make it public! The Object returning save/get method is just a very special helper. And this down-cast is not "unsafe" as long as you do not reuse the method from somewhere outside of Analyzer.
          Hide
          Robert Muir added a comment -

          updated analysis patch.
          I did not touch StopFilter or StopAnalyzer due to some mixed CharArraySet / Set<String> usage... any ideas on this one Uwe?

          Show
          Robert Muir added a comment - updated analysis patch. I did not touch StopFilter or StopAnalyzer due to some mixed CharArraySet / Set<String> usage... any ideas on this one Uwe?
          Hide
          Karthik K added a comment -

          Classes affected:

          • TermInfosReader
          • SegmentReader
          • FieldsReader
          Show
          Karthik K added a comment - Classes affected: TermInfosReader SegmentReader FieldsReader
          Hide
          Uwe Schindler added a comment -

          I did not touch StopFilter or StopAnalyzer due to some mixed CharArraySet / Set<String> usage... any ideas on this one Uwe?

          I am hanging on that, too. See also LUCENE-1987 and LUCENE-1989. As this set needs no type safety (when it is implemented by CharArraySet) it does not matter if the contains methods uses char[] or String or even Object. It always compares the string representation of the tested value. As CharArraySet is defined as Set<Object>, we should define all these as Set<Object> in StopFilter. Or declare them as CharArraySet and convert the anonyous Set<?> to CharArraySet in the ctor (I would prefer this).

          Show
          Uwe Schindler added a comment - I did not touch StopFilter or StopAnalyzer due to some mixed CharArraySet / Set<String> usage... any ideas on this one Uwe? I am hanging on that, too. See also LUCENE-1987 and LUCENE-1989 . As this set needs no type safety (when it is implemented by CharArraySet) it does not matter if the contains methods uses char[] or String or even Object. It always compares the string representation of the tested value. As CharArraySet is defined as Set<Object>, we should define all these as Set<Object> in StopFilter. Or declare them as CharArraySet and convert the anonyous Set<?> to CharArraySet in the ctor (I would prefer this).
          Hide
          Uwe Schindler added a comment -

          I removed some unneeded patches.

          Show
          Uwe Schindler added a comment - I removed some unneeded patches.
          Hide
          Uwe Schindler added a comment -

          Comitted:
          LUCENE-1257-CloseableThreadLocal.patch 2009-10-18 06:31 PM Kay Kay 4 kB
          LUCENE-1257_analysis.patch 2009-10-18 05:41 PM Robert Muir 8 kB

          At revision: 826601

          Show
          Uwe Schindler added a comment - Comitted: LUCENE-1257 -CloseableThreadLocal.patch 2009-10-18 06:31 PM Kay Kay 4 kB LUCENE-1257 _analysis.patch 2009-10-18 05:41 PM Robert Muir 8 kB At revision: 826601
          Hide
          Uwe Schindler added a comment -

          One note: I do not want to apply any test-related generics patches, as it makes it harder to port patches to the backwards branch currently.
          As soon as all deprecations are removed, we could start with fixing the tests. Before removing all deprecations it may often be needed to also apply changes to the backwards branch, which is Java 1.4 for backwards testing with 2.9.

          Show
          Uwe Schindler added a comment - One note: I do not want to apply any test-related generics patches, as it makes it harder to port patches to the backwards branch currently. As soon as all deprecations are removed, we could start with fixing the tests. Before removing all deprecations it may often be needed to also apply changes to the backwards branch, which is Java 1.4 for backwards testing with 2.9.
          Hide
          Karthik K added a comment -
          • FieldValueHitQueue
          • TopDocsCollector
          • TopScoreDocsCollector
          • TopFieldHitsCollector
          Show
          Karthik K added a comment - FieldValueHitQueue TopDocsCollector TopScoreDocsCollector TopFieldHitsCollector
          Hide
          Uwe Schindler added a comment -

          better generification of MultiTermQueryWrapperFilter (no more casts in sub-classes).

          Show
          Uwe Schindler added a comment - better generification of MultiTermQueryWrapperFilter (no more casts in sub-classes).
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257-MTQWF.patch 2009-10-19 10:55 PM Uwe Schindler 5 kB
          LUCENE-1257-TopDocsCollector.patch 2009-10-19 08:47 PM Kay Kay 8 kB
          LUCENE-1257-FieldCacheImpl.patch 2009-10-19 08:23 PM Kay Kay 8 kB

          (with some modifications in FieldCacheImpl, where Class was not generified to Class<?>).

          At revision: 826857

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 -MTQWF.patch 2009-10-19 10:55 PM Uwe Schindler 5 kB LUCENE-1257 -TopDocsCollector.patch 2009-10-19 08:47 PM Kay Kay 8 kB LUCENE-1257 -FieldCacheImpl.patch 2009-10-19 08:23 PM Kay Kay 8 kB (with some modifications in FieldCacheImpl, where Class was not generified to Class<?>). At revision: 826857
          Hide
          Karthik K added a comment -
          I updated the parser generator task to use Java 1.5. If you want to generify the other parts of QueryParser, update the .jj file and regenerate the java files. I will do this tomorrow. Will go to bed now.

          What's the version of javacc being used/suggested currently ( the latest release seems to be 5.0 ) .

          Show
          Karthik K added a comment - I updated the parser generator task to use Java 1.5. If you want to generify the other parts of QueryParser, update the .jj file and regenerate the java files. I will do this tomorrow. Will go to bed now. What's the version of javacc being used/suggested currently ( the latest release seems to be 5.0 ) .
          Hide
          Uwe Schindler added a comment -

          FieldCacheRangeFilter generified + type safe accessor methods.

          Committed revision: 826883

          Show
          Uwe Schindler added a comment - FieldCacheRangeFilter generified + type safe accessor methods. Committed revision: 826883
          Hide
          Uwe Schindler added a comment -

          What's the version of javacc being used/suggested currently ( the latest release seems to be 5.0 ) .

          From BUILD.txt (I suggest to use this version 4.1, e.g. 4.2 has a bug that corrupts the parser somehow):

          Step 3) Install JavaCC

          Building the Lucene distribution from the source does not require the JavaCC
          parser generator, but if you wish to regenerate any of the pre-generated
          parser pieces, you will need to install JavaCC. Version 4.1 is tested to
          work correctly.

          http://javacc.dev.java.net

          Follow the download links and download the zip file to a temporary
          location on your file system.

          After JavaCC is installed, create a build.properties file
          (as in step 2), and add the line

          javacc.home=/javacc

          where this points to the root directory of your javacc installation
          (the directory that contains bin/lib/javacc.jar).

          Show
          Uwe Schindler added a comment - What's the version of javacc being used/suggested currently ( the latest release seems to be 5.0 ) . From BUILD.txt (I suggest to use this version 4.1, e.g. 4.2 has a bug that corrupts the parser somehow): Step 3) Install JavaCC Building the Lucene distribution from the source does not require the JavaCC parser generator, but if you wish to regenerate any of the pre-generated parser pieces, you will need to install JavaCC. Version 4.1 is tested to work correctly. http://javacc.dev.java.net Follow the download links and download the zip file to a temporary location on your file system. After JavaCC is installed, create a build.properties file (as in step 2), and add the line javacc.home=/javacc where this points to the root directory of your javacc installation (the directory that contains bin/lib/javacc.jar).
          Hide
          Karthik K added a comment -

          QueryParser.jj patch separately for generics

          Show
          Karthik K added a comment - QueryParser.jj patch separately for generics
          Hide
          Karthik K added a comment -

          MultiFieldQueryParser

          Show
          Karthik K added a comment - MultiFieldQueryParser
          Hide
          Karthik K added a comment -

          common-build.xml , build comments match those in build.txt

          Show
          Karthik K added a comment - common-build.xml , build comments match those in build.txt
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_javacc_upgrade.patch 2009-10-20 12:06 AM Kay Kay 0.8 kB
          LUCENE-1257_MultiFieldQueryParser.patch 2009-10-19 11:50 PM Kay Kay 3 kB
          LUCENE-1257_queryParser_jj.patch 2009-10-19 11:45 PM Kay Kay 6 kB

          Also removed deprecated API from QueryParser.

          At revision: 827717

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _javacc_upgrade.patch 2009-10-20 12:06 AM Kay Kay 0.8 kB LUCENE-1257 _MultiFieldQueryParser.patch 2009-10-19 11:50 PM Kay Kay 3 kB LUCENE-1257 _queryParser_jj.patch 2009-10-19 11:45 PM Kay Kay 6 kB Also removed deprecated API from QueryParser. At revision: 827717
          Hide
          Karthik K added a comment -

          Remove unnecessary cast across the codebase (as a result of generification)

          Show
          Karthik K added a comment - Remove unnecessary cast across the codebase (as a result of generification)
          Hide
          Uwe Schindler added a comment -

          Thanks! Much cleaner code. – Committed revision: 827811

          Show
          Uwe Schindler added a comment - Thanks! Much cleaner code. – Committed revision: 827811
          Hide
          DM Smith added a comment -

          Migrates to Java 5 enums in core and contrib. All tests pass.
          Deprecates o.a.l.util.Parameter. It probably can be removed.

          Show
          DM Smith added a comment - Migrates to Java 5 enums in core and contrib. All tests pass. Deprecates o.a.l.util.Parameter. It probably can be removed.
          Hide
          Uwe Schindler added a comment -

          DM Smith: Can you open a new issue. This is more complicated because of backwards compatibility, so a separate issue would be great.

          I like your code rewrite (it could even be done with Parameter in the same way). By the way: Java5's Enum is also Serializable, so no problem.

          Show
          Uwe Schindler added a comment - DM Smith: Can you open a new issue. This is more complicated because of backwards compatibility, so a separate issue would be great. I like your code rewrite (it could even be done with Parameter in the same way). By the way: Java5's Enum is also Serializable, so no problem.
          Hide
          DM Smith added a comment -

          I've moved the patch for Java 5 enums to its own issue: LUCENE-1998

          Show
          DM Smith added a comment - I've moved the patch for Java 5 enums to its own issue: LUCENE-1998
          Hide
          Uwe Schindler added a comment -

          Created a new issue out of clone invariance patch: LUCENE-2000

          Show
          Uwe Schindler added a comment - Created a new issue out of clone invariance patch: LUCENE-2000
          Hide
          Uwe Schindler added a comment -

          Committed:

          • LUCENE-1257_more_unnecessary_casts.patch
          • Remove the rest of unchecked warnings. I added a TODO, where I do not understand the code and not for sure know, whats inside the collections. This could be fixed some time later. But the core code now compiles without any unchecked warning.

          Revision: 828011

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _more_unnecessary_casts.patch Remove the rest of unchecked warnings. I added a TODO, where I do not understand the code and not for sure know, whats inside the collections. This could be fixed some time later. But the core code now compiles without any unchecked warning. Revision: 828011
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_contrib_benchmark.patch 2009-10-22 05:53 PM Kay Kay 56 kB
          LUCENE-1257_unnnecessary_casts_2.patch 2009-10-22 08:16 PM Kay Kay 22 kB

          Revision: 829013

          With the highlighter patch I will wait until LUCENE-2003 is done to not break the patch of Mark.

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _contrib_benchmark.patch 2009-10-22 05:53 PM Kay Kay 56 kB LUCENE-1257 _unnnecessary_casts_2.patch 2009-10-22 08:16 PM Kay Kay 22 kB Revision: 829013 With the highlighter patch I will wait until LUCENE-2003 is done to not break the patch of Mark.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_contrib_benchmark_2.patch 2009-10-23 03:36 PM Kay Kay 22 kB
          LUCENE-1257_contrib_highlighting.patch

          Kay Kay, if you want usage of Class.forName() correct generics without casts, you have to do it that way:

          MyClass instance = Class.forName(nameOfClassToInstantiate).asSubclass(MyClass.class).newInstance()
          

          Same applies to other usages of Class<?>. I modified your benchmark changes to do this pattern.

          Revision: 829524

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _contrib_benchmark_2.patch 2009-10-23 03:36 PM Kay Kay 22 kB LUCENE-1257 _contrib_highlighting.patch Kay Kay, if you want usage of Class.forName() correct generics without casts, you have to do it that way: MyClass instance = Class .forName(nameOfClassToInstantiate).asSubclass(MyClass.class).newInstance() Same applies to other usages of Class<?>. I modified your benchmark changes to do this pattern. Revision: 829524
          Hide
          Karthik K added a comment - - edited

          I modified your benchmark changes to do this pattern.

          Thanks for the heads up. Future patches take care of the same.

          Show
          Karthik K added a comment - - edited I modified your benchmark changes to do this pattern. Thanks for the heads up. Future patches take care of the same.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_precendence_parser.patch 2009-10-25 02:35 PM Kay Kay 9 kB
          LUCENE-1257_contrib_misc.patch 2009-10-25 02:24 PM Kay Kay 18 kB
          LUCENE-1257_contrib_memory.patch 2009-10-25 02:02 PM Kay Kay 26 kB

          I had to fix some small things (one extended for-loop removed, because loop variable had to be global). Also moved the AnalyzerUtil/SynonyMap to wordnet contrib (see LUCENE-1904). Also converted the stop words in PatternAnalyzer to CharArraySet/Set<?> like in other analyzers.

          At revision: 830790

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _precendence_parser.patch 2009-10-25 02:35 PM Kay Kay 9 kB LUCENE-1257 _contrib_misc.patch 2009-10-25 02:24 PM Kay Kay 18 kB LUCENE-1257 _contrib_memory.patch 2009-10-25 02:02 PM Kay Kay 26 kB I had to fix some small things (one extended for-loop removed, because loop variable had to be global). Also moved the AnalyzerUtil/SynonyMap to wordnet contrib (see LUCENE-1904 ). Also converted the stop words in PatternAnalyzer to CharArraySet/Set<?> like in other analyzers. At revision: 830790
          Hide
          Robert Muir added a comment -

          generics patch for smartcn.

          type safety is useful here, it found a problem in the javadocs

          Show
          Robert Muir added a comment - generics patch for smartcn. type safety is useful here, it found a problem in the javadocs
          Hide
          Uwe Schindler added a comment -

          Robert: I think you can commit it yourself, I am away now Looks good on a diagonal view... Even you have the stopwords set with <?> - like everywhere else. I trust you!

          Show
          Uwe Schindler added a comment - Robert: I think you can commit it yourself, I am away now Looks good on a diagonal view... Even you have the stopwords set with <?> - like everywhere else. I trust you!
          Hide
          Robert Muir added a comment -

          Uwe, thanks for taking a look. i think theres some complex hashmaps mapping Integer to ArrayList where i could have used ? extends List instead of ArrayList, but its internally used anyway, so I didn't bother.

          I'll commit the patch shortly.

          Show
          Robert Muir added a comment - Uwe, thanks for taking a look. i think theres some complex hashmaps mapping Integer to ArrayList where i could have used ? extends List instead of ArrayList, but its internally used anyway, so I didn't bother. I'll commit the patch shortly.
          Hide
          Robert Muir added a comment -

          Committed LUCENE-1257_contrib_smartcn.patch, revision 831121.

          Show
          Robert Muir added a comment - Committed LUCENE-1257 _contrib_smartcn.patch, revision 831121.
          Hide
          Karthik K added a comment -

          generics patch for a couple of files

          Show
          Karthik K added a comment - generics patch for a couple of files
          Hide
          Robert Muir added a comment -

          contrib/swing,contrib/wikipedia,contrib/wordnet,contrib/xml-query-parser

          Show
          Robert Muir added a comment - contrib/swing,contrib/wikipedia,contrib/wordnet,contrib/xml-query-parser
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_swing_wikipedia_wordnet_xmlqp.patch 2009-11-10 02:11 AM Robert Muir 24 kB
          LUCENE-1257_o_a_l_demo.patch 2009-11-09 06:57 PM Kay Kay 1 kB

          At revision: 834414

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _swing_wikipedia_wordnet_xmlqp.patch 2009-11-10 02:11 AM Robert Muir 24 kB LUCENE-1257 _o_a_l_demo.patch 2009-11-09 06:57 PM Kay Kay 1 kB At revision: 834414
          Hide
          Karthik K added a comment -

          lucli generics

          Show
          Karthik K added a comment - lucli generics
          Hide
          Simon Willnauer added a comment -

          I revised this patch and did some minor cleanups in lucil.

          • Changed while(iterator.hasNext()) to java 5 loops.
          • added suppress warnings
          • changed some sysouts to messag()
          Show
          Simon Willnauer added a comment - I revised this patch and did some minor cleanups in lucil. Changed while(iterator.hasNext()) to java 5 loops. added suppress warnings changed some sysouts to messag()
          Hide
          Robert Muir added a comment -

          simon, cool, I think it would be good if we were consistent with generics formatting tho:

          public static <K, V extends Comparable<V>> Map.Entry<K,V>[]
          

          I think it should be K,V with no space?

          Show
          Robert Muir added a comment - simon, cool, I think it would be good if we were consistent with generics formatting tho: public static <K, V extends Comparable<V>> Map.Entry<K,V>[] I think it should be K,V with no space?
          Hide
          Uwe Schindler added a comment -

          I think for such complex thing, sometimes spaces may be good But for the typical <K,V>, <String,String> and so on it is hard. There are already some in AttributeSource like this.

          By the way, I uploaded a new Eclipse code style during ApacheCon to wiki.

          Show
          Uwe Schindler added a comment - I think for such complex thing, sometimes spaces may be good But for the typical <K,V>, <String,String> and so on it is hard. There are already some in AttributeSource like this. By the way, I uploaded a new Eclipse code style during ApacheCon to wiki.
          Hide
          Uwe Schindler added a comment -

          Oh god:

          +  @SuppressWarnings("unchecked")
          +  public static <K, V extends Comparable<V>> Map.Entry<K,V>[]
          +    getSortedMapEntries(Map<K,V> m) {
          +    Set<Map.Entry<K, V>> set = m.entrySet();
          +    Map.Entry<K,V>[] entries =
          +       set.toArray(new Map.Entry[set.size()]);
          +    Arrays.sort(entries, new Comparator<Map.Entry<K,V>>() {
          +      public int compare(Map.Entry<K, V> o1, Map.Entry<K, V> o2) {
          +        V v1 = o1.getValue();
          +        V v2 = o2.getValue();
          +        return v2.compareTo(v1); //descending order
          

          What's that g. It needs suppress warnings because of the array creation?

          Show
          Uwe Schindler added a comment - Oh god: + @SuppressWarnings( "unchecked" ) + public static <K, V extends Comparable<V>> Map.Entry<K,V>[] + getSortedMapEntries(Map<K,V> m) { + Set<Map.Entry<K, V>> set = m.entrySet(); + Map.Entry<K,V>[] entries = + set.toArray( new Map.Entry[set.size()]); + Arrays.sort(entries, new Comparator<Map.Entry<K,V>>() { + public int compare(Map.Entry<K, V> o1, Map.Entry<K, V> o2) { + V v1 = o1.getValue(); + V v2 = o2.getValue(); + return v2.compareTo(v1); //descending order What's that g . It needs suppress warnings because of the array creation?
          Hide
          Robert Muir added a comment -

          I think for such complex thing, sometimes spaces may be good But for the typical <K,V>, <String,String> and so on it is hard. There are already some in AttributeSource like this.

          Simon/Uwe I wasn't trying to be picky, more for my own knowledge.
          Contrib formatting is insanity anyway, so I don't object to any patch that introduces type safety

          Show
          Robert Muir added a comment - I think for such complex thing, sometimes spaces may be good But for the typical <K,V>, <String,String> and so on it is hard. There are already some in AttributeSource like this. Simon/Uwe I wasn't trying to be picky, more for my own knowledge. Contrib formatting is insanity anyway, so I don't object to any patch that introduces type safety
          Hide
          Karthik K added a comment -

          I am not sure what the consensus w.r.t warnings are - but as much as possible leaving out SuppressWarnings would be better so that it is transparent in the future to address should the dependent library change.

          Show
          Karthik K added a comment - I am not sure what the consensus w.r.t warnings are - but as much as possible leaving out SuppressWarnings would be better so that it is transparent in the future to address should the dependent library change.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_lucil.patch 2009-11-10 11:09 PM Simon Willnauer 11 kB

          At revision: 834720 - Thanks Simon & Kay

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _lucil.patch 2009-11-10 11:09 PM Simon Willnauer 11 kB At revision: 834720 - Thanks Simon & Kay
          Hide
          Uwe Schindler added a comment -

          Kay Kay: We only have SuppressWarnings at some places in core, marked with a big TODO (will be done when flex indeixng comes). The "wanted" @SuppressWarnings are only at places, where generic Arrays are created. There is no way to fix this (see Sun Generics Howto).

          Show
          Uwe Schindler added a comment - Kay Kay: We only have SuppressWarnings at some places in core, marked with a big TODO (will be done when flex indeixng comes). The "wanted" @SuppressWarnings are only at places, where generic Arrays are created. There is no way to fix this (see Sun Generics Howto).
          Hide
          Karthik K added a comment -

          contrib ant

          Show
          Karthik K added a comment - contrib ant
          Hide
          Robert Muir added a comment -

          generics in benchmark, spellcheck, spatial, snowball, and queries.
          i didn't finish MoreLikeThis in contrib/queries so it still needs to be done.

          Show
          Robert Muir added a comment - generics in benchmark, spellcheck, spatial, snowball, and queries. i didn't finish MoreLikeThis in contrib/queries so it still needs to be done.
          Hide
          Robert Muir added a comment -

          I replace my previous patch with this much larger patch.

          some of it is autogen but I reviewed/corrected

          Show
          Robert Muir added a comment - I replace my previous patch with this much larger patch. some of it is autogen but I reviewed/corrected
          Hide
          Uwe Schindler added a comment -

          I removed also the suppresswarnings and generified TermsHash code. I also added more generics to the changes you already did (because Eclipse is not able to automatically add generics inside generics, e.g. Map<X,Collection<Y>>).

          This patch also contains the ant task.

          Will commit, when all tests finished.

          Show
          Uwe Schindler added a comment - I removed also the suppresswarnings and generified TermsHash code. I also added more generics to the changes you already did (because Eclipse is not able to automatically add generics inside generics, e.g. Map<X,Collection<Y>>). This patch also contains the ant task. Will commit, when all tests finished.
          Hide
          Uwe Schindler added a comment -

          Committed:
          LUCENE-1257_heavy.patch 2009-11-11 12:14 PM Uwe Schindler 69 kB (contains LUCENE-1257_contrib_ant.patch)

          At revision: 834847 - Thanks Robert, Kay, me

          Show
          Uwe Schindler added a comment - Committed: LUCENE-1257 _heavy.patch 2009-11-11 12:14 PM Uwe Schindler 69 kB (contains LUCENE-1257 _contrib_ant.patch) At revision: 834847 - Thanks Robert, Kay, me
          Hide
          Uwe Schindler added a comment -

          Robert: Is there anything else in contrib to convert? If no, I would close this issue for now.

          If we find further Java5 conversions after release of 3.0, we can open new issues.

          Show
          Uwe Schindler added a comment - Robert: Is there anything else in contrib to convert? If no, I would close this issue for now. If we find further Java5 conversions after release of 3.0, we can open new issues.
          Hide
          Robert Muir added a comment -

          If we find further Java5 conversions after release of 3.0, we can open new issues.

          Uwe, I agree, I think you should close the issue.

          Show
          Robert Muir added a comment - If we find further Java5 conversions after release of 3.0, we can open new issues. Uwe, I agree, I think you should close the issue.
          Hide
          Simon Willnauer added a comment -

          We gonna reopen it in 3.0 anyway so just go ahead and close for now!

          Show
          Simon Willnauer added a comment - We gonna reopen it in 3.0 anyway so just go ahead and close for now!
          Hide
          Uwe Schindler added a comment -

          Closed for 3.0. Further updates of tests and internal APIs may follow for 3.1 in a new issue.

          Show
          Uwe Schindler added a comment - Closed for 3.0. Further updates of tests and internal APIs may follow for 3.1 in a new issue.
          Hide
          Karthik K added a comment -
          Further updates of tests and internal APIs may follow for 3.1 in a new issue

          LUCENE-2065 in place for 3.1 to address remaining changes.

          Show
          Karthik K added a comment - Further updates of tests and internal APIs may follow for 3.1 in a new issue LUCENE-2065 in place for 3.1 to address remaining changes.

            People

            • Assignee:
              Uwe Schindler
              Reporter:
              Cédric Champeau
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development