Reduce code duplication in VectorizedParquetDefinitionLevelReader

[wypoon]

In VectorizedParquetDefinitionLevelReader, there are a number of nested reader classes; these extend one of two nested base classes, NumericBaseReader and BaseReader. Each of these base classes define nextBatch and nextDictEncodedBatch methods. These 4 methods (NumericBaseReader::nextBatch, NumericBaseReader::nextDictEncodedBatch, BaseReader::nextBatch, BaseReader::nextDictEncodedBatch) are actually structurally similar:

      int idx = startOffset;
      int left = numValsToRead;
      while (left > 0) {
        if (currentCount == 0) {
          readNextGroup();
        }
        int numValues = Math.min(left, currentCount);
        ...
        switch (mode) {
          case RLE:
            ...
          case PACKED:
            ...
        }
        left -= numValues;
        currentCount -= numValues;
      }

We can separate out the differences into what gets called in the RLE and PACKED cases for each of the two base classes, and even there, for dict encoded batches, there is no difference in logic between NumericBaseReader and BaseReader. Thus there is quite a bit of duplication that can be reduced.

This PR simply refactors the common logic into a superclass (CommonBaseReader) of NumericBaseReader and BaseReader, in a nextCommonBatch method that nextBatch and nextDictEncodedBatch delegate to.

No tests are added as there is no functionality change. The methods are exercised by existing tests.

Additional notes:
NumericBaseReader and BaseReader both define abstract nextDictEncodedVal methods with the same set of parameters but in different order! In this refactor, we define a common nextDictEncodedVal for them in CommonBaseReader.
NumericBaseReader and BaseReader each define abstract nextVal methods as well, but with different sets of parameters, and these we have left alone.

[wypoon]

This is incremented in the method body and incrementing a parameter (idx) is a checkstyle violation. Hence this variable.
The same applies to 3 other methods below.

[wypoon]

@nastra can you please review this?
My impetus for the refactor is that I want to implement Parquet page-skipping, and with the refactor, a core change in logic can be applied in one method instead of 4 similarly structured methods.

[nastra]

in the previous code this was only done for the BaseReader and not the NumericBaseReader but now it's generally done?

[wypoon]

That is correct. We initialize a byte array and it gets passed to nextRleBatch and nextPackedBatch; these two methods are abstract and overridden by NumericBaseReader and BaseReader; in NumericBaseReader, these two methods simply ignore the byteArray that is passed.

[wypoon]

@nastra thank you for reviewing this. I have done some renaming as you suggested.

[wypoon]

Hi @nastra, I have addressed your feedback last week. Can you please review this again?

[wypoon]

@nastra @RussellSpitzer ping.

[wypoon]

Hi @RussellSpitzer, I see that @nastra requested a review from you on this. I believe he wants a second pair of eyes to check that the refactor is correct. Can you please review?

[wypoon]

@nastra any chance of moving this forward?

[nastra]

@RussellSpitzer @amogh-jahagirdar @pvary could you take a look at this one please?

[wypoon]

@amogh-jahagirdar can you please take a look?