dbtsai commented on code in PR #131:
URL:
https://github.com/apache/spark-connect-rust/pull/131#discussion_r4433527230
##########
crates/spark-connect/src/types.rs:
##########
@@ -865,7 +865,11 @@ impl DataType {
match self {
DataType::Struct { .. } => {
let mut out = String::from("root\n");
- self.append_tree(&mut out, " |", 1, max_depth);
+ self.append_tree(
+ &mut out,
+ " |",
+ if max_depth > 0 { max_depth } else { i32::MAX },
Review Comment:
Updated the public method documentation: depth counts nested structs, arrays
and maps, top-level fields are depth 1, and zero or negative limits render the
full tree.
##########
crates/spark-connect/src/types.rs:
##########
@@ -875,33 +879,94 @@ impl DataType {
}
}
- fn append_tree(&self, out: &mut String, prefix: &str, depth: i32,
max_depth: i32) {
- if let DataType::Struct { fields } = self {
- for f in fields {
- // A nested struct is rendered as the bare type name
("struct") and its
- // fields are shown by recursing below, so `max_depth` can
truncate them.
- // Using `simple_string()` here would inline the children (e.g.
- // "struct<inner:int>"), leaking them past `max_depth`.
Non-struct types
- // have no depth-controlled children, so their simple string
is used.
- let is_struct = matches!(f.data_type, DataType::Struct { .. });
- let type_repr = if is_struct {
- f.data_type.type_name()
- } else {
- f.data_type.simple_string()
- };
- out.push_str(&format!(
- "{}-- {}: {} (nullable = {})\n",
- prefix, f.name, type_repr, f.nullable
- ));
- if is_struct && depth < max_depth {
- f.data_type.append_tree(
+ fn tree_type_name(&self) -> String {
+ match self {
+ DataType::Decimal { .. }
+ | DataType::Char { .. }
+ | DataType::Varchar { .. }
+ | DataType::Time { .. }
+ | DataType::YearMonthInterval { .. }
+ | DataType::DayTimeInterval { .. } => self.simple_string(),
+ DataType::Udt {
+ python_class: Some(python_class),
+ ..
+ } => python_class
+ .rsplit('.')
+ .next()
+ .filter(|class_name| !class_name.is_empty())
+ // UserDefinedType.typeName() preserves the entire class name,
+ // unlike DataType.typeName(), which strips the "Type" suffix.
+ .map(str::to_lowercase)
+ .unwrap_or_else(|| self.type_name()),
+ _ => self.type_name(),
+ }
+ }
+
+ fn append_tree(&self, out: &mut String, prefix: &str, remaining: i32) {
+ if remaining <= 0 {
+ return;
+ }
+ let child_prefix = format!("{prefix} |");
Review Comment:
Moved child_prefix construction into the Struct, Array and Map arms, so
scalar leaf calls no longer allocate an unused prefix.
##########
crates/spark-connect/src/types.rs:
##########
@@ -875,33 +879,94 @@ impl DataType {
}
}
- fn append_tree(&self, out: &mut String, prefix: &str, depth: i32,
max_depth: i32) {
- if let DataType::Struct { fields } = self {
- for f in fields {
- // A nested struct is rendered as the bare type name
("struct") and its
- // fields are shown by recursing below, so `max_depth` can
truncate them.
- // Using `simple_string()` here would inline the children (e.g.
- // "struct<inner:int>"), leaking them past `max_depth`.
Non-struct types
- // have no depth-controlled children, so their simple string
is used.
- let is_struct = matches!(f.data_type, DataType::Struct { .. });
- let type_repr = if is_struct {
- f.data_type.type_name()
- } else {
- f.data_type.simple_string()
- };
- out.push_str(&format!(
- "{}-- {}: {} (nullable = {})\n",
- prefix, f.name, type_repr, f.nullable
- ));
- if is_struct && depth < max_depth {
- f.data_type.append_tree(
+ fn tree_type_name(&self) -> String {
+ match self {
+ DataType::Decimal { .. }
+ | DataType::Char { .. }
+ | DataType::Varchar { .. }
+ | DataType::Time { .. }
+ | DataType::YearMonthInterval { .. }
+ | DataType::DayTimeInterval { .. } => self.simple_string(),
+ DataType::Udt {
+ python_class: Some(python_class),
+ ..
+ } => python_class
+ .rsplit('.')
+ .next()
+ .filter(|class_name| !class_name.is_empty())
+ // UserDefinedType.typeName() preserves the entire class name,
+ // unlike DataType.typeName(), which strips the "Type" suffix.
+ .map(str::to_lowercase)
+ .unwrap_or_else(|| self.type_name()),
+ _ => self.type_name(),
+ }
+ }
+
+ fn append_tree(&self, out: &mut String, prefix: &str, remaining: i32) {
+ if remaining <= 0 {
+ return;
+ }
+ let child_prefix = format!("{prefix} |");
+ match self {
+ DataType::Struct { fields } => {
+ for f in fields {
+ out.push_str(prefix);
+ out.push_str("-- ");
+ for ch in f.name.chars() {
Review Comment:
Extracted push_escaped_name(out, &f.name). It keeps the single-pass writes
and the same seven escapes; append_tree now focuses on structure.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
