Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-1079

Rename omitTf to omitTermFreqAndPositions

    Details

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

      Issue Links

        Activity

        Hide
        gsingers Grant Ingersoll added a comment -

        Bulk close for Solr 1.4

        Show
        gsingers Grant Ingersoll added a comment - Bulk close for Solr 1.4
        Hide
        shalinmangar Shalin Shekhar Mangar added a comment -

        Committed revision 764291 as part of SOLR-940 changes.

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Committed revision 764291 as part of SOLR-940 changes.
        Hide
        yseeley@gmail.com 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
        yseeley@gmail.com 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
        shalinmangar 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
        shalinmangar 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
        thetaphi 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
        thetaphi 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
        shalinmangar Shalin Shekhar Mangar added a comment -

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

        Show
        shalinmangar Shalin Shekhar Mangar added a comment - Please ignore the last patch. Some more changes are required. I'll post a new one soon.
        Hide
        shalinmangar 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
        shalinmangar 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
        hossman 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
        hossman 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
        yseeley@gmail.com 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
        yseeley@gmail.com 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
        otis 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 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
        noble.paul Noble Paul added a comment -

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

        Show
        noble.paul Noble Paul added a comment - keeping it short is better . The documentation can have details .
        Hide
        shalinmangar 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
        shalinmangar 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
        yseeley@gmail.com 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
        yseeley@gmail.com 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?

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development