ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0 #11031

pitrou · 2021-08-30T15:43:13Z

Introduce PARQUET_VERSION_2_4 and PARQUET_VERSION_2_6 which allow fine-grained selection of post-1.0 Parquet format features.

github-actions · 2021-08-30T15:43:35Z

https://issues.apache.org/jira/browse/ARROW-13794

cpp/src/parquet/column_writer.cc

emkornfield · 2021-08-31T03:48:49Z

python/pyarrow/_parquet.pxd

-        ParquetVersion_V2" parquet::ParquetVersion::PARQUET_2_0"
+        ParquetVersion_V2_0" parquet::ParquetVersion::PARQUET_2_0"
+        ParquetVersion_V2_4" parquet::ParquetVersion::PARQUET_2_4"
+        ParquetVersion_V2_6" parquet::ParquetVersion::PARQUET_2_6"


should latest be exposed?

For now it's not being used in the PyArrow code. We can expose it when needed.

From a user point of view, would it be useful to have this though? That you can specify version="latest" or version="2.x" or something to always get the latest version? (although that might not be good practice to do anyway)

I don't know. As you say, it may not be good practice. If people need features from a specific version, they should probably use that version?

We can always add this later, in case there is demand for it / a good use case.

emkornfield · 2021-08-31T03:51:02Z

A few small nits but LGTM. Thanks for cleaning this up.

Introduce PARQUET_VERSION_2_4 and PARQUET_VERSION_2_6 which allow fine-grained selection of post-1.0 Parquet format features.

pitrou · 2021-08-31T09:52:20Z

@jorisvandenbossche Can you check the Python changes here?

jorisvandenbossche

Thanks a lot for doing this!
Mostly some minor docstring comments.

jorisvandenbossche · 2021-08-31T09:36:37Z

cpp/src/parquet/metadata.h

+  ///
+  /// The Parquet file metadata stores the version as a single integer,
+  /// therefore the returned value can only be an approximation of the
+  /// Parquet format version actually supported by the producer of the file.


Sidenote: It's also not very clear in general what this field is for (cfr the core features discussion at apache/parquet-format#164 (comment), parquet-mr always uses 1)

Wow. Thanks for the pointer. We may want to deprecate this API as a separate JIRA, then...

jorisvandenbossche · 2021-08-31T09:46:07Z

cpp/src/parquet/type_fwd.h

+    /// Enable only pre-2.0 Parquet format features when writing
+    ///
+    /// This setting is useful for maximum compatibility with legacy readers.
+    PARQUET_1_0,


I don't know into what detail we want to go here, but this is not "exactly" correct that it only enables pre-2.0 features. We currently already do write for example TIMESTAMP_MILLIS with version="1.0", although strictly speaking this was only introduced in Parquet format 2.2. I think it are only the several integer logical types that we don't write with version 1.0 (as those can be interpreted incorrectly if you don't understand the type annotation, since they are physically stored as int32 or int64; which is what matters most for compatibility with older readers).

So I think from a practical point of view, you can ignore the above nitpick :)

Right. The problem is that, as for PARQUET_2_0, the set of features enabled does not map to a well-defined version. That said, I may write "pre-2.2" instead.

TIMESTAMP_MILLIS is actually only from 2.2, so "pre 2.2" doesn't help that much ... (but, yeah, this is a mess :))

jorisvandenbossche · 2021-08-31T09:54:32Z

python/pyarrow/parquet.py

-    case ``allow_truncated_timestamps=True`` can be used to suppress the
-    raised exception.
+    microseconds ('us'), and seconds to milliseconds ('ms') by default.
+    For ``version='2.4'``, nanoseconds will be cast to microseconds.


This didn't change compared to version=1.0 (the sentence before)?

Ah, you're right. Parquet doesn't have seconds.

jorisvandenbossche · 2021-08-31T09:55:04Z

python/pyarrow/parquet.py

-    raised exception.
+    microseconds ('us'), and seconds to milliseconds ('ms') by default.
+    For ``version='2.4'``, nanoseconds will be cast to microseconds.
+    For ``version='2.6'``, the original resolution is always preserved.


Maybe clarify that this sentence is only about nanoseconds? (I think seconds are still cast to milliseconds?)

Indeed, seconds are still cast.

jorisvandenbossche · 2021-08-31T09:58:24Z

python/pyarrow/_parquet.pxd

-        ParquetVersion_V2" parquet::ParquetVersion::PARQUET_2_0"
+        ParquetVersion_V2_0" parquet::ParquetVersion::PARQUET_2_0"
+        ParquetVersion_V2_4" parquet::ParquetVersion::PARQUET_2_4"
+        ParquetVersion_V2_6" parquet::ParquetVersion::PARQUET_2_6"


From a user point of view, would it be useful to have this though? That you can specify version="latest" or version="2.x" or something to always get the latest version? (although that might not be good practice to do anyway)

jorisvandenbossche · 2021-08-31T10:07:23Z

python/pyarrow/parquet.py

+    Files written with version='2.4' or '2.6' may not be readable in all
+    Parquet implementations, so version='1.0' is likely the choice that
+    maximizes file compatibility.
+    Logical types and UINT32 are only available with version '2.4'.


We actually write logical types for version 1.0 ..

Hmm, thanks. This is a mess.

pitrou · 2021-08-31T12:14:43Z

@jorisvandenbossche Could you take a look at the updated wording?

jorisvandenbossche · 2021-08-31T12:35:07Z

python/pyarrow/parquet.py

+    of resolution.  Seconds are always cast to milliseconds ('ms') by default,
+    as Parquet does not have any temporal type with seconds resolution.
+    If the casting results in loss of data, it will raise an exception
+    unless ``allow_truncated_timestamps=True`` is given.


Very clear now!

Introduce PARQUET_VERSION_2_4 and PARQUET_VERSION_2_6 which allow fine-grained selection of post-1.0 Parquet format features. Closes apache#11031 from pitrou/ARROW-13794-parquet-version Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>

pitrou requested review from jorisvandenbossche and emkornfield August 30, 2021 15:43

github-actions bot added Component: C++ Component: Python Component: Parquet labels Aug 30, 2021

emkornfield reviewed Aug 31, 2021

View reviewed changes

cpp/src/parquet/column_writer.cc Outdated Show resolved Hide resolved

emkornfield reviewed Aug 31, 2021

View reviewed changes

pitrou added 3 commits August 31, 2021 10:17

ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0

dcb8ac7

Introduce PARQUET_VERSION_2_4 and PARQUET_VERSION_2_6 which allow fine-grained selection of post-1.0 Parquet format features.

Try to fix MSVC compile error

bb4a0b0

Spell out type explicitly

279778a

pitrou force-pushed the ARROW-13794-parquet-version branch from cb54ed6 to 279778a Compare August 31, 2021 08:21

Add missing header for boost to squash unrelated compile error

4a6c5f6

github-actions bot added the Component: FlightRPC label Aug 31, 2021

pitrou removed the Component: FlightRPC label Aug 31, 2021

jorisvandenbossche reviewed Aug 31, 2021

View reviewed changes

Address review comments

1ac6878

github-actions bot added the Component: FlightRPC label Aug 31, 2021

jorisvandenbossche reviewed Aug 31, 2021

View reviewed changes

jorisvandenbossche approved these changes Aug 31, 2021

View reviewed changes

pitrou closed this in 542e81b Aug 31, 2021

pitrou deleted the ARROW-13794-parquet-version branch August 31, 2021 12:51

emkornfield mentioned this pull request Sep 1, 2021

ARROW-13646: [Go][Parquet] adding the parquet metadata package #10951

Closed

asfimport mentioned this pull request Sep 1, 2021

[C++] Deprecate Parquet pseudo-version "2.0" #29420

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0 #11031

ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0 #11031

pitrou commented Aug 30, 2021

github-actions bot commented Aug 30, 2021

emkornfield Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

emkornfield commented Aug 31, 2021

pitrou commented Aug 31, 2021

jorisvandenbossche left a comment •

edited

Loading

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

jorisvandenbossche Aug 31, 2021

jorisvandenbossche Aug 31, 2021

pitrou Aug 31, 2021

pitrou commented Aug 31, 2021

jorisvandenbossche Aug 31, 2021

ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0 #11031

ARROW-13794: [C++] Deprecate PARQUET_VERSION_2_0 #11031

Conversation

pitrou commented Aug 30, 2021

github-actions bot commented Aug 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emkornfield commented Aug 31, 2021

pitrou commented Aug 31, 2021

jorisvandenbossche left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pitrou commented Aug 31, 2021

Choose a reason for hiding this comment

jorisvandenbossche left a comment •

edited

Loading