Lucene - Core
  1. Lucene - Core
  2. LUCENE-3552

TaxonomyReader/Writer and their Lucene* implementation

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5, 4.0-ALPHA
    • Component/s: modules/facet
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      The facet module contains two interfaces TaxonomyWriter and TaxonomyReader, with two implementations Lucene*. We've never actually implemented two TaxonomyWriters/Readers, so I'm not sure if these interfaces are useful anymore. Therefore I'd like to propose that we do either of the following:

      1. Remove the interfaces and remove the Lucene part from the implementation classes (to end up with TW/TR impls). Or,
      2. Keep the interfaces, but rename the Lucene* impls to Directory*.

      Whatever we do, I'd like to make the impls/interfaces impl also TwoPhaseCommit.

      Any preferences?

        Activity

        Uwe Schindler made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Uwe Schindler added a comment -

        Bulk close after release of 3.5

        Show
        Uwe Schindler added a comment - Bulk close after release of 3.5
        Shai Erera made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Shai Erera added a comment -

        Thanks Doron, good catch !

        I also renamed o.a.l.facet.taxonomy.lucene to *.directory.

        Committed revision 1196963 (trunk).
        Committed revision 1196965 (3x).

        Show
        Shai Erera added a comment - Thanks Doron, good catch ! I also renamed o.a.l.facet.taxonomy.lucene to *.directory. Committed revision 1196963 (trunk). Committed revision 1196965 (3x).
        Shai Erera committed 1196965 (57 files)
        Reviews: none

        LUCENE-3552: rename LuceneTaxonomyReader/Writer to DirectoryTaxonomyReader/Writer

        Lucene branch_3x
        Shai Erera committed 1196963 (57 files)
        Reviews: none

        LUCENE-3552: rename LuceneTaxonomyReader/Writer to DirectoryTaxonomyReader/Writer

        Lucene trunk
        Hide
        Doron Cohen added a comment -

        Shai Patch looks good!

        Applies cleanly (after svn mv'ing the two renamed classes) and all facet tests run.

        Only userguide.html still mentions the old classes:

        TaxonomyWriter taxo = new LuceneTaxonomyWriter(taxoDir, OpenMode.CREATE);
        ..
        TaxonomyReader taxo = new LuceneTaxonomyReader(taxoDir);
        

        Other than that good to go.

        Show
        Doron Cohen added a comment - Shai Patch looks good! Applies cleanly (after svn mv'ing the two renamed classes) and all facet tests run. Only userguide.html still mentions the old classes: TaxonomyWriter taxo = new LuceneTaxonomyWriter(taxoDir, OpenMode.CREATE); .. TaxonomyReader taxo = new LuceneTaxonomyReader(taxoDir); Other than that good to go.
        Shai Erera made changes -
        Field Original Value New Value
        Attachment LUCENE-3552.patch [ 12501964 ]
        Hide
        Shai Erera added a comment -

        Patch renames LTW/R to DirectoryTW/TR. Also, I renamed LTW's openLuceneIndex/closeLuceneIndex to open/closeIndexWriter.

        I've also made TW extend TwoPhaseCommit.

        I think that it's ready to commit. I'll port the changes to trunk afterwards. I'll wait until tomorrow before I commit (the changes are trivial).

        Show
        Shai Erera added a comment - Patch renames LTW/R to DirectoryTW/TR. Also, I renamed LTW's openLuceneIndex/closeLuceneIndex to open/closeIndexWriter. I've also made TW extend TwoPhaseCommit. I think that it's ready to commit. I'll port the changes to trunk afterwards. I'll wait until tomorrow before I commit (the changes are trivial).
        Hide
        Shai Erera added a comment -

        Ok, let's keep the interface. It's harmful and I can certainly think of alternate taxonomy implementations. Perhaps one day someone would want to implement the taxonomy over a serialized FST or some other data structure ...

        So in this case, I'll:

        • Keep the interfaces
        • Make TaxomomyWriter extend TwoPhaseCommit
        • Rename Lucene TaxoWriter/Reader to DirectoryTaxoWriterReader.

        Any suggestions about an alternative names to the implementations?

        Show
        Shai Erera added a comment - Ok, let's keep the interface. It's harmful and I can certainly think of alternate taxonomy implementations. Perhaps one day someone would want to implement the taxonomy over a serialized FST or some other data structure ... So in this case, I'll: Keep the interfaces Make TaxomomyWriter extend TwoPhaseCommit Rename Lucene TaxoWriter/Reader to DirectoryTaxoWriterReader. Any suggestions about an alternative names to the implementations?
        Hide
        Dafna Sheinwald added a comment -

        Hi Shai,
        Yes, the old (already forgotten) DB implementation of taxonomy index, and the subsequent Lucene implementation gave rise to the definition of TaxonomyReader/Writer interface. Currently, indeed, and we are left with Lucene (Directory) implementation only. But I thought that as the interface is already there, and we have some thoughts of yet another implementation of taxonomy index (and hence TaxonomyReader/Writer), as part of the Lucene search index, which may help to ease the burden of synch between these two indices, why not let the interface live a bit longer before we remove it. Regards, Dafna.

        Show
        Dafna Sheinwald added a comment - Hi Shai, Yes, the old (already forgotten) DB implementation of taxonomy index, and the subsequent Lucene implementation gave rise to the definition of TaxonomyReader/Writer interface. Currently, indeed, and we are left with Lucene (Directory) implementation only. But I thought that as the interface is already there, and we have some thoughts of yet another implementation of taxonomy index (and hence TaxonomyReader/Writer), as part of the Lucene search index, which may help to ease the burden of synch between these two indices, why not let the interface live a bit longer before we remove it. Regards, Dafna.
        Hide
        Shai Erera added a comment -

        What do you mean? If we'll combine the search and taxonomy index, I don't believe we'll call it TaxonomyWriter. The purpose of the interface is for allowing a different implementation of the taxonomy, i.e. something that is not based on a Lucene Directory (e.g. a DB or something).

        Perhaps I misunderstand you?

        Show
        Shai Erera added a comment - What do you mean? If we'll combine the search and taxonomy index, I don't believe we'll call it TaxonomyWriter. The purpose of the interface is for allowing a different implementation of the taxonomy, i.e. something that is not based on a Lucene Directory (e.g. a DB or something). Perhaps I misunderstand you?
        Hide
        Dafna Sheinwald added a comment -

        Hi Shai,
        I would vote for (2). We may want to still maintain the option to implement both in one Lucene index, in order to avoid the burden of synch-ing them.
        Regards, Dafna.

        Show
        Dafna Sheinwald added a comment - Hi Shai, I would vote for (2). We may want to still maintain the option to implement both in one Lucene index, in order to avoid the burden of synch-ing them. Regards, Dafna.
        Shai Erera created issue -

          People

          • Assignee:
            Shai Erera
            Reporter:
            Shai Erera
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development