BIT_PACKING is written by default when Definition or Repetition levels are empty

[willtemperley]

As discussed in apache/arrow#47113 , this package is writing the deprecated BIT_PACKING encoding to the column metadata and the DataPageHeader. This is only seems to happens when rep/def levels are empty. I think this is where it's happening:

public class DevNullValuesWriter extends ValuesWriter {
  ....
  public Encoding getEncoding() {
    return BIT_PACKED;
  }
}

Wouldn't it make sense to just return RLE?

I've seen this in parquet-java 1.15.1 and Overture Maps files using 1.13.1.

[wgtmac]

I guess BIT_PACKED was the only encoding for definition and repetition levels at the time implementing this so it was used as the default value. It seems better to return null from DevNullValuesWriter.getEncoding() and totally get rid of it in the encoding metadata.

[willtemperley]

That would be make sense, but what would be written to the DataPageHeader in that case? Both definition_level_encoding and repetition_level_encoding are required fields.

I noticed that pyarrow writes RLE when there are no rep levels. It only writes Parquet 2 though I believe. Demonstrated by:

import pyarrow as pa
import pyarrow.parquet as pq

# 1. Create a Parquet file with a required, non-repeated column
table = pa.table({"x": pa.array([1, 2, 3], type=pa.int32())})
pq.write_table(table, "test.parquet", version="2.6", use_dictionary=False)

Which creates:

test.parquet.zip

[wgtmac]

I think the encoding value depends on the writer implementation. For pyarrow (which uses parquet-cpp internally) it makes sense to write RLE.

[willtemperley]

I think the encoding value depends on the writer implementation. For pyarrow (which uses parquet-cpp internally) it makes sense to write RLE.

Why does it make sense in this particular case?

To move this discusson forward, do you agree with this statement:

Something has to be written to the the DataPageHeader when there are no rep or def levels, and this value should not be BIT_PACKED.

[wgtmac]

I mean a writer implementation which still encodes levels using deprecated BIT_PACKED should write BIT_PACKED in the header. Otherwise, all new writer implementations should use RLE and thus write RLE in the header. Here the parquet-java already uses RLE to encode levels, so it should write RLE. If this is the case, perhaps we don't need to eliminate RLE from the encoding list because this value exists in the page header.

[willtemperley]

Ah I see what you mean now. We're discussing the situation where parquet-java isn't actually writing any BIT_PACKED data, or any RLE data - it's just writing a default to the metadata when there are no actual levels.

From the docs
The key point being "The definition and repetition levels are optional"

Currently BIT_PACKING is being written when these are not present.