Solr
  1. Solr
  2. SOLR-1079

Rename omitTf to omitTermFreqAndPositions

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4
    • Component/s: documentation, update
    • Labels:
      None
    1. SOLR-1079.patch
      10 kB
      Shalin Shekhar Mangar
    2. SOLR-1079.patch
      2 kB
      Shalin Shekhar Mangar
    3. SOLR-1079-fixcompilation.patch
      3 kB
      Uwe Schindler

      Issue Links

        Activity

        Hide
        Yonik Seeley added a comment -

        We need to get the latest lucene libs and adjust our code, but I'd vote for keeping our schema flag as "omitTf" for the following reasons:

        • omitTermFreqAndPositions is looong
        • it's not comprehensive anyway - omitTermFreqAndPositionsAndPayloads? ;-P
        • and still... some people may not realize that phrase queries need positions, or that things that don't look like phrase queries may get turned into phrase queries (foo-bar and WDF for example).
        • most Solr users don't need to know what it does... we default non-text fields to use omitTf... text fields should always default to that.
        • almost no one will have pre-conceived notions about what "omitTf" will do anyway... they will need to look at the description. Hence this is really only a documentation problem, not a naming problem.

        On the other side of the coin.... if we do the correct defaults (which I believe we do) then omitTermFreqAndPositions would rarely appear in anyone's schema anyway. Only the most advanced user would choose to change the defaults and for them a consistency with lucene naming may be desired?

        After all that, I guess I should have argued against Lucene renaming omitTf... and at this point Solr should probably just follow suit?

        Show
        Yonik Seeley added a comment - We need to get the latest lucene libs and adjust our code, but I'd vote for keeping our schema flag as "omitTf" for the following reasons: omitTermFreqAndPositions is looong it's not comprehensive anyway - omitTermFreqAndPositionsAndPayloads? ;-P and still... some people may not realize that phrase queries need positions, or that things that don't look like phrase queries may get turned into phrase queries (foo-bar and WDF for example). most Solr users don't need to know what it does... we default non-text fields to use omitTf... text fields should always default to that. almost no one will have pre-conceived notions about what "omitTf" will do anyway... they will need to look at the description. Hence this is really only a documentation problem, not a naming problem. On the other side of the coin.... if we do the correct defaults (which I believe we do) then omitTermFreqAndPositions would rarely appear in anyone's schema anyway. Only the most advanced user would choose to change the defaults and for them a consistency with lucene naming may be desired? After all that, I guess I should have argued against Lucene renaming omitTf... and at this point Solr should probably just follow suit?
        Hide
        Shalin Shekhar Mangar added a comment -

        almost no one will have pre-conceived notions about what "omitTf" will do anyway... they will need to look at the description. Hence this is really only a documentation problem, not a naming problem.

        Good point! It if confuses me, I'm sure to check the documentation before changing it.

        I'm open to keeping it as is or changing it to conform to Lucene. I don't think it matters too much.

        My main goal is to make everybody aware of Lucene's changes and to have more documentation on this. Neither the example schema nor the SchemaXml wiki page have any details about omitTf. The only place it is mentioned is the change log.

        Show
        Shalin Shekhar Mangar added a comment - almost no one will have pre-conceived notions about what "omitTf" will do anyway... they will need to look at the description. Hence this is really only a documentation problem, not a naming problem. Good point! It if confuses me, I'm sure to check the documentation before changing it. I'm open to keeping it as is or changing it to conform to Lucene. I don't think it matters too much. My main goal is to make everybody aware of Lucene's changes and to have more documentation on this. Neither the example schema nor the SchemaXml wiki page have any details about omitTf. The only place it is mentioned is the change log.
        Hide
        Noble Paul added a comment -

        keeping it short is better . The documentation can have details .

        Show
        Noble Paul added a comment - keeping it short is better . The documentation can have details .
        Hide
        Otis Gospodnetic added a comment -

        I agree with shorter is better... though we should avoid cryptic or misleading. I think omitTf is misleading. I'd rather we think of something that's maybe less descriptive (since one will need to look at the docs anyway), but not misleading (making the person think looking at the docs is not necessary)

        maybe omitTerm<something>? "Info"? That would sort of match Lucene's TermInfo object (which doesn't encompass Payloads, though).

        Show
        Otis Gospodnetic added a comment - I agree with shorter is better... though we should avoid cryptic or misleading. I think omitTf is misleading. I'd rather we think of something that's maybe less descriptive (since one will need to look at the docs anyway), but not misleading (making the person think looking at the docs is not necessary) maybe omitTerm<something>? "Info"? That would sort of match Lucene's TermInfo object (which doesn't encompass Payloads, though).
        Hide
        Yonik Seeley added a comment -

        I disagree that "omitTf" is misleading... one would already have to be an expert to assume that it meant anything.
        But while I started arguing one way in my previous message, I convinced myself of the opposite by the end - we should just rename to whatever it is in Lucene since the flag will pretty much never appear in Solr schemas anyway.

        Show
        Yonik Seeley added a comment - I disagree that "omitTf" is misleading... one would already have to be an expert to assume that it meant anything. But while I started arguing one way in my previous message, I convinced myself of the opposite by the end - we should just rename to whatever it is in Lucene since the flag will pretty much never appear in Solr schemas anyway.
        Hide
        Hoss Man added a comment -

        we should just rename to whatever it is in Lucene since the flag will pretty much never appear in Solr schemas anyway.

        reading the msgs in order, you convinced me, and then you un-convinced me, and then you re-un-convinced me.

        as long as it's going to be an option 99% of the world doesn't need to know about, and will need lots of documentation to explain anyway, it might as well be a long name that's consistent with the underlying java code.

        Show
        Hoss Man added a comment - we should just rename to whatever it is in Lucene since the flag will pretty much never appear in Solr schemas anyway. reading the msgs in order, you convinced me, and then you un-convinced me, and then you re-un-convinced me. as long as it's going to be an option 99% of the world doesn't need to know about, and will need lots of documentation to explain anyway, it might as well be a long name that's consistent with the underlying java code.
        Hide
        Shalin Shekhar Mangar added a comment -

        Patch to rename omitTf to omitTermFreqAndPositions.

        Do we all agree that we should go with whatever name Lucene selects?

        Show
        Shalin Shekhar Mangar added a comment - Patch to rename omitTf to omitTermFreqAndPositions. Do we all agree that we should go with whatever name Lucene selects?
        Hide
        Shalin Shekhar Mangar added a comment -

        Please ignore the last patch. Some more changes are required. I'll post a new one soon.

        Show
        Shalin Shekhar Mangar added a comment - Please ignore the last patch. Some more changes are required. I'll post a new one soon.
        Hide
        Uwe Schindler added a comment -

        This is a first patch to remove the dummy implementation of Fieldable in QueryComponent (its not needed). This private class prevents the usage of Lucene 2.9 trunk without modifying the source (as the interface changed again in 2.9).
        This patch just removes the FieldAble implementation colpletely and uses a conventional Out-Of-The-Box-Field to do the same. It is initialized like the StringFieldable at the beginning an then reused. The fieldname and initial value are just dummies and Field.Store.YES is used to make the Field c'tor happy.

        All tests pass.

        Show
        Uwe Schindler added a comment - This is a first patch to remove the dummy implementation of Fieldable in QueryComponent (its not needed). This private class prevents the usage of Lucene 2.9 trunk without modifying the source (as the interface changed again in 2.9). This patch just removes the FieldAble implementation colpletely and uses a conventional Out-Of-The-Box-Field to do the same. It is initialized like the StringFieldable at the beginning an then reused. The fieldname and initial value are just dummies and Field.Store.YES is used to make the Field c'tor happy. All tests pass.
        Hide
        Shalin Shekhar Mangar added a comment -

        Attached new patch which includes Uwe's changes to QueryComponent.

        We'd need to commit this issue, SOLR-940 updates and new lucene jars in one go to avoid compilation failures.

        Show
        Shalin Shekhar Mangar added a comment - Attached new patch which includes Uwe's changes to QueryComponent. We'd need to commit this issue, SOLR-940 updates and new lucene jars in one go to avoid compilation failures.
        Hide
        Yonik Seeley added a comment -

        Just got back from vacation...

        Do we all agree that we should go with whatever name Lucene selects?

        +1

        Show
        Yonik Seeley added a comment - Just got back from vacation... Do we all agree that we should go with whatever name Lucene selects? +1
        Hide
        Shalin Shekhar Mangar added a comment -

        Committed revision 764291 as part of SOLR-940 changes.

        Show
        Shalin Shekhar Mangar added a comment - Committed revision 764291 as part of SOLR-940 changes.
        Hide
        Grant Ingersoll added a comment -

        Bulk close for Solr 1.4

        Show
        Grant Ingersoll added a comment - Bulk close for Solr 1.4

          People

          • Assignee:
            Shalin Shekhar Mangar
            Reporter:
            Shalin Shekhar Mangar
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development