Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-3738

Be consistent about negative vInt/vLong

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Blocker
    • Resolution: Fixed
    • None
    • 3.6, 4.0-ALPHA
    • None
    • None
    • New

    Description

      Today, write/readVInt "allows" a negative int, in that it will encode and decode correctly, just horribly inefficiently (5 bytes).

      However, read/writeVLong fails (trips an assert).

      I'd prefer that both vInt/vLong trip an assert if you ever try to write a negative number... it's badly trappy today. But, unfortunately, we sometimes rely on this... had we had this assert in 'since the beginning' we could have avoided that.

      So, if we can't add that assert in today, I think we should at least fix readVLong to handle negative longs... but then you quietly spend 9 bytes (even more trappy!).

      Attachments

        1. ByteArrayDataInput.java.patch
          4 kB
          Uwe Schindler
        2. LUCENE-3738.patch
          14 kB
          Uwe Schindler
        3. LUCENE-3738.patch
          11 kB
          Uwe Schindler
        4. LUCENE-3738.patch
          1 kB
          Michael McCandless
        5. LUCENE-3738.patch
          1 kB
          Uwe Schindler
        6. LUCENE-3738.patch
          0.6 kB
          Uwe Schindler
        7. LUCENE-3738-improvement.patch
          7 kB
          Uwe Schindler

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            uschindler Uwe Schindler
            mikemccand Michael McCandless
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment