You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/08/07 17:28:42 UTC

[GitHub] [arrow-datafusion] alamb opened a new pull request, #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

alamb opened a new pull request, #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218

   ## Which issue does this PR close?
   
   Related to https://github.com/apache/arrow-datafusion/issues/7193
   Follows https://github.com/apache/arrow-datafusion/pull/7215 from @izveigor 
   
   ## Rationale for this change
   In order to have a good experience accessing fields, it should be easy to create field / lists accesses
   
   ## What changes are included in this PR?
   
   See subject
   ## Are these changes tested?
   Yes -- with doc tests 
   ## Are there any user-facing changes?
   New methods on `Expr`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218#issuecomment-1668465516

   Hitting https://github.com/apache/arrow-datafusion/issues/7221


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218#discussion_r1286191009


##########
datafusion/sql/src/expr/identifier.rs:
##########
@@ -136,12 +136,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                         )));
                     }
                     let nested_name = nested_names[0].to_string();
-                    Ok(Expr::GetIndexedField(GetIndexedField::new(
-                        Box::new(Expr::Column(field.qualified_column())),
-                        GetFieldAccess::NamedStructField {
-                            name: ScalarValue::Utf8(Some(nested_name)),
-                        },
-                    )))
+                    Ok(Expr::Column(field.qualified_column()).field(nested_name))

Review Comment:
   This shows the kind of API cleanup that is possible using this new API



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb merged pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] izveigor commented on a diff in pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "izveigor (via GitHub)" <gi...@apache.org>.
izveigor commented on code in PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218#discussion_r1286704664


##########
datafusion/expr/src/expr.rs:
##########
@@ -359,19 +359,20 @@ impl ScalarUDF {
     }
 }
 
+/// Access a sub field of a nested type, such as `Field` or `List`
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub enum GetFieldAccess {
-    /// returns the field `struct[field]`. For example `struct["name"]`
+    /// Named field, For example `struct["name"]`

Review Comment:
   ```suggestion
       /// Named field, for example `struct["name"]`
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218#discussion_r1286962622


##########
datafusion/expr/src/expr.rs:
##########
@@ -115,8 +115,8 @@ pub enum Expr {
     IsNotUnknown(Box<Expr>),
     /// arithmetic negation of an expression, the operand must be of a signed numeric data type
     Negative(Box<Expr>),
-    /// Returns the field of a [`arrow::array::ListArray`] or [`arrow::array::StructArray`] by key
-    ///
+    /// Returns the field of a [`arrow::array::ListArray`] or
+    /// [`arrow::array::StructArray`] by key or key range

Review Comment:
   ```suggestion
       /// [`arrow::array::StructArray`] by index or range
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] yjshen commented on a diff in pull request #7218: Add `Expr::field`, `Expr::index`, and `Expr::slice`, add docs

Posted by "yjshen (via GitHub)" <gi...@apache.org>.
yjshen commented on code in PR #7218:
URL: https://github.com/apache/arrow-datafusion/pull/7218#discussion_r1286594189


##########
datafusion/expr/src/expr.rs:
##########
@@ -115,8 +115,8 @@ pub enum Expr {
     IsNotUnknown(Box<Expr>),
     /// arithmetic negation of an expression, the operand must be of a signed numeric data type
     Negative(Box<Expr>),
-    /// Returns the field of a [`arrow::array::ListArray`] or [`arrow::array::StructArray`] by key
-    ///
+    /// Returns the field of a [`arrow::array::ListArray`] or
+    /// [`arrow::array::StructArray`] by key or key range

Review Comment:
   List range instead of key range?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org