Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a365b61
add uuid as opt in
paleolimbot Mar 20, 2025
4cee020
convert in other direction
paleolimbot Mar 20, 2025
3d1a4ec
one more convert change
paleolimbot Mar 20, 2025
dc696cf
enable arrow_extensions_enabled in pyarrow
paleolimbot Mar 24, 2025
300f5eb
Add Python test
paleolimbot Mar 24, 2025
af8227c
Add C++ test
paleolimbot Mar 24, 2025
bd9c9a9
format
paleolimbot Mar 24, 2025
f187586
fix comment that was lying
paleolimbot Mar 24, 2025
d11fcd8
clean up schema.cc
paleolimbot Mar 24, 2025
f7a965e
fix python lint
paleolimbot Mar 24, 2025
b4d0730
apply improvement to arrow_extensions_enabled
paleolimbot Mar 31, 2025
2193fd4
Check physical length before inferring Arrow UUID type
paleolimbot Mar 31, 2025
a81d5c1
fix test comments in arrow_schema_test.cc
paleolimbot Mar 31, 2025
62f92b3
Attempt to reduce duplication in extension type restoration
paleolimbot Mar 31, 2025
d1b37e8
remove unused header
paleolimbot Apr 1, 2025
4bd607b
support nested extension components
paleolimbot Apr 1, 2025
7529947
remove unneeded branch
paleolimbot Apr 1, 2025
807a48f
Merge branch 'main' into parquet-uuid
paleolimbot Apr 4, 2025
ced4d6b
Merge branch 'main' into parquet-uuid
paleolimbot Apr 10, 2025
d2d98f2
fix merge
paleolimbot Apr 10, 2025
f3894b8
Update cpp/src/parquet/arrow/arrow_schema_test.cc
paleolimbot Apr 18, 2025
a101c8b
Update cpp/src/parquet/arrow/arrow_schema_test.cc
paleolimbot Apr 18, 2025
4c66a75
Update cpp/src/parquet/arrow/schema_internal.cc
paleolimbot Apr 18, 2025
234475e
Update cpp/src/parquet/arrow/schema.cc
paleolimbot Apr 18, 2025
8e8d5ee
Update cpp/src/parquet/arrow/arrow_schema_test.cc
paleolimbot Apr 18, 2025
26afc40
Update cpp/src/parquet/arrow/arrow_schema_test.cc
paleolimbot Apr 18, 2025
4b63ec7
clang-format
paleolimbot Apr 18, 2025
b48f7f5
move supported storage type test to UuidType, move common code out of…
paleolimbot Apr 18, 2025
555ed75
maybe simplify it all again
paleolimbot Apr 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cpp/src/arrow/dataset/file_parquet.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ parquet::ArrowReaderProperties MakeArrowReaderProperties(
arrow_properties.set_io_context(
parquet_scan_options.arrow_reader_properties->io_context());
arrow_properties.set_use_threads(options.use_threads);
arrow_properties.set_arrow_extensions_enabled(
parquet_scan_options.arrow_reader_properties->get_arrow_extensions_enabled());
return arrow_properties;
}

Expand Down
65 changes: 64 additions & 1 deletion cpp/src/parquet/arrow/arrow_schema_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

#include "arrow/array.h"
#include "arrow/extension/json.h"
#include "arrow/extension/uuid.h"
#include "arrow/ipc/writer.h"
#include "arrow/testing/gtest_util.h"
#include "arrow/type.h"
Expand Down Expand Up @@ -866,7 +867,7 @@ Status ArrowSchemaToParquetMetadata(std::shared_ptr<::arrow::Schema>& arrow_sche
return Status::OK();
}

TEST_F(TestConvertParquetSchema, ParquetSchemaArrowExtensions) {
TEST_F(TestConvertParquetSchema, ParquetSchemaArrowJsonExtension) {
std::vector<NodePtr> parquet_fields;
parquet_fields.push_back(PrimitiveNode::Make(
"json_1", Repetition::OPTIONAL, ParquetType::BYTE_ARRAY, ConvertedType::JSON));
Expand Down Expand Up @@ -948,6 +949,68 @@ TEST_F(TestConvertParquetSchema, ParquetSchemaArrowExtensions) {
}
}

TEST_F(TestConvertParquetSchema, ParquetSchemaArrowUuidExtension) {
std::vector<NodePtr> parquet_fields;
parquet_fields.push_back(PrimitiveNode::Make("uuid", Repetition::OPTIONAL,
LogicalType::UUID(),
ParquetType::FIXED_LEN_BYTE_ARRAY, 16));

{
// Parquet file does not contain Arrow schema.
// By default, field should be treated as fixed_size_binary(16) in Arrow.
auto arrow_schema =
::arrow::schema({::arrow::field("uuid", ::arrow::fixed_size_binary(16), true)});
Comment thread
paleolimbot marked this conversation as resolved.
Outdated
std::shared_ptr<KeyValueMetadata> metadata{};
ASSERT_OK(ConvertSchema(parquet_fields, metadata));
CheckFlatSchema(arrow_schema);
}

{
// Parquet file does not contain Arrow schema.
// If Arrow extensions are enabled, field will be interpreted as uuid()
// extension field.
ArrowReaderProperties props;
props.set_arrow_extensions_enabled(true);
auto arrow_schema =
::arrow::schema({::arrow::field("uuid", ::arrow::extension::uuid(), true)});
Comment thread
paleolimbot marked this conversation as resolved.
Outdated
std::shared_ptr<KeyValueMetadata> metadata{};
ASSERT_OK(ConvertSchema(parquet_fields, metadata, props));
CheckFlatSchema(arrow_schema);
}

{
// Parquet file contains Arrow schema.
// uuid will be interpreted as uuid() field
ArrowReaderProperties props;
props.set_arrow_extensions_enabled(false);
std::shared_ptr<KeyValueMetadata> field_metadata =
Comment thread
wgtmac marked this conversation as resolved.
::arrow::key_value_metadata({"foo", "bar"}, {"biz", "baz"});
auto arrow_schema = ::arrow::schema(
{::arrow::field("uuid", ::arrow::extension::uuid(), true, field_metadata)});
Comment thread
paleolimbot marked this conversation as resolved.
Outdated

std::shared_ptr<KeyValueMetadata> metadata;
ASSERT_OK(ArrowSchemaToParquetMetadata(arrow_schema, metadata));
ASSERT_OK(ConvertSchema(parquet_fields, metadata, props));
CheckFlatSchema(arrow_schema, true /* check_metadata */);
}

{
// Parquet file contains Arrow schema.
// uuid will be interpreted as uuid() field even though extensions are not enabled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are below.

ArrowReaderProperties props;
props.set_arrow_extensions_enabled(true);
std::shared_ptr<KeyValueMetadata> field_metadata =
::arrow::key_value_metadata({"foo", "bar"}, {"biz", "baz"});
auto arrow_schema = ::arrow::schema(
{::arrow::field("uuid", ::arrow::extension::uuid(), true, field_metadata)});
Comment thread
paleolimbot marked this conversation as resolved.
Outdated

std::shared_ptr<KeyValueMetadata> metadata;
ASSERT_OK(ArrowSchemaToParquetMetadata(arrow_schema, metadata));
ASSERT_OK(ConvertSchema(parquet_fields, metadata, props));
CheckFlatSchema(arrow_schema, true /* check_metadata */);
}
}

class TestConvertArrowSchema : public ::testing::Test {
public:
virtual void SetUp() {}
Expand Down
30 changes: 30 additions & 0 deletions cpp/src/parquet/arrow/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <vector>

#include "arrow/extension/json.h"
#include "arrow/extension/uuid.h"
#include "arrow/extension_type.h"
#include "arrow/io/memory.h"
#include "arrow/ipc/api.h"
Expand Down Expand Up @@ -434,7 +435,13 @@ Status FieldToNode(const std::string& name, const std::shared_ptr<Field>& field,
type = ParquetType::BYTE_ARRAY;
logical_type = LogicalType::JSON();
break;
} else if (ext_type->extension_name() == std::string("arrow.uuid")) {
type = ParquetType::FIXED_LEN_BYTE_ARRAY;
logical_type = LogicalType::UUID();
length = 16;
break;
}

std::shared_ptr<::arrow::Field> storage_field = ::arrow::field(
name, ext_type->storage_type(), field->nullable(), field->metadata());
return FieldToNode(name, storage_field, properties, arrow_properties, out);
Expand Down Expand Up @@ -1053,6 +1060,29 @@ Result<bool> ApplyOriginalMetadata(const Field& origin_field, SchemaField* infer
// inferred_type is arrow::extension::json(arrow::utf8())
auto origin_storage_field = origin_field.WithType(ex_type.storage_type());

// Apply metadata recursively to storage type
RETURN_NOT_OK(ApplyOriginalStorageMetadata(*origin_storage_field, inferred));
inferred->field = inferred->field->WithType(origin_type);
} else if (inferred_type->id() == ::arrow::Type::FIXED_SIZE_BINARY &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These branches are growing longer while they look pretty similar. Not sure if we can refactor it a little bit to look nicer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent quite a bit of time rewriting the logic here...I think it's better than it was?

ex_type.extension_name() == std::string("arrow.uuid")) {
// Schema mismatch.
//
// Arrow extensions are DISABLED in Parquet.
// origin_type is ::arrow::extension::uuid()
// inferred_type is ::arrow::fixed_size_binary()
//
// Origin type is restored as Arrow should be considered the source of truth.
inferred->field = inferred->field->WithType(origin_type);
RETURN_NOT_OK(ApplyOriginalStorageMetadata(origin_field, inferred));
} else if (inferred_type->id() == ::arrow::Type::EXTENSION &&
ex_type.extension_name() == std::string("arrow.uuid")) {
// Schema match.
//
// Arrow extensions are ENABLED in Parquet.
// origin_type is arrow::extension::uuid()
// inferred_type is arrow::extension::uuid()
auto origin_storage_field = origin_field.WithType(ex_type.storage_type());

// Apply metadata recursively to storage type
RETURN_NOT_OK(ApplyOriginalStorageMetadata(*origin_storage_field, inferred));
inferred->field = inferred->field->WithType(origin_type);
Expand Down
13 changes: 10 additions & 3 deletions cpp/src/parquet/arrow/schema_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "parquet/arrow/schema_internal.h"

#include "arrow/extension/json.h"
#include "arrow/extension/uuid.h"
#include "arrow/type.h"

#include "parquet/properties.h"
Expand Down Expand Up @@ -134,16 +135,22 @@ Result<std::shared_ptr<ArrowType>> FromByteArray(
}
}

Result<std::shared_ptr<ArrowType>> FromFLBA(const LogicalType& logical_type,
int32_t physical_length) {
Result<std::shared_ptr<ArrowType>> FromFLBA(
const LogicalType& logical_type, int32_t physical_length,
const ArrowReaderProperties& reader_properties) {
switch (logical_type.type()) {
case LogicalType::Type::DECIMAL:
return MakeArrowDecimal(logical_type);
case LogicalType::Type::FLOAT16:
return ::arrow::float16();
case LogicalType::Type::NONE:
case LogicalType::Type::INTERVAL:
return ::arrow::fixed_size_binary(physical_length);
case LogicalType::Type::UUID:
if (reader_properties.get_arrow_extensions_enabled()) {
return ::arrow::extension::uuid();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check physical_length here?

}

return ::arrow::fixed_size_binary(physical_length);
default:
return Status::NotImplemented("Unhandled logical logical_type ",
Comment thread
paleolimbot marked this conversation as resolved.
Outdated
Expand Down Expand Up @@ -211,7 +218,7 @@ Result<std::shared_ptr<ArrowType>> GetArrowType(
case ParquetType::BYTE_ARRAY:
return FromByteArray(logical_type, reader_properties);
case ParquetType::FIXED_LEN_BYTE_ARRAY:
return FromFLBA(logical_type, type_length);
return FromFLBA(logical_type, type_length, reader_properties);
default: {
// PARQUET-1565: This can occur if the file is corrupt
return Status::IOError("Invalid physical column type: ",
Expand Down
25 changes: 20 additions & 5 deletions python/pyarrow/_dataset_parquet.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
cache_options : pyarrow.CacheOptions, default None
Cache options used when pre_buffer is enabled. The default values should
be good for most use cases. You may want to adjust these for example if
you have exceptionally high latency to the file system.
you have exceptionally high latency to the file system.
thrift_string_size_limit : int, default None
If not None, override the maximum total string size allocated
when decoding Thrift structures. The default limit should be
Expand All @@ -720,6 +720,9 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
Parquet file.
page_checksum_verification : bool, default False
If True, verify the page checksum for each page read from the file.
arrow_extensions_enabled : bool, default False
If True, read Parquet logical types as Arrow Extension Types where possible,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments I posted on this on the Geometry PR.

(e.g., JSON as arrow.json or UUID as arrow.uuid).
"""

# Avoid mistakingly creating attributes
Expand All @@ -733,7 +736,8 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
thrift_container_size_limit=None,
decryption_config=None,
decryption_properties=None,
bint page_checksum_verification=False):
bint page_checksum_verification=False,
bint arrow_extensions_enabled=False):
self.init(shared_ptr[CFragmentScanOptions](
new CParquetFragmentScanOptions()))
self.use_buffered_stream = use_buffered_stream
Expand All @@ -752,6 +756,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
if decryption_properties is not None:
self.decryption_properties = decryption_properties
self.page_checksum_verification = page_checksum_verification
self.arrow_extensions_enabled = arrow_extensions_enabled

cdef void init(self, const shared_ptr[CFragmentScanOptions]& sp):
FragmentScanOptions.init(self, sp)
Expand Down Expand Up @@ -868,6 +873,14 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
def page_checksum_verification(self, bint page_checksum_verification):
self.reader_properties().set_page_checksum_verification(page_checksum_verification)

@property
def arrow_extensions_enabled(self):
return self.arrow_reader_properties().get_arrow_extensions_enabled()

@arrow_extensions_enabled.setter
def arrow_extensions_enabled(self, bint arrow_extensions_enabled):
self.arrow_reader_properties().set_arrow_extensions_enabled(arrow_extensions_enabled)

def equals(self, ParquetFragmentScanOptions other):
"""
Parameters
Expand All @@ -881,11 +894,12 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
attrs = (
self.use_buffered_stream, self.buffer_size, self.pre_buffer, self.cache_options,
self.thrift_string_size_limit, self.thrift_container_size_limit,
self.page_checksum_verification)
self.page_checksum_verification, self.arrow_extensions_enabled)
other_attrs = (
other.use_buffered_stream, other.buffer_size, other.pre_buffer, other.cache_options,
other.thrift_string_size_limit,
other.thrift_container_size_limit, other.page_checksum_verification)
other.thrift_container_size_limit, other.page_checksum_verification,
other.arrow_extensions_enabled)
return attrs == other_attrs

@staticmethod
Expand All @@ -902,7 +916,8 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
cache_options=self.cache_options,
thrift_string_size_limit=self.thrift_string_size_limit,
thrift_container_size_limit=self.thrift_container_size_limit,
page_checksum_verification=self.page_checksum_verification
page_checksum_verification=self.page_checksum_verification,
arrow_extensions_enabled=self.arrow_extensions_enabled
)
return ParquetFragmentScanOptions._reconstruct, (kwargs,)

Expand Down
2 changes: 2 additions & 0 deletions python/pyarrow/_parquet.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,8 @@ cdef extern from "parquet/api/reader.h" namespace "parquet" nogil:
CCacheOptions cache_options() const
void set_coerce_int96_timestamp_unit(TimeUnit unit)
TimeUnit coerce_int96_timestamp_unit() const
void set_arrow_extensions_enabled(c_bool extensions_enabled)
c_bool get_arrow_extensions_enabled() const

ArrowReaderProperties default_arrow_reader_properties()

Expand Down
6 changes: 5 additions & 1 deletion python/pyarrow/_parquet.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,8 @@ cdef class ParquetReader(_Weakrefable):
FileDecryptionProperties decryption_properties=None,
thrift_string_size_limit=None,
thrift_container_size_limit=None,
page_checksum_verification=False):
page_checksum_verification=False,
arrow_extensions_enabled=False):
"""
Open a parquet file for reading.

Expand All @@ -1471,6 +1472,7 @@ cdef class ParquetReader(_Weakrefable):
thrift_string_size_limit : int, optional
thrift_container_size_limit : int, optional
page_checksum_verification : bool, default False
arrow_extensions_enabled : bool, default False
"""
cdef:
shared_ptr[CFileMetaData] c_metadata
Expand Down Expand Up @@ -1520,6 +1522,8 @@ cdef class ParquetReader(_Weakrefable):
arrow_props.set_coerce_int96_timestamp_unit(
string_to_timeunit(coerce_int96_timestamp_unit))

arrow_props.set_arrow_extensions_enabled(arrow_extensions_enabled)

self.source = source
get_reader(source, use_memory_map, &self.rd_handle)

Expand Down
20 changes: 17 additions & 3 deletions python/pyarrow/parquet/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ class ParquetFile:
it will be parsed as an URI to determine the filesystem.
page_checksum_verification : bool, default False
If True, verify the checksum for each page read from the file.
arrow_extensions_enabled : bool, default False
If True, read Parquet logical types as Arrow Extension Types where possible,
(e.g., JSON as arrow.json or UUID as arrow.uuid).

Examples
--------
Expand Down Expand Up @@ -302,7 +305,7 @@ def __init__(self, source, *, metadata=None, common_metadata=None,
pre_buffer=False, coerce_int96_timestamp_unit=None,
decryption_properties=None, thrift_string_size_limit=None,
thrift_container_size_limit=None, filesystem=None,
page_checksum_verification=False):
page_checksum_verification=False, arrow_extensions_enabled=False):

self._close_source = getattr(source, 'closed', True)

Expand All @@ -322,6 +325,7 @@ def __init__(self, source, *, metadata=None, common_metadata=None,
thrift_string_size_limit=thrift_string_size_limit,
thrift_container_size_limit=thrift_container_size_limit,
page_checksum_verification=page_checksum_verification,
arrow_extensions_enabled=arrow_extensions_enabled,
)
self.common_metadata = common_metadata
self._nested_paths_by_prefix = self._build_nested_paths()
Expand Down Expand Up @@ -1264,6 +1268,9 @@ class ParquetDataset:
sufficient for most Parquet files.
page_checksum_verification : bool, default False
If True, verify the page checksum for each page read from the file.
arrow_extensions_enabled : bool, default False
If True, read Parquet logical types as Arrow Extension Types where possible,
(e.g., JSON as arrow.json or UUID as arrow.uuid).

Examples
--------
Expand All @@ -1276,7 +1283,8 @@ def __init__(self, path_or_paths, filesystem=None, schema=None, *, filters=None,
coerce_int96_timestamp_unit=None,
decryption_properties=None, thrift_string_size_limit=None,
thrift_container_size_limit=None,
page_checksum_verification=False):
page_checksum_verification=False,
arrow_extensions_enabled=False):

import pyarrow.dataset as ds

Expand All @@ -1287,6 +1295,7 @@ def __init__(self, path_or_paths, filesystem=None, schema=None, *, filters=None,
"thrift_string_size_limit": thrift_string_size_limit,
"thrift_container_size_limit": thrift_container_size_limit,
"page_checksum_verification": page_checksum_verification,
"arrow_extensions_enabled": arrow_extensions_enabled,
}
if buffer_size:
read_options.update(use_buffered_stream=True,
Expand Down Expand Up @@ -1674,6 +1683,9 @@ def partitioning(self):
sufficient for most Parquet files.
page_checksum_verification : bool, default False
If True, verify the checksum for each page read from the file.
arrow_extensions_enabled : bool, default False
If True, read Parquet logical types as Arrow Extension Types where possible,
(e.g., JSON as arrow.json or UUID as arrow.uuid).

Returns
-------
Expand Down Expand Up @@ -1768,7 +1780,8 @@ def read_table(source, *, columns=None, use_threads=True,
pre_buffer=True, coerce_int96_timestamp_unit=None,
decryption_properties=None, thrift_string_size_limit=None,
thrift_container_size_limit=None,
page_checksum_verification=False):
page_checksum_verification=False,
arrow_extensions_enabled=False):

try:
dataset = ParquetDataset(
Expand All @@ -1787,6 +1800,7 @@ def read_table(source, *, columns=None, use_threads=True,
thrift_string_size_limit=thrift_string_size_limit,
thrift_container_size_limit=thrift_container_size_limit,
page_checksum_verification=page_checksum_verification,
arrow_extensions_enabled=arrow_extensions_enabled,
)
except ImportError:
# fall back on ParquetFile for simple cases when pyarrow.dataset
Expand Down
Loading