GH-50915: [FORMAT] Allow TIMESTAMP logical type to annotate FIXED_LEN_BYTE_ARRAY(12)

[divjotarora]

Rationale for this change

See apache/parquet-format#600 for rationale.

What changes are included in this PR?

This PR adds support for using TimestampType to annotate FIXED_LEN_BYTE_ARRAY(12) values.

Are these changes tested?

Yes, via unit tests and an e2e test that reads the file added in parquet-testing (apache/parquet-testing#123).

Are there any user-facing changes?

No

• GitHub Issue: [Format] Allow TimestampType to annotate FLBA(12) #50915

[github-actions]

⚠️ GitHub issue #50915 has been automatically assigned in GitHub to PR creator.

[emkornfield]

Some questions and comments I think the biggest one is scope and whether we should have an option to convert this value to a proper arrow type. Wemight also want to make it configurable the target of the arrow type

[emkornfield]

I think the conversion code always assumes a 1:1 mapping between arrow timestamp granularity and parquet granularity. I think in the common path when schema is inferred this is workable, but IIRC users can also supply there own schema (we should add test coverage for this path).

[divjotarora]

I think the conversion code always assumes a 1:1 mapping between arrow timestamp granularity and parquet granularity. I think in the common path when schema is inferred this is workable, but IIRC users can also supply there own schema (we should add test coverage for this path).

There doesn't seem to be an API at this level to supply a custom schema. The Arrow timestamp unit is derived from the Parquet logical type during schema conversion and that's passed down to the data converters. Based on this, I don't think any scaling is needed in this PR. To ensure correctness, I added a defensive check that fails the conversion if the Arrow and Parquet units differ.

[emkornfield]

one more thing to look at. I need to double check if there is a better idiom to express the conversion we want to do here, but we should at least look at using the visitor.

[emkornfield]

LGTM, just one more missing test case I think.

[emkornfield]

@divjotarora looks like parquet-reader-test is failing (didn't look at root cause)

[divjotarora]

@emkornfield The new FLBA 12 timestamp test is failing, but it's due to:

unknown file: Failure
C++ exception with description "IOError: Failed to open local file '/arrow/cpp/submodules/parquet-testing/data/flba12_timestamp.parquet'
/arrow/cpp/src/arrow/io/file.cc:106  ::arrow::internal::FileOpenReadable(file_name_)
/arrow/cpp/src/arrow/io/file.cc:307  file->impl_->Open(path). Detail: [errno 2] No such file or directory" thrown in the test body.

AFAICT this is expected as the test is referencing a parquet-testing file that's not merged in. I think this will pass once we merge apache/parquet-testing#123 and update the submodule hash in this repo. I've verified that it passes locally be overriding PARQUET_TEST_DATA

[emkornfield]

Changes look reasonable we can fix CI once vote passes and artifacts are merged.