izveigor commented on code in PR #7220:
URL: https://github.com/apache/arrow-datafusion/pull/7220#discussion_r1286712027


##########
datafusion/expr/src/field_util.rs:
##########
@@ -20,64 +20,65 @@
 use arrow::datatypes::{DataType, Field};
 use datafusion_common::{plan_err, DataFusionError, Result, ScalarValue};
 
-pub enum GetFieldAccessCharacteristic {
-    /// returns the field `struct[field]`. For example `struct["name"]`
+/// Types of the field access expression of a nested type, such as `Field` or 
`List`
+pub enum GetFieldAccessSchema {
+    /// Named field, For example `struct["name"]`
     NamedStructField { name: ScalarValue },
-    /// single list index
-    // list[i]
+    /// Single list index, for example: `list[i]`
     ListIndex { key_dt: DataType },
-    /// list range `list[i:j]`
+    /// List range, for example `list[i:j]`
     ListRange {
         start_dt: DataType,
         stop_dt: DataType,
     },
 }
 
-/// Returns the field access indexed by `key` and/or `extra_key` from a 
[`DataType::List`] or [`DataType::Struct`]
-/// # Error
-/// Errors if
-/// * the `data_type` is not a Struct or a List,
-/// * the `data_type` of extra key does not match with `data_type` of key
-/// * there is no field key is not of the required index type
-pub fn get_indexed_field(
-    data_type: &DataType,
-    field_characteristic: &GetFieldAccessCharacteristic,
-) -> Result<Field> {
-    match field_characteristic {
-        GetFieldAccessCharacteristic::NamedStructField{ name } => {
-            match (data_type, name) {
-                (DataType::Struct(fields), ScalarValue::Utf8(Some(s))) => {
-                    if s.is_empty() {
-                        plan_err!(
-                            "Struct based indexed access requires a non empty 
string"
-                        )
-                    } else {
-                        let field = fields.iter().find(|f| f.name() == s);
-                        field.ok_or(DataFusionError::Plan(format!("Field {s} 
not found in struct"))).map(|f| f.as_ref().clone())
+impl GetFieldAccessSchema {
+    /// Returns the schema [`Field`] from a [`DataType::List`] or
+    /// [`DataType::Struct`] indexed by this structure
+    ///
+    /// # Error
+    /// Errors if
+    /// * the `data_type` is not a Struct or a List,
+    /// * the `data_type` of extra key does not match with `data_type` of key

Review Comment:
   It is better to replace `extra_key` and `key` with new terms



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to