Support nested schema projection (#5148) #5149

tustvold · 2023-11-29T13:46:14Z

Which issue does this PR close?

Closes #5148.

Rationale for this change

See ticket

What changes are included in this PR?

Are there any user-facing changes?

tustvold · 2023-11-29T13:47:58Z

arrow-schema/src/fields.rs

+    /// ]);
+    /// assert_eq!(filtered, expected);
+    /// ```
+    pub fn filter_leaves<F: FnMut(usize, &FieldRef) -> bool>(&self, mut filter: F) -> Self {


Originally I had two methods, one which was called for all Fields, however, it wasn't clear to me how such an API could be used - as it would lack any context as to where in the schema a given field appeared. I therefore just stuck with filter_leaves which has immediate concrete utility for #5135. A case could be made for a fully-fledged visitor pattern, but I'm not aware of any immediate use-cases for this, and therefore what the requirements might be for such an API.

FYI @Jefffrey

I wonder how we would support selecting a subfield that is itself a field?

For example:

{ a : { b: { c: 1, }, d: 2 }

Could I use this API to support only selecting a.b?

{ a : { b: { c: 1, }, }

Maybe we could if the parent FieldRef was also passed

/// calls filter(depth, parent_field, child_field)` pub fn filter_leaves<F: FnMut(usize, &FieldRef, &FieldRef) -> bool>(&self, mut filter: F) -> Self { ... }

🤔

You would select b by selecting all its children, if you call filter with intermediate fields you end up in a position of how much context do you provide, just the parent, what about the parent's parent - reasoning in terms of leaves is the only coherent mechanism I can think of

tustvold · 2023-11-29T13:50:06Z

arrow-schema/src/fields.rs

@@ -99,6 +100,93 @@ impl Fields {
                .all(|(a, b)| Arc::ptr_eq(a, b) || a.contains(b))
    }

+    /// Performs a depth-first scan of [`Fields`] filtering the [`FieldRef`] with no children
+    ///
+    /// Invokes `filter` with each leaf [`FieldRef`], i.e. one containing no children, and a


I'm sure this could be expressed better, but I struggled to come up with something 😅

alamb

Thank you @tustvold -- this seems like a good API to me -- I tried to suggest some comment improvements and had some questions about making is more general, but I think since it is better than what currently exists, this is a nice step forward

alamb · 2023-11-29T18:52:05Z

arrow-schema/src/fields.rs

@@ -99,6 +100,93 @@ impl Fields {
                .all(|(a, b)| Arc::ptr_eq(a, b) || a.contains(b))
    }

+    /// Performs a depth-first scan of [`Fields`] filtering the [`FieldRef`] with no children


Here is a proposal of how to better phrase what this is doing (as well as give a usecase that might not be obvious to the casual reader)

Suggested change

/// Performs a depth-first scan of [`Fields`] filtering the [`FieldRef`] with no children

/// Returns a copy of this [`Fields`], with only those non nested (leaf) [`FieldRef`]s that

/// pass a predicate.

///

/// This can be used to select only a subset of fields in nested types such as

/// [`DataType::Struct`].

/// Leaf [`FieldRef`]s `DataType`s have no nested `FieldRef`s`. For example

/// a field with [`DatatType::Int32`] would be a leaf.

I incoporated this into 8d97950. I tweaked the wording a little as the depth-first part is critical to understanding what the leaf ordering means. I also added some comments to the doctest to highlight

Thank you -- the comments are most helpful

alamb · 2023-11-29T18:58:01Z

arrow-schema/src/fields.rs

+    /// Invokes `filter` with each leaf [`FieldRef`], i.e. one containing no children, and a
+    /// count of the number of previous calls to `filter` - i.e. the leaf's index.


Suggested change

/// Invokes `filter` with each leaf [`FieldRef`], i.e. one containing no children, and a

/// count of the number of previous calls to `filter` - i.e. the leaf's index.

/// Invokes `filter` with each leaf [`FieldRef`] and a

/// count of the depth to the `field` (i.e. the number of previous calls to `filter`, and the leaf's index.)

This isn't correct, it is the count of leaves encountered, not the depth

I tried to clarify this in 8d97950

alamb · 2023-11-29T19:00:42Z

arrow-schema/src/fields.rs

+    /// ]);
+    /// assert_eq!(filtered, expected);
+    /// ```
+    pub fn filter_leaves<F: FnMut(usize, &FieldRef) -> bool>(&self, mut filter: F) -> Self {


I wonder how we would support selecting a subfield that is itself a field?

For example:

{ a : { b: { c: 1, }, d: 2 }

Could I use this API to support only selecting a.b?

{ a : { b: { c: 1, }, }

Maybe we could if the parent FieldRef was also passed

/// calls filter(depth, parent_field, child_field)` pub fn filter_leaves<F: FnMut(usize, &FieldRef, &FieldRef) -> bool>(&self, mut filter: F) -> Self { ... }

🤔

alamb · 2023-11-29T19:04:27Z

arrow-schema/src/fields.rs

+                Dictionary(k, v) => (Some(k.clone()), v.as_ref()),
+                d => (None, d),
+            };
+            let d = match v {


Map and RunEndEncoded appear to have embedded fields too. It seems like they might also need to be handled 🤔

The usecase might be "I want only the values of the map? Or maybe Arrow can't express that concept (Apply the filter to only the values 🤔 )

Map is there, I missed RunEndEncoded 👍

RunEndEncoded support added in 8d97950

alamb · 2023-11-29T19:04:35Z

arrow-schema/src/fields.rs

+            use DataType::*;
+
+            let (k, v) = match f.data_type() {
+                Dictionary(k, v) => (Some(k.clone()), v.as_ref()),


this presumes the key itself doesn't have any leaves which I think is reasonable, but figured I would point it out.

Added comments in 8d97950

alamb · 2023-11-29T21:44:54Z

Looks very nice @tustvold -- thank you

alamb · 2023-11-29T21:46:02Z

One question I still have is does this API support the usecase in #5149 (comment) (selecting a nested subfield)? If not, I can file a ticket tracking that feature request

tustvold · 2023-11-29T21:56:08Z

One question I still have is does this API support the usecase in #5149 (comment) (selecting a nested subfield)?

Yes, I responded to the comment as such?

alamb · 2023-11-30T00:42:08Z

One question I still have is does this API support the usecase in #5149 (comment) (selecting a nested subfield)?

Yes, I responded to the comment as such?

Sorry -- the github UI is confusing me. I see the response in #5149 (comment) now. Thanks

Support nested schema projection

b84408b

github-actions bot added the arrow Changes to the arrow crate label Nov 29, 2023

tustvold commented Nov 29, 2023

View reviewed changes

Tweak doc

0aab1d1

tustvold commented Nov 29, 2023

View reviewed changes

alamb mentioned this pull request Nov 29, 2023

DataFusion weekly project plan (Andrew Lamb) - Nov 27, 2023 apache/datafusion#8329

Closed

8 tasks

alamb approved these changes Nov 29, 2023

View reviewed changes

Review feedback

8d97950

tustvold merged commit 6d4b8bb into apache:master Nov 29, 2023
26 checks passed

Jefffrey mentioned this pull request Nov 30, 2023

Parquet: clear metadata and project fields of ParquetRecordBatchStream::schema #5135

Merged

tustvold mentioned this pull request Jan 5, 2024

Nested Schema Projection #5148

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support nested schema projection (#5148) #5149

Support nested schema projection (#5148) #5149

tustvold commented Nov 29, 2023

tustvold Nov 29, 2023 •

edited

Loading

alamb Nov 29, 2023

tustvold Nov 29, 2023 •

edited

Loading

tustvold Nov 29, 2023

alamb left a comment

alamb Nov 29, 2023

tustvold Nov 29, 2023

alamb Nov 29, 2023

alamb Nov 29, 2023

tustvold Nov 29, 2023

tustvold Nov 29, 2023

alamb Nov 29, 2023

alamb Nov 29, 2023

tustvold Nov 29, 2023

tustvold Nov 29, 2023

alamb Nov 29, 2023

tustvold Nov 29, 2023

alamb commented Nov 29, 2023

alamb commented Nov 29, 2023 •

edited

Loading

tustvold commented Nov 29, 2023

alamb commented Nov 30, 2023

-    /// Performs a depth-first scan of [`Fields`] filtering the [`FieldRef`] with no children
+    /// Returns a copy of this [`Fields`], with only those non nested (leaf) [`FieldRef`]s that
+    /// pass a predicate.
+    ///
+    /// This can be used to select only a subset of fields in nested types such as
+    /// [`DataType::Struct`].
+    /// Leaf [`FieldRef`]s `DataType`s have no nested `FieldRef`s`. For example
+    /// a field with [`DatatType::Int32`] would be a leaf.

		/// Invokes `filter` with each leaf [`FieldRef`], i.e. one containing no children, and a
		/// count of the number of previous calls to `filter` - i.e. the leaf's index.

Support nested schema projection (#5148) #5149

Support nested schema projection (#5148) #5149

Conversation

tustvold commented Nov 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?

tustvold Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tustvold Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

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

alamb commented Nov 29, 2023

alamb commented Nov 29, 2023 • edited Loading

tustvold commented Nov 29, 2023

alamb commented Nov 30, 2023

tustvold Nov 29, 2023 •

edited

Loading

tustvold Nov 29, 2023 •

edited

Loading

alamb commented Nov 29, 2023 •

edited

Loading