Expose page-level arrow reader API (#4298) #4307

tustvold · 2023-05-29T21:36:16Z

Which issue does this PR close?

Rationale for this change

This makes it possible to use the arrow conversion plumbing with custom logic for fetching the sourcing the underlying data pages.

What changes are included in this PR?

Are there any user-facing changes?

The only changes are to experimental APIs, so no breaking changes

tustvold · 2023-05-29T21:37:03Z

parquet/src/arrow/arrow_reader/mod.rs

+    ///
+    /// Note: this is a low-level interface see [`ParquetRecordBatchReader::try_new`] for a
+    /// higher-level interface for reading parquet data from a file
+    pub fn try_new_with_row_groups(


This lets you construct a ParquetRecordBatchReader from an arbitrary source of pages

tustvold · 2023-05-29T21:38:02Z

parquet/src/arrow/schema/mod.rs

+///
+/// Note: this is a low-level API, most users will want to make use of the higher-level
+/// [`parquet_to_arrow_schema`] for decoding metadata from a parquet file.
+pub fn parquet_to_arrow_field_levels(


This is the alternative to making parquet_to_array_schema_and_fields public, and does not expose the details of what ParquetField is.

tustvold · 2023-05-29T21:38:16Z

parquet/src/arrow/schema/mod.rs

 }

 /// Extracts the arrow metadata
-pub(crate) fn parquet_to_array_schema_and_fields(
+pub(crate) fn parquet_to_arrow_schema_and_fields(


I realised the name was a typo, so fixed it 😄

tustvold · 2023-05-29T21:38:36Z

@sundy-li could you perhaps take a look and see if this would suit your use-case

tustvold · 2023-05-29T21:39:25Z

parquet/src/arrow/arrow_reader/mod.rs

+        row_groups: &dyn RowGroups,
+        batch_size: usize,
+        selection: Option<RowSelection>,
+    ) -> Result<Self> {


We don't need to include a ProjectionMask as this can be passed to parquet_to_arrow_field_levels instead

sundy-li · 2023-05-30T00:45:56Z

Almost LGTM, how about exposing InMemoryRowGroup and scan_ranges of RowSelector ? Otherwise, we need a similar struct to work.

tustvold · 2023-05-30T07:49:17Z

They're fairly tightly coupled with how ParquetRecordBatchStream does IO, something you indicated you wished to handle differently. What do you think of implementing RowGroups for your custom in-memory representation?

sundy-li · 2023-05-30T09:31:16Z

What do you think of implementing RowGroups for your custom in-memory representation?

Yes, we would have our own RowGroups implementation but we also want to access the function of RowSelector to inject the page index, so how about making scan_ranges be public.

alamb

The code looks good to me -- thanks @sundy-li and @tustvold

I also suggest someone write up a tested example of using these new APIs so they don't get inadvertently broken during some future refactor

alamb · 2023-05-30T17:31:22Z

parquet/src/arrow/schema/mod.rs

+    Ok((schema, field_levels.levels))
+}
+
+/// Stores the parquet level information for a set of arrow [`Fields`]


I don't understand why the comment refers to parquet level information when the actual structure contains the an optional mapping to a ParquetField 🤔

Maybe the docs are out of date? Or perhaps mean to highlight the fact that the level information is stored in ParquetField?

alamb · 2023-05-30T17:32:01Z

parquet/src/arrow/schema/mod.rs

+/// Stores the parquet level information for a set of arrow [`Fields`]
+#[derive(Debug, Clone)]
+pub struct FieldLevels {
+    pub(crate) fields: Fields,


Is there any way to get access to fields and levels outside of the parquet crate? Should there be?

Maybe they need accessors 🤔 Or maybe the point is to pass it opaquely to try_new_with_row_groups? If so perhaps that is worth a comment

The intention of this container is to hide the internal notion of what is necessary to decode a field. Will clarify

alamb · 2023-05-30T17:33:15Z

parquet/src/arrow/schema/complex.rs

@@ -35,6 +35,7 @@ fn get_repetition(t: &Type) -> Repetition {
 }

 /// Representation of a parquet file, in terms of arrow schema elements


When reading this PR I see this comment and wonder "is it really a parquet file" or is it more like a "parquet field"? Or a "possibly nested field" 🤔

Reworded, I agree the use of file here is very misleading

alamb · 2023-05-30T17:34:16Z

parquet/src/arrow/arrow_reader/selection.rs

+    ///
+    /// This is useful for determining what byte ranges to fetch from underlying storage
+    ///
+    /// Note: this method does not make any effort to combine consecutive ranges, nor coalesce


sundy-li · 2023-05-30T23:53:52Z

I also suggest someone write up a tested example of using these new APIs so they don't get inadvertently broken during some future refactor

Sure, I can help later after this pr is merged.

alamb · 2023-05-31T12:02:30Z

parquet/src/arrow/schema/complex.rs

@@ -34,7 +34,7 @@ fn get_repetition(t: &Type) -> Repetition {
    }
 }

-/// Representation of a parquet file, in terms of arrow schema elements
+/// Representation of a parquet schema element, in terms of arrow schema elements


Expose page-level arrow reader API (apache#4298)

f03bd83

github-actions bot added the parquet Changes to the parquet crate label May 29, 2023

tustvold commented May 29, 2023

View reviewed changes

Make scan_ranges public

ad80d20

sundy-li approved these changes May 30, 2023

View reviewed changes

alamb approved these changes May 30, 2023

View reviewed changes

tustvold added 2 commits May 31, 2023 11:02

Review feedback

de965b9

Merge remote-tracking branch 'upstream/master' into page-level-api

7af197c

tustvold merged commit bbb47a6 into apache:master May 31, 2023

alamb reviewed May 31, 2023

View reviewed changes

sundy-li mentioned this pull request Jun 1, 2023

feat: add read parquet by custom rowgroup examples #4332

Merged

tustvold mentioned this pull request Jun 2, 2023

export function parquet_to_array_schema_and_fields #4298

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose page-level arrow reader API (#4298) #4307

Expose page-level arrow reader API (#4298) #4307

tustvold commented May 29, 2023

tustvold May 29, 2023

tustvold May 29, 2023

tustvold May 29, 2023

tustvold commented May 29, 2023

tustvold May 29, 2023

sundy-li commented May 30, 2023 •

edited

Loading

tustvold commented May 30, 2023

sundy-li commented May 30, 2023 •

edited

Loading

alamb left a comment

alamb May 30, 2023

alamb May 30, 2023

tustvold May 31, 2023

alamb May 30, 2023

tustvold May 31, 2023

alamb May 30, 2023

sundy-li commented May 30, 2023

alamb May 31, 2023

		@@ -35,6 +35,7 @@ fn get_repetition(t: &Type) -> Repetition {
		}

		/// Representation of a parquet file, in terms of arrow schema elements

Expose page-level arrow reader API (#4298) #4307

Expose page-level arrow reader API (#4298) #4307

Conversation

tustvold commented May 29, 2023

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tustvold commented May 29, 2023

Choose a reason for hiding this comment

sundy-li commented May 30, 2023 • edited Loading

tustvold commented May 30, 2023

sundy-li commented May 30, 2023 • edited Loading

alamb left a 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

sundy-li commented May 30, 2023

Choose a reason for hiding this comment

sundy-li commented May 30, 2023 •

edited

Loading

sundy-li commented May 30, 2023 •

edited

Loading