This is an automated email from the ASF dual-hosted git repository.
dbtsai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark-connect-rust.git
The following commit(s) were added to refs/heads/master by this push:
new c36c846 [SPARK-60585][CONNECT][FOLLOWUP] Address row-sharing review
feedback (#106)
c36c846 is described below
commit c36c846fcb4f22c9995e5de5c69c45ae76a5ad9a
Author: DB Tsai <[email protected]>
AuthorDate: Tue Oct 27 15:51:32 2026 -0700
[SPARK-60585][CONNECT][FOLLOWUP] Address row-sharing review feedback (#106)
Reserve decoder row values, document the owned/shared name representation,
and keep tests focused on decoded values and field-name sharing.
Co-authored-by: Isaac <[email protected]>
---
crates/spark-connect/src/catalog.rs | 52 -----------------------------------
crates/spark-connect/src/dataframe.rs | 2 +-
crates/spark-connect/src/row.rs | 41 ++++-----------------------
3 files changed, 6 insertions(+), 89 deletions(-)
diff --git a/crates/spark-connect/src/catalog.rs
b/crates/spark-connect/src/catalog.rs
index 7ce672a..ec705fc 100644
--- a/crates/spark-connect/src/catalog.rs
+++ b/crates/spark-connect/src/catalog.rs
@@ -1082,55 +1082,3 @@ impl Catalog {
Ok(request)
}
}
-
-#[cfg(test)]
-mod tests {
- use super::*;
-
- #[test]
- fn parse_database_preserves_metadata_and_nullable_columns() {
- let fields = vec![
- "name".to_string(),
- "catalog".to_string(),
- "description".to_string(),
- "locationUri".to_string(),
- ];
- let populated = Row::new(
- fields.clone(),
- vec![
- Value::String("one".to_string()),
- Value::String("spark_catalog".to_string()),
- Value::String("first database".to_string()),
- Value::String("file:/one".to_string()),
- ],
- );
- let nullable = Row::new(
- fields,
- vec![
- Value::String("two".to_string()),
- Value::Null,
- Value::Null,
- Value::String("file:/two".to_string()),
- ],
- );
-
- assert_eq!(
- parse_database(&populated),
- Database {
- name: "one".to_string(),
- catalog: Some("spark_catalog".to_string()),
- description: Some("first database".to_string()),
- location_uri: "file:/one".to_string(),
- }
- );
- assert_eq!(
- parse_database(&nullable),
- Database {
- name: "two".to_string(),
- catalog: None,
- description: None,
- location_uri: "file:/two".to_string(),
- }
- );
- }
-}
diff --git a/crates/spark-connect/src/dataframe.rs
b/crates/spark-connect/src/dataframe.rs
index 72dff2f..ea0e2c6 100644
--- a/crates/spark-connect/src/dataframe.rs
+++ b/crates/spark-connect/src/dataframe.rs
@@ -2092,7 +2092,7 @@ pub(crate) fn decode_arrow_batch(
.collect::<Vec<_>>();
for row_idx in 0..num_rows {
- let mut values = vec![];
+ let mut values = Vec::with_capacity(record_batch.num_columns());
for (column, layout) in
record_batch.columns().iter().zip(&layouts) {
let value = arrow_value_at_with_layout(column.as_ref(),
row_idx, layout)?;
diff --git a/crates/spark-connect/src/row.rs b/crates/spark-connect/src/row.rs
index a4b3f52..4c9cf5c 100644
--- a/crates/spark-connect/src/row.rs
+++ b/crates/spark-connect/src/row.rs
@@ -281,6 +281,7 @@ impl StructValue {
}
}
+// Keep local constructors free of Arc conversion allocations while sharing
decoded batch names.
#[derive(Clone)]
enum RowFields {
Owned(Vec<String>),
@@ -465,45 +466,13 @@ mod tests {
}
#[test]
- fn cloned_rows_share_field_names_and_keep_values_independent() {
- let fields: Arc<[String]> = vec!["items".to_string()].into();
- let row = Row::with_shared_fields(
- Arc::clone(&fields),
- vec![Value::List(vec![Value::String("original".to_string())])],
- );
- let mut cloned = row.clone();
+ fn cloned_rows_share_field_names() {
+ let fields: Arc<[String]> = vec!["id".to_string()].into();
+ let row = Row::with_shared_fields(Arc::clone(&fields),
vec![Value::Long(1)]);
+ let cloned = row.clone();
assert!(std::ptr::eq(fields.as_ref(), row.fields()));
assert!(std::ptr::eq(row.fields(), cloned.fields()));
- let Value::List(items) = &mut cloned.values[0] else {
- panic!("expected a list value");
- };
- items[0] = Value::String("changed".to_string());
- assert_eq!(
- row.get(0),
- Some(&Value::List(vec![Value::String("original".to_string())]))
- );
- assert_ne!(row.values(), cloned.values());
- }
-
- #[test]
- fn constructed_rows_clone_values_independently() {
- let row = Row::new(
- vec!["items".to_string()],
- vec![Value::List(vec![Value::String("original".to_string())])],
- );
-
- let mut cloned = row.clone();
- assert_eq!(row.fields(), cloned.fields());
- let Value::List(items) = &mut cloned.values[0] else {
- panic!("expected a list value");
- };
- items[0] = Value::String("changed".to_string());
- assert_eq!(
- row.get(0),
- Some(&Value::List(vec![Value::String("original".to_string())]))
- );
- assert_ne!(row.values(), cloned.values());
}
#[test]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
