alamb commented on code in PR #14023:
URL: https://github.com/apache/datafusion/pull/14023#discussion_r1904210872


##########
datafusion/functions/src/strings.rs:
##########
@@ -18,102 +18,12 @@
 use std::mem::size_of;
 
 use arrow::array::{
-    make_view, Array, ArrayAccessor, ArrayDataBuilder, ArrayIter, ByteView,
-    GenericStringArray, LargeStringArray, OffsetSizeTrait, StringArray, 
StringViewArray,
-    StringViewBuilder,
+    make_view, Array, ArrayAccessor, ArrayDataBuilder, ByteView, 
LargeStringArray,
+    StringArray, StringViewArray, StringViewBuilder,
 };
 use arrow::datatypes::DataType;
 use arrow_buffer::{MutableBuffer, NullBuffer, NullBufferBuilder};
 
-/// Abstracts iteration over different types of string arrays.
-///
-/// The [`StringArrayType`] trait helps write generic code for string 
functions that can work with
-/// different types of string arrays.
-///
-/// Currently three types are supported:
-/// - [`StringArray`]
-/// - [`LargeStringArray`]
-/// - [`StringViewArray`]
-///
-/// It is inspired / copied from [arrow-rs].
-///
-/// [arrow-rs]: 
https://github.com/apache/arrow-rs/blob/bf0ea9129e617e4a3cf915a900b747cc5485315f/arrow-string/src/like.rs#L151-L157
-///
-/// # Examples
-/// Generic function that works for [`StringArray`], [`LargeStringArray`]
-/// and [`StringViewArray`]:
-/// ```
-/// # use arrow::array::{StringArray, LargeStringArray, StringViewArray};
-/// # use datafusion_functions::strings::StringArrayType;
-///
-/// /// Combines string values for any StringArrayType type. It can be invoked 
on
-/// /// and combination of `StringArray`, `LargeStringArray` or 
`StringViewArray`
-/// fn combine_values<'a, S1, S2>(array1: S1, array2: S2) -> Vec<String>
-///   where S1: StringArrayType<'a>, S2: StringArrayType<'a>
-/// {
-///   // iterate over the elements of the 2 arrays in parallel
-///   array1
-///   .iter()
-///   .zip(array2.iter())
-///   .map(|(s1, s2)| {
-///      // if both values are non null, combine them
-///      if let (Some(s1), Some(s2)) = (s1, s2) {
-///        format!("{s1}{s2}")
-///      } else {
-///        "None".to_string()
-///     }
-///    })
-///   .collect()
-/// }
-///
-/// let string_array = StringArray::from(vec!["foo", "bar"]);
-/// let large_string_array = LargeStringArray::from(vec!["foo2", "bar2"]);
-/// let string_view_array = StringViewArray::from(vec!["foo3", "bar3"]);
-///
-/// // can invoke this function a string array and large string array
-/// assert_eq!(
-///   combine_values(&string_array, &large_string_array),
-///   vec![String::from("foofoo2"), String::from("barbar2")]
-/// );
-///
-/// // Can call the same function with string array and string view array
-/// assert_eq!(
-///   combine_values(&string_array, &string_view_array),
-///   vec![String::from("foofoo3"), String::from("barbar3")]
-/// );
-/// ```
-///
-/// [`LargeStringArray`]: arrow::array::LargeStringArray
-pub trait StringArrayType<'a>: ArrayAccessor<Item = &'a str> + Sized {

Review Comment:
   It might be nicer to deprecate this trait instead of immediately removing it 
as described here:
   
   
https://datafusion.apache.org/library-user-guide/api-health.html#deprecation-guidelines
   
   Basically we could remove the comments and just leave a `#[deprecated]` 
comment



##########
datafusion/functions/src/strings.rs:
##########
@@ -18,102 +18,12 @@
 use std::mem::size_of;
 
 use arrow::array::{
-    make_view, Array, ArrayAccessor, ArrayDataBuilder, ArrayIter, ByteView,
-    GenericStringArray, LargeStringArray, OffsetSizeTrait, StringArray, 
StringViewArray,
-    StringViewBuilder,
+    make_view, Array, ArrayAccessor, ArrayDataBuilder, ByteView, 
LargeStringArray,
+    StringArray, StringViewArray, StringViewBuilder,
 };
 use arrow::datatypes::DataType;
 use arrow_buffer::{MutableBuffer, NullBuffer, NullBufferBuilder};
 
-/// Abstracts iteration over different types of string arrays.

Review Comment:
   I double checked and this was made public in arrow in this PR:
   - https://github.com/apache/arrow-rs/pull/6720/files#r1844543809
   
   Thanks @tlm365 (for both changes!)



-- 
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