[C++][Parquet] Should we support PARQUET_2_8 version?

[mapleFU]

Describe the enhancement requested

Nowadays, we support BYTE_STREAM_SPLIT in parquet. However, during writing, our highest format is PARQUET_2_6. So, do we need to support Parquet 2.8 or higher version

Changelogs: https://github.com/apache/parquet-format/blob/master/CHANGES.md#version-280

Component(s)

C++, Parquet

[mapleFU]

cc @pitrou @wgtmac

[wgtmac]

To provide some facts:

It seems that the WriterProperties does not check if any feature does not belong to a certain version. I have only observed several places it was checked:

arrow/cpp/src/parquet/column_writer.cc

Lines 2062 to 2072 in 2d32efe

	} else if ((version == ParquetVersion::PARQUET_1_0 ||
	version == ParquetVersion::PARQUET_2_4) &&
	source_type.unit() == ::arrow::TimeUnit::NANO) {
	// Absent superseding user instructions, when writing Parquet version <= 2.4 files,
	// timestamps in nanoseconds are coerced to microseconds
	std::shared_ptr<ArrowWriterProperties> properties =
	(ArrowWriterProperties::Builder())
	.coerce_timestamps(::arrow::TimeUnit::MICRO)
	->disallow_truncated_timestamps()
	->build();
	return WriteCoerce(properties.get());

arrow/cpp/src/parquet/metadata.cc

Lines 1467 to 1470 in 2d32efe

	if (properties_->version() == ParquetVersion::PARQUET_1_0) {
	thrift_encodings.push_back(ToThrift(Encoding::PLAIN));
	} else {
	thrift_encodings.push_back(ToThrift(properties_->dictionary_page_encoding()));

arrow/cpp/src/parquet/arrow/schema.cc

Lines 319 to 323 in 2d32efe

	if (properties.version() == ::parquet::ParquetVersion::PARQUET_1_0) {
	type = ParquetType::INT64;
	} else {
	type = ParquetType::INT32;
	logical_type = LogicalType::Int(32, false);

parquet-mr does not recognize any 2.x format version and always hardcodes version 1 to the footer metadata:

https://github.com/apache/parquet-mr/blob/d5a4ce05643e6709312e3060838cb9236e882014/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileWriter.java#L1339-L1343

[wgtmac]

My question is: should we check if any enabled feature is beyond the support of the specified format version? If yes, should we support deduce the version from enabled feature set? It is not easy for a user to know which format version to set but it is much easier to know what features are needed.

[mapleFU]

Hmmm I go through the Rust implementions, and found that it just uses "1.0" or "2.0". All implementions use different adhoc way to setting this...
And maybe user uses BYTE_STREAM_SPLIT with 2.6 in arrow, I guess it's a disaster when we really wants to check this...

[wgtmac]

How about adding a Status WriterProperties::validate_format(). This won't break current users but provide a way to check format integrity.

[wgtmac]

cc @emkornfield

[jorisvandenbossche]

Hmmm I go through the Rust implementions, and found that it just uses "1.0" or "2.0". All implementions use different adhoc way to setting this...

Yeah, this "version" field in the footer metadata is not very well specified. See also related discussion at apache/parquet-format#164 (comment)

[pitrou]

How about adding a Status WriterProperties::validate_format(). This won't break current users but provide a way to check format integrity.

Something like that could be useful, yes.

[mapleFU]

I guess it's a big hard, because checking is separted to different places...

1. under FieldToNode in src/parquet/arrow/schema.cc

    case ArrowTypeId::TIMESTAMP:
      RETURN_NOT_OK(
          GetTimestampMetadata(static_cast<::arrow::TimestampType&>(*field->type()),
                               properties, arrow_properties, &type, &logical_type));
      break;

2. arrow/cpp/src/parquet/column_writer.cc

   Lines 2062 to 2072 in 2d32efe

   	} else if ((version == ParquetVersion::PARQUET_1_0 ||
   	version == ParquetVersion::PARQUET_2_4) &&
   	source_type.unit() == ::arrow::TimeUnit::NANO) {
   	// Absent superseding user instructions, when writing Parquet version <= 2.4 files,
   	// timestamps in nanoseconds are coerced to microseconds
   	std::shared_ptr<ArrowWriterProperties> properties =
   	(ArrowWriterProperties::Builder())
   	.coerce_timestamps(::arrow::TimeUnit::MICRO)
   	->disallow_truncated_timestamps()
   	->build();
   	return WriteCoerce(properties.get());

   when write timestamp

I guess we need a validate_format like:

validate_format(const WriterProperties& properties, const ArrowWriterProperties& arrow_properties, Schema);

[wgtmac]

I see the problem. IMO, we can add a new option in the WriterProperties to enable format validation and call validate_format you proposed while creating the parquet writer? @mapleFU