jayzhan211 commented on code in PR #14057: URL: https://github.com/apache/datafusion/pull/14057#discussion_r1911800158
########## 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: 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. The downside of the current approach is that whenever the schema is changed, the index of meta columns need to adjust too. I think this is error prone. Minimize the dependency of meta schema and schema is better -- 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