GH-39419: [C++][Parquet] Style: Using arrow::Buffer data_as api rather than reinterpret_cast

[mapleFU]

Rationale for this change

This patch using {mutable}_data_as<T>() api to replace interpret_cast<{const} T*>. It's just a style fixing.

What changes are included in this PR?

Just api replacement for ::arrow::Buffer

• reinterpret_cast<T*> -> mutable_data_as<T>()
• reinterpret_cast<const T*> -> data_as<T>()

Also, for auto {variable_name} = reinterpret_cast<{mutable} T*>( ... ), I changed it to:

1. const auto* for data_as<T>().
2. auto* for mutable_data_as<T>()

This didn't change the syntax, but make it more readable.

Are these changes tested?

No need

Are there any user-facing changes?

no

• Closes: [C++][Parquet] Minor: Using arrow::Buffer "data_as" api to replace the reinterpret_cast #39419

[github-actions]

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

[mapleFU]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: f769b1c

Submitted crossbow builds: ursacomputing/crossbow @ actions-53d0242ebe

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-fedora-38-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-no-threading

[mapleFU]

cc @felipecrv would you mind take a look?

[kou]

+1

[kou]

Do we need to specify const and * explicitly here?

[mapleFU]

No, const auto* v = ... is equal to auto dict_values = ... It's just a style problem

Let me update it

[felipecrv]

I like auto* because it communicates the assignment is just a pointer assignment and not some expensive copy/move operation. And it's creates a good symmetry with auto& that requires the & otherwise it becomes a copy.

[mapleFU]

Updated: I just change auto to auto* and const auto*. I think they're no different, and * might be more readable for me.

Will wait some days to see if other like this style, glad to change it

[felipecrv]

LGTM. Including the const auto* and all.

[mapleFU]

Will merge it before Friday if no negative comments

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 01deb94.

There were 4 benchmark results indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2024-01-05 19:20:08Z
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-10, scale_factor=1
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-11, scale_factor=1
• and 2 more (see the report linked below)

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.