Fixes reindexing bug with filter on long column

[tejaswini-imply]

Description

Filtering on a long column while reindexing results in __time column values to be parsed the same as this long column. BlockLayoutColumnarLongs close method is not nullifying the internal buffer. Hence the __time LongBuffer is overridden by this filtering column.

DruidSegment input rows:

Reindex spec used:

{
  "type": "index_parallel",
  "spec": {
    "ioConfig": {
      "type": "index_parallel",
      "inputSource": {
        "type": "druid",
        "dataSource": "inline_data",
        "interval": "2022-10-30T00:00:00/2022-10-31T00:00:00"
      }
    },
    "tuningConfig": {
      "type": "index_parallel",
      "partitionsSpec": {
        "type": "dynamic"
      },
      "maxNumConcurrentSubTasks": 10
    },
    "dataSchema": {
      "timestampSpec": {
        "column": "__time",
        "format": "millis"
      },
      "granularitySpec": {
        "rollup": false,
        "queryGranularity": "none",
        "segmentGranularity": "day"
      },
      "dimensionsSpec": {
        "dimensions": [
          {
            "name": "a",
            "type": "string"
          },
          {
            "name": "b",
            "type": "string"
          },
          {
            "name": "longCol",
            "type": "long"
          }
        ]
      },
      "metricsSpec": [],
      "dataSource": "inline_data",
      "transformSpec": {
        "filter": {
          "type": "or",
          "fields": [
            {
              "type": "not",
              "field": {
                "type": "selector",
                "dimension": "a",
                "value": "foo1",
                "extractionFn": null
              }
            },
            {
              "type": "not",
              "field": {
                "type": "selector",
                "dimension": "b",
                "value": "bar1",
                "extractionFn": null
              }
            },
            {
              "type": "selector",
              "dimension": "longCol",
              "value": "3",
              "extractionFn": null
            }
          ]
        }
      }
    }
  }
}

Re-indexing (before changes):

{
  "ingestionState": "COMPLETED",
  "unparseableEvents": {
    "buildSegments": []
  },
  "rowStats": {
    "buildSegments": {
      "processed": 0,
      "processedWithError": 0,
      "thrownAway": 2,
      "unparseable": 0
    }
  },
  "errorMsg": null,
  "segmentAvailabilityConfirmed": false,
  "segmentAvailabilityWaitTimeMs": 0
}

The 2 final rows are thrownAway since internally, while loading __time LongBuffer, it's being overridden by longCol values while creating FilteredOffset by QueryableIndexCursorSequenceBuilder.

Re-indexing (after changes):

Key changed/added classes in this PR

• BlockLayoutColumnarLongsSupplier

---

This PR has:

• been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
• added documentation for new or modified features or behaviors.
• a release note entry in the PR description.
• added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
• added or updated version, license, or notice information in licenses.yaml
• added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
• added integration tests.
• been tested in a test Druid cluster.

[clintropolis]

good find 👍

i believe this can also be a query time issue in certain cases since NotFilter and a few other things also use the method which is known to double close the time column

[clintropolis]

nit: I guess currBufferNum is the only thing that truly matters here, but just to be consistent maybe we should also null out longBuffer since we are doing that for everything else.

Additionally, while I am not aware of anywhere which is double closing any float or double columns, they do suffer from the same flaw of not resetting currBufferNum when closed, and so could also read from a buffer that has been returned to the pool if that happened,

• https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/data/BlockLayoutColumnarFloatsSupplier.java#L183
• https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/data/BlockLayoutColumnarDoublesSupplier.java#L183

[imply-cheddar]

+1 for fixing the other classes too.

[tejaswini-imply]

Thanks for pointing that out, fixed other supplier classes too.

[imply-cheddar]

This is a bit of a nit, but "s" and "str" and "stringCol" are all not too very long, but the longer ones actually convey more information on their own. I'd suggest either using "str" and "dbl" or "strCol" and "dblCol" or "stringCol" and "doubleCol", just to be more nice to the next person who reads the code.

[tejaswini-imply]

Just focused on refactoring the test to be used by custom dimensions and metrics spec. Updated in the latest commit.

[imply-cheddar]

+1 for fixing the other classes too.

[clintropolis]

👍 after CI (looks like spotbugs check is complaining about missing @Nullable on things https://app.travis-ci.com/github/apache/druid/jobs/589008836#L873)

[tejaswini-imply]

Fixed spot bugs. Thanks, @clintropolis, for the review.