chenkovsky commented on code in PR #14057:
URL: https://github.com/apache/datafusion/pull/14057#discussion_r1911814285


##########
datafusion/common/src/dfschema.rs:
##########
@@ -319,52 +465,56 @@ impl DFSchema {
                 qualifiers.push(qualifier.cloned());
             }
         }
-        let mut metadata = self.inner.metadata.clone();
-        metadata.extend(other_schema.inner.metadata.clone());
+        let mut metadata = self.inner.schema.metadata.clone();
+        metadata.extend(other_schema.inner.schema.metadata.clone());
 
         let finished = schema_builder.finish();
         let finished_with_metadata = finished.with_metadata(metadata);
-        self.inner = finished_with_metadata.into();
-        self.field_qualifiers.extend(qualifiers);
+        self.inner.schema = finished_with_metadata.into();
+        self.inner.field_qualifiers.extend(qualifiers);
     }
 
     /// Get a list of fields
     pub fn fields(&self) -> &Fields {
-        &self.inner.fields
+        &self.inner.schema.fields
     }
 
     /// Returns an immutable reference of a specific `Field` instance selected 
using an
     /// offset within the internal `fields` vector
     pub fn field(&self, i: usize) -> &Field {
-        &self.inner.fields[i]
+        if i >= self.inner.len() {
+            if let Some(metadata) = &self.metadata {
+                return metadata.field(i - self.inner.len());
+            }
+        }
+        self.inner.field(i)
     }
 
     /// Returns an immutable reference of a specific `Field` instance selected 
using an
     /// offset within the internal `fields` vector and its qualifier
     pub fn qualified_field(&self, i: usize) -> (Option<&TableReference>, 
&Field) {
-        (self.field_qualifiers[i].as_ref(), self.field(i))
+        if i >= self.inner.len() {
+            if let Some(metadata) = &self.metadata {
+                return metadata.qualified_field(i - self.inner.len());
+            }
+        }
+        self.inner.qualified_field(i)

Review Comment:
   I see. it's error prone. Can we change the offsets of metadata columns, e.g. 
(-1 as usize) (-2 as usize) then there's no such problem. I see some databases 
use this trick.
   
   > Isn't only where you need meta columns you need to change the code with 
meta_field? Others code that call with field remain the same.
   
   yes, we can. but many api use Vec<usize> to represent columns. I have to 
change many structs and method defnitions to pass extra parameters.
   
   



-- 
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-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to