From 29270caa2ccf2b6f7646f344498ffbc6c5e69000 Mon Sep 17 00:00:00 2001 From: Runji Wang Date: Tue, 27 Dec 2022 17:41:08 +0800 Subject: [PATCH] perf(expr): optimize casting to varchar (#7066) This PR optimizes the performance of casting values to varchar. It introduced write API for `ToText`, so that strings can be directly written to array buffers without generating String. The display function of interval and timestampz was also optimized. perf-cast
Click to show full results bench | Before time(us) | After time(us) | Change(%) | Speedup -- | -- | -- | -- | -- cast(timestampz->varchar) | 508.640 | 121.600 | -76.1% | 3.2 cast(timestamp->varchar) | 166.200 | 58.245 | -65.0% | 1.9 cast(float64->varchar) | 78.386 | 57.597 | -26.5% | 0.4 cast(float32->varchar) | 57.903 | 37.384 | -35.4% | 0.5 cast(date->varchar) | 86.896 | 32.669 | -62.4% | 1.7 cast(time->varchar) | 47.508 | 28.428 | -40.2% | 0.7 cast(decimal->varchar) | 67.682 | 28.317 | -58.2% | 1.4 cast(int16->varchar) | 29.532 | 12.337 | -58.2% | 1.4 cast(int64->varchar) | 52.043 | 12.319 | -76.3% | 3.2 cast(int32->varchar) | 28.863 | 12.258 | -57.5% | 1.4 cast(boolean->varchar) | 26.826 | 6.396 | -76.2% | 3.2 bool_out(boolean) | 25.480 | 5.126 | -79.9% | 4.0
The `writer` argument of string functions was also changed from `StringWriter<'_>` to `&mut dyn Write`, making them decouple from array. I tried to use `&mut impl Write` but was blocked by annoying lifetime issues. Anyways, the performance of these operations is still slightly improved: perf-string-ops
Click to show full results bench | Before time(us) | After time(us) | Change(%) | Speedup -- | -- | -- | -- | -- rtrim(varchar,varchar) | 21.780 | 15.768 | -27.6% | 0.4 substr(varchar,int32,int32) | 11.126 | 8.090 | -27.3% | 0.4 rtrim(varchar) | 10.537 | 7.712 | -26.8% | 0.4 substr(varchar,int32) | 9.198 | 7.111 | -22.7% | 0.3 ltrim(varchar) | 9.661 | 8.010 | -17.1% | 0.2 trim(varchar) | 11.308 | 9.618 | -14.9% | 0.2 overlay(varchar,varchar,int32,int32) | 17.107 | 14.697 | -14.1% | 0.2 overlay(varchar,varchar,int32) | 13.408 | 12.007 | -10.4% | 0.1 ltrim(varchar,varchar) | 21.198 | 19.021 | -10.3% | 0.1 trim(varchar,varchar) | 20.876 | 19.205 | -8.0% | 0.1 split_part(varchar,varchar,int32) | 30.708 | 29.293 | -4.6% | 0.0 md5(varchar) | 346.010 | 331.670 | -4.1% | 0.0
Approved-By: BowenXiao1999 Approved-By: BugenZhao --- src/common/src/array/bytes_array.rs | 21 +-- src/common/src/array/list_array.rs | 11 +- src/common/src/array/struct_array.rs | 21 ++- src/common/src/array/utf8_array.rs | 65 ++----- src/common/src/types/chrono_wrapper.rs | 24 +-- src/common/src/types/decimal.rs | 8 +- src/common/src/types/interval.rs | 49 +++--- src/common/src/types/to_text.rs | 201 ++++++++++------------ src/expr/src/expr/expr_concat_ws.rs | 7 +- src/expr/src/expr/expr_unary.rs | 32 ++-- src/expr/src/expr/template.rs | 64 ++++--- src/expr/src/vector_op/cast.rs | 67 +++++--- src/expr/src/vector_op/concat_op.rs | 22 +-- src/expr/src/vector_op/lower.rs | 18 +- src/expr/src/vector_op/ltrim.rs | 17 +- src/expr/src/vector_op/md5.rs | 18 +- src/expr/src/vector_op/overlay.rs | 37 ++-- src/expr/src/vector_op/repeat.rs | 20 +-- src/expr/src/vector_op/replace.rs | 35 ++-- src/expr/src/vector_op/rtrim.rs | 18 +- src/expr/src/vector_op/split_part.rs | 32 ++-- src/expr/src/vector_op/substr.rs | 58 +++---- src/expr/src/vector_op/to_char.rs | 19 +- src/expr/src/vector_op/translate.rs | 25 ++- src/expr/src/vector_op/trim.rs | 23 +-- src/expr/src/vector_op/trim_characters.rs | 44 ++--- src/expr/src/vector_op/upper.rs | 18 +- 27 files changed, 423 insertions(+), 551 deletions(-) diff --git a/src/common/src/array/bytes_array.rs b/src/common/src/array/bytes_array.rs index 25a231607733b..1f2736817a7bf 100644 --- a/src/common/src/array/bytes_array.rs +++ b/src/common/src/array/bytes_array.rs @@ -110,18 +110,6 @@ impl Array for BytesArray { } impl BytesArray { - /// Retrieve the ownership of the single bytes value. - /// - /// Panics if there're multiple or no values. - pub fn into_single_value(self) -> Option> { - assert_eq!(self.len(), 1); - if !self.is_null(0) { - Some(self.data.into_boxed_slice()) - } else { - None - } - } - #[cfg(test)] pub(super) fn data(&self) -> &[u8] { &self.data @@ -257,13 +245,10 @@ pub struct BytesWriter<'a> { builder: &'a mut BytesArrayBuilder, } -pub struct WrittenGuard(()); - impl<'a> BytesWriter<'a> { /// `write_ref` will consume `BytesWriter` and pass the ownership of `builder` to `BytesGuard`. - pub fn write_ref(self, value: &[u8]) -> WrittenGuard { + pub fn write_ref(self, value: &[u8]) { self.builder.append(Some(value)); - WrittenGuard(()) } /// `begin` will create a `PartialBytesWriter`, which allow multiple appendings to create a new @@ -290,10 +275,8 @@ impl<'a> PartialBytesWriter<'a> { /// `finish` will be called while the entire record is written. /// Exactly one new record was appended and the `builder` can be safely used. - pub fn finish(self) -> WrittenGuard { + pub fn finish(self) { self.builder.finish_partial(); - - WrittenGuard(()) } } diff --git a/src/common/src/array/list_array.rs b/src/common/src/array/list_array.rs index f6c00f18edf99..04c1a0bc767dd 100644 --- a/src/common/src/array/list_array.rs +++ b/src/common/src/array/list_array.rs @@ -477,7 +477,7 @@ impl Debug for ListRef<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { iter_elems_ref!(self, it, { for v in it { - v.fmt(f)?; + Debug::fmt(&v, f)?; } Ok(()) }) @@ -487,9 +487,10 @@ impl Debug for ListRef<'_> { impl ToText for ListRef<'_> { // This function will be invoked when pgwire prints a list value in string. // Refer to PostgreSQL `array_out` or `appendPGArray`. - fn to_text(&self) -> String { + fn write(&self, f: &mut W) -> std::fmt::Result { iter_elems_ref!(self, it, { - format!( + write!( + f, "{{{}}}", it.format_with(",", |datum_ref, f| { let s = datum_ref.to_text(); @@ -521,9 +522,9 @@ impl ToText for ListRef<'_> { }) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - DataType::List { .. } => self.to_text(), + DataType::List { .. } => self.write(f), _ => unreachable!(), } } diff --git a/src/common/src/array/struct_array.rs b/src/common/src/array/struct_array.rs index f11d0f97cd716..320065b5ba2a5 100644 --- a/src/common/src/array/struct_array.rs +++ b/src/common/src/array/struct_array.rs @@ -414,18 +414,25 @@ impl Debug for StructRef<'_> { } impl ToText for StructRef<'_> { - fn to_text(&self) -> String { + fn write(&self, f: &mut W) -> std::fmt::Result { iter_fields_ref!(self, it, { - format!( - "({})", - it.map(|x| x.to_text()).collect::>().join(",") - ) + write!(f, "(")?; + let mut is_first = true; + for x in it { + if is_first { + is_first = false; + } else { + write!(f, ",")?; + } + ToText::write(&x, f)?; + } + write!(f, ")") }) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - DataType::Struct(_) => self.to_text(), + DataType::Struct(_) => self.write(f), _ => unreachable!(), } } diff --git a/src/common/src/array/utf8_array.rs b/src/common/src/array/utf8_array.rs index c087fa948f97b..a5ae4c13d3562 100644 --- a/src/common/src/array/utf8_array.rs +++ b/src/common/src/array/utf8_array.rs @@ -16,7 +16,7 @@ use std::fmt::{Display, Write}; use risingwave_pb::data::{Array as ProstArray, ArrayType}; -use super::bytes_array::{BytesWriter, PartialBytesWriter, WrittenGuard}; +use super::bytes_array::{BytesWriter, PartialBytesWriter}; use super::{Array, ArrayBuilder, ArrayMeta, BytesArray, BytesArrayBuilder}; use crate::array::ArrayBuilderImpl; use crate::buffer::Bitmap; @@ -89,16 +89,6 @@ impl<'a> FromIterator<&'a str> for Utf8Array { } impl Utf8Array { - /// Retrieve the ownership of the single string value. - /// - /// Panics if there're multiple or no values. - #[inline] - pub fn into_single_value(self) -> Option> { - self.bytes - .into_single_value() - .map(|bytes| unsafe { std::str::from_boxed_utf8_unchecked(bytes) }) - } - pub fn into_bytes_array(self) -> BytesArray { self.bytes } @@ -169,23 +159,6 @@ pub struct StringWriter<'a> { } impl<'a> StringWriter<'a> { - /// `write_ref` will consume `StringWriter` and pass the ownership of `builder` to `BytesGuard`. - #[inline] - pub fn write_ref(self, value: &str) -> WrittenGuard { - self.bytes.write_ref(value.as_bytes()) - } - - /// `write_from_char_iter` will consume `StringWriter` and write the characters from the `iter`. - /// - /// Prefer [`StringWriter::begin`] for writing multiple string pieces. - pub fn write_from_char_iter(self, iter: impl Iterator) -> WrittenGuard { - let mut writer = self.begin(); - for c in iter { - writer.write_char(c).unwrap(); - } - writer.finish() - } - /// `begin` will create a `PartialStringWriter`, which allow multiple appendings to create a new /// record. pub fn begin(self) -> PartialStringWriter<'a> { @@ -202,24 +175,16 @@ pub struct PartialStringWriter<'a> { } impl<'a> PartialStringWriter<'a> { - /// `write_ref` will append partial dirty data to `builder`. - /// `PartialStringWriter::write_ref` is different from `StringWriter::write_ref` - /// in that it allows us to call it multiple times. - #[inline] - pub fn write_ref(&mut self, value: &str) { - self.bytes.write_ref(value.as_bytes()); - } - /// `finish` will be called while the entire record is written. /// Exactly one new record was appended and the `builder` can be safely used. - pub fn finish(self) -> WrittenGuard { + pub fn finish(self) { self.bytes.finish() } } impl Write for PartialStringWriter<'_> { fn write_str(&mut self, s: &str) -> std::fmt::Result { - self.write_ref(s); + self.bytes.write_ref(s.as_bytes()); Ok(()) } } @@ -249,11 +214,11 @@ mod tests { #[test] fn test_utf8_partial_writer() { let mut builder = Utf8ArrayBuilder::new(0); - let _guard: WrittenGuard = { + { let writer = builder.writer(); let mut partial_writer = writer.begin(); for _ in 0..2 { - partial_writer.write_ref("ran"); + partial_writer.write_str("ran").unwrap(); } partial_writer.finish() }; @@ -267,31 +232,29 @@ mod tests { fn test_utf8_partial_writer_failed() { let mut builder = Utf8ArrayBuilder::new(0); // Write a record. - let _guard: WrittenGuard = { + { let writer = builder.writer(); let mut partial_writer = writer.begin(); - partial_writer.write_ref("Dia"); - partial_writer.write_ref("na"); + partial_writer.write_str("Dia").unwrap(); + partial_writer.write_str("na").unwrap(); partial_writer.finish() }; // Write a record failed. - let _maybe_guard: Option = { + { let writer = builder.writer(); let mut partial_writer = writer.begin(); - partial_writer.write_ref("Ca"); - partial_writer.write_ref("rol"); - + partial_writer.write_str("Ca").unwrap(); + partial_writer.write_str("rol").unwrap(); // We don't finish here. - None }; // Write a record. - let _guard: WrittenGuard = { + { let writer = builder.writer(); let mut partial_writer = writer.begin(); - partial_writer.write_ref("Ki"); - partial_writer.write_ref("ra"); + partial_writer.write_str("Ki").unwrap(); + partial_writer.write_str("ra").unwrap(); partial_writer.finish() }; diff --git a/src/common/src/types/chrono_wrapper.rs b/src/common/src/types/chrono_wrapper.rs index a92a6196e49f1..9253b731c1b26 100644 --- a/src/common/src/types/chrono_wrapper.rs +++ b/src/common/src/types/chrono_wrapper.rs @@ -72,39 +72,39 @@ impl Default for NaiveDateTimeWrapper { } impl ToText for NaiveDateWrapper { - fn to_text(&self) -> String { - self.0.to_string() + fn write(&self, f: &mut W) -> std::fmt::Result { + write!(f, "{}", self.0) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - super::DataType::Date => self.to_text(), + super::DataType::Date => self.write(f), _ => unreachable!(), } } } impl ToText for NaiveTimeWrapper { - fn to_text(&self) -> String { - self.0.to_string() + fn write(&self, f: &mut W) -> std::fmt::Result { + write!(f, "{}", self.0) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - super::DataType::Time => self.to_text(), + super::DataType::Time => self.write(f), _ => unreachable!(), } } } impl ToText for NaiveDateTimeWrapper { - fn to_text(&self) -> String { - self.0.to_string() + fn write(&self, f: &mut W) -> std::fmt::Result { + write!(f, "{}", self.0) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - super::DataType::Timestamp => self.to_text(), + super::DataType::Timestamp => self.write(f), _ => unreachable!(), } } diff --git a/src/common/src/types/decimal.rs b/src/common/src/types/decimal.rs index 69ac4e11c571c..8a954a10cc6c9 100644 --- a/src/common/src/types/decimal.rs +++ b/src/common/src/types/decimal.rs @@ -42,13 +42,13 @@ pub enum Decimal { } impl ToText for Decimal { - fn to_text(&self) -> String { - self.to_string() + fn write(&self, f: &mut W) -> std::fmt::Result { + write!(f, "{self}") } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - DataType::Decimal => self.to_text(), + DataType::Decimal => self.write(f), _ => unreachable!(), } } diff --git a/src/common/src/types/interval.rs b/src/common/src/types/interval.rs index 114996b62cc28..eb46c9460e6a6 100644 --- a/src/common/src/types/interval.rs +++ b/src/common/src/types/interval.rs @@ -14,7 +14,7 @@ use std::cmp::Ordering; use std::error::Error; -use std::fmt::{Display, Formatter, Write as _}; +use std::fmt::{Display, Formatter}; use std::hash::{Hash, Hasher}; use std::io::Write; use std::ops::{Add, Neg, Sub}; @@ -25,7 +25,6 @@ use bytes::BytesMut; use num_traits::{CheckedAdd, CheckedSub, Zero}; use postgres_types::{to_sql_checked, FromSql}; use risingwave_pb::data::IntervalUnit as IntervalUnitProto; -use smallvec::SmallVec; use super::ops::IsNegative; use super::to_binary::ToBinary; @@ -645,13 +644,13 @@ impl Neg for IntervalUnit { } impl ToText for crate::types::IntervalUnit { - fn to_text(&self) -> String { - self.to_string() + fn write(&self, f: &mut W) -> std::fmt::Result { + write!(f, "{self}") } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> std::fmt::Result { match ty { - DataType::Interval => self.to_string(), + DataType::Interval => self.write(f), _ => unreachable!(), } } @@ -662,37 +661,47 @@ impl Display for IntervalUnit { let years = self.months / 12; let months = self.months % 12; let days = self.days; - let mut v = SmallVec::<[String; 4]>::new(); + let mut space = false; + let mut write = |arg: std::fmt::Arguments<'_>| { + if space { + write!(f, " ")?; + } + write!(f, "{arg}")?; + space = true; + Ok(()) + }; if years == 1 { - v.push(format!("{years} year")); + write(format_args!("{years} year"))?; } else if years != 0 { - v.push(format!("{years} years")); + write(format_args!("{years} years"))?; } if months == 1 { - v.push(format!("{months} mon")); + write(format_args!("{months} mon"))?; } else if months != 0 { - v.push(format!("{months} mons")); + write(format_args!("{months} mons"))?; } if days == 1 { - v.push(format!("{days} day")); + write(format_args!("{days} day"))?; } else if days != 0 { - v.push(format!("{days} days")); + write(format_args!("{days} days"))?; } if self.ms != 0 || self.months == 0 && self.days == 0 { let hours = self.ms / 1000 / 3600; let minutes = (self.ms / 1000 / 60) % 60; let seconds = self.ms % 60000 / 1000; let secs_fract = self.ms % 1000; - let mut format_time = format!("{hours:0>2}:{minutes:0>2}:{seconds:0>2}"); + write(format_args!("{hours:0>2}:{minutes:0>2}:{seconds:0>2}"))?; if secs_fract != 0 { - write!(format_time, ".{:03}", secs_fract)?; - while format_time.ends_with('0') { - format_time.pop(); - } + let mut buf = [0u8; 4]; + write!(buf.as_mut_slice(), ".{:03}", secs_fract).unwrap(); + write!( + f, + "{}", + std::str::from_utf8(&buf).unwrap().trim_end_matches('0') + )?; } - v.push(format_time); } - Display::fmt(&v.join(" "), f) + Ok(()) } } diff --git a/src/common/src/types/to_text.rs b/src/common/src/types/to_text.rs index 3bb7aec8f7d46..18dbec56afd0f 100644 --- a/src/common/src/types/to_text.rs +++ b/src/common/src/types/to_text.rs @@ -12,17 +12,27 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt::Write; +use std::fmt::{Result, Write}; use std::num::FpCategory; use chrono::{TimeZone, Utc}; -use num_traits::ToPrimitive; use super::{DataType, DatumRef, ScalarRefImpl}; +use crate::for_all_scalar_variants; // Used to convert ScalarRef to text format pub trait ToText { - fn to_text_with_type(&self, ty: &DataType) -> String; + /// Write the text to the writer. + fn write(&self, f: &mut W) -> Result; + + fn write_with_type(&self, _ty: &DataType, f: &mut W) -> Result; + + fn to_text_with_type(&self, ty: &DataType) -> String { + let mut s = String::new(); + self.write_with_type(ty, &mut s).unwrap(); + s + } + /// `to_text` is a special version of `to_text_with_type`, it convert the scalar to default type /// text. E.g. for Int64, it will convert to text as a Int64 type. /// We should prefer to use `to_text_with_type` because it's more clear and readable. @@ -46,19 +56,23 @@ pub trait ToText { /// /// Exception: /// The scalar of `DataType::Timestampz` is the `ScalarRefImpl::Int64`. - fn to_text(&self) -> String; + fn to_text(&self) -> String { + let mut s = String::new(); + self.write(&mut s).unwrap(); + s + } } macro_rules! implement_using_to_string { ($({ $scalar_type:ty , $data_type:ident} ),*) => { $( impl ToText for $scalar_type { - fn to_text(&self) -> String { - self.to_string() + fn write(&self, f: &mut W) -> Result { + write!(f, "{self}") } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::$data_type => self.to_text(), + DataType::$data_type => self.write(f), _ => unreachable!(), } } @@ -71,12 +85,12 @@ macro_rules! implement_using_itoa { ($({ $scalar_type:ty , $data_type:ident} ),*) => { $( impl ToText for $scalar_type { - fn to_text(&self) -> String { - itoa::Buffer::new().format(*self).to_owned() + fn write(&self, f: &mut W) -> Result { + write!(f, "{}", itoa::Buffer::new().format(*self)) } - fn to_text_with_type(&self, ty:&DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::$data_type => self.to_text(), + DataType::$data_type => self.write(f), _ => unreachable!(), } } @@ -96,49 +110,44 @@ implement_using_itoa! { } macro_rules! implement_using_ryu { - ($({ $scalar_type:ty, $to_std_type:ident, $data_type:ident } ),*) => { + ($({ $scalar_type:ty, $data_type:ident } ),*) => { $( impl ToText for $scalar_type { - fn to_text(&self) -> String { + fn write(&self, f: &mut W) -> Result { match self.classify() { - FpCategory::Infinite if self.is_sign_negative() => "-Infinity".to_owned(), - FpCategory::Infinite => "Infinity".to_owned(), - FpCategory::Zero if self.is_sign_negative() => "-0".to_owned(), - FpCategory::Nan => "NaN".to_owned(), - _ => match self.$to_std_type() { - Some(v) => { - let mut buf = ryu::Buffer::new(); - let mut s = buf.format_finite(v); - if let Some(trimmed) = s.strip_suffix(".0") { - s = trimmed; + FpCategory::Infinite if self.is_sign_negative() => write!(f, "-Infinity"), + FpCategory::Infinite => write!(f, "Infinity"), + FpCategory::Zero if self.is_sign_negative() => write!(f, "-0"), + FpCategory::Nan => write!(f, "NaN"), + _ => { + let mut buf = ryu::Buffer::new(); + let mut s = buf.format_finite(self.0); + if let Some(trimmed) = s.strip_suffix(".0") { + s = trimmed; + } + if let Some(mut idx) = s.as_bytes().iter().position(|x| *x == b'e') { + idx += 1; + write!(f, "{}", &s[..idx])?; + if s.as_bytes()[idx] == b'-' { + write!(f, "-")?; + idx += 1; + } else { + write!(f, "+")?; } - let mut s_chars = s.chars().peekable(); - let mut s_owned = s.to_owned(); - let mut index = 0; - while let Some(c) = s_chars.next() { - index += 1; - if c == 'e' { - if s_chars.peek() != Some(&'-') { - s_owned.insert(index, '+'); - } else { - index += 1; - } - - if index + 1 == s.len() { - s_owned.insert(index,'0'); - } - break; - } + if idx + 1 == s.len() { + write!(f, "0")?; } - s_owned + write!(f, "{}", &s[idx..])?; + } else { + write!(f, "{}", s)?; } - None => "NaN".to_owned(), - }, + Ok(()) + } } } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::$data_type => self.to_text(), + DataType::$data_type => self.write(f), _ => unreachable!(), } } @@ -148,18 +157,18 @@ macro_rules! implement_using_ryu { } implement_using_ryu! { - { crate::types::OrderedF32, to_f32,Float32 }, - { crate::types::OrderedF64, to_f64,Float64 } + { crate::types::OrderedF32, Float32 }, + { crate::types::OrderedF64, Float64 } } impl ToText for i64 { - fn to_text(&self) -> String { - self.to_string() + fn write(&self, f: &mut W) -> Result { + write!(f, "{self}") } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::Int64 => self.to_text(), + DataType::Int64 => self.write(f), DataType::Timestampz => { // Just a meaningful representation as placeholder. The real implementation depends // on TimeZone from session. See #3552. @@ -168,7 +177,8 @@ impl ToText for i64 { let instant = Utc.timestamp_opt(secs, nsecs as u32).unwrap(); // PostgreSQL uses a space rather than `T` to separate the date and time. // https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT - instant.format("%Y-%m-%d %H:%M:%S%.f%:z").to_string() + // same as `instant.format("%Y-%m-%d %H:%M:%S%.f%:z")` but faster + write!(f, "{}+00:00", instant.naive_local()) } _ => unreachable!(), } @@ -176,91 +186,66 @@ impl ToText for i64 { } impl ToText for bool { - fn to_text(&self) -> String { + fn write(&self, f: &mut W) -> Result { if *self { - "t".to_string() + write!(f, "t") } else { - "f".to_string() + write!(f, "f") } } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::Boolean => self.to_text(), + DataType::Boolean => self.write(f), _ => unreachable!(), } } } impl ToText for &[u8] { - fn to_text(&self) -> String { - let mut s = String::with_capacity(2 + 2 * self.len()); - write!(s, "\\x{}", hex::encode(self)).unwrap(); - s + fn write(&self, f: &mut W) -> Result { + write!(f, "\\x{}", hex::encode(self)) } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match ty { - DataType::Bytea => self.to_text(), + DataType::Bytea => self.write(f), _ => unreachable!(), } } } -impl ToText for ScalarRefImpl<'_> { - fn to_text(&self) -> String { - match self { - ScalarRefImpl::Bool(b) => b.to_text(), - ScalarRefImpl::Int16(i) => i.to_text(), - ScalarRefImpl::Int32(i) => i.to_text(), - ScalarRefImpl::Int64(i) => i.to_text(), - ScalarRefImpl::Float32(f) => f.to_text(), - ScalarRefImpl::Float64(f) => f.to_text(), - ScalarRefImpl::Decimal(d) => d.to_text(), - ScalarRefImpl::Interval(i) => i.to_text(), - ScalarRefImpl::NaiveDate(d) => d.to_text(), - ScalarRefImpl::NaiveTime(t) => t.to_text(), - ScalarRefImpl::NaiveDateTime(dt) => dt.to_text(), - ScalarRefImpl::List(l) => l.to_text(), - ScalarRefImpl::Struct(s) => s.to_text(), - ScalarRefImpl::Utf8(v) => v.to_text(), - ScalarRefImpl::Bytea(v) => v.to_text(), - } - } +macro_rules! impl_totext_for_scalar { + ($({ $variant_name:ident, $suffix_name:ident, $scalar:ty, $scalar_ref:ty }),*) => { + impl ToText for ScalarRefImpl<'_> { + fn write(&self, f: &mut W) -> Result { + match self { + $(ScalarRefImpl::$variant_name(v) => v.write(f),)* + } + } - fn to_text_with_type(&self, ty: &DataType) -> String { - match self { - ScalarRefImpl::Bool(b) => b.to_text_with_type(ty), - ScalarRefImpl::Int16(i) => i.to_text_with_type(ty), - ScalarRefImpl::Int32(i) => i.to_text_with_type(ty), - ScalarRefImpl::Int64(i) => i.to_text_with_type(ty), - ScalarRefImpl::Float32(f) => f.to_text_with_type(ty), - ScalarRefImpl::Float64(f) => f.to_text_with_type(ty), - ScalarRefImpl::Decimal(d) => d.to_text_with_type(ty), - ScalarRefImpl::Interval(i) => i.to_text_with_type(ty), - ScalarRefImpl::NaiveDate(d) => d.to_text_with_type(ty), - ScalarRefImpl::NaiveTime(t) => t.to_text_with_type(ty), - ScalarRefImpl::NaiveDateTime(dt) => dt.to_text_with_type(ty), - ScalarRefImpl::List(l) => l.to_text_with_type(ty), - ScalarRefImpl::Struct(s) => s.to_text_with_type(ty), - ScalarRefImpl::Utf8(v) => v.to_text_with_type(ty), - ScalarRefImpl::Bytea(v) => v.to_text_with_type(ty), + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { + match self { + $(ScalarRefImpl::$variant_name(v) => v.write_with_type(ty, f),)* + } + } } - } + }; } +for_all_scalar_variants! { impl_totext_for_scalar } impl ToText for DatumRef<'_> { - fn to_text(&self) -> String { + fn write(&self, f: &mut W) -> Result { match self { - Some(data) => data.to_text(), - None => "NULL".to_string(), + Some(data) => data.write(f), + None => write!(f, "NULL"), } } - fn to_text_with_type(&self, ty: &DataType) -> String { + fn write_with_type(&self, ty: &DataType, f: &mut W) -> Result { match self { - Some(data) => data.to_text_with_type(ty), - None => "NULL".to_string(), + Some(data) => data.write_with_type(ty, f), + None => write!(f, "NULL"), } } } diff --git a/src/expr/src/expr/expr_concat_ws.rs b/src/expr/src/expr/expr_concat_ws.rs index a6b5e6a1db11d..b0953518fd56e 100644 --- a/src/expr/src/expr/expr_concat_ws.rs +++ b/src/expr/src/expr/expr_concat_ws.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::convert::TryFrom; +use std::fmt::Write; use std::sync::Arc; use risingwave_common::array::{ @@ -74,15 +75,15 @@ impl Expression for ConcatWsExpression { let mut string_columns = string_columns_ref.iter(); for string_column in string_columns.by_ref() { if let Some(string) = string_column.value_at(row_idx) { - writer.write_ref(string); + writer.write_str(string).unwrap(); break; } } for string_column in string_columns { if let Some(string) = string_column.value_at(row_idx) { - writer.write_ref(sep); - writer.write_ref(string); + writer.write_str(sep).unwrap(); + writer.write_str(string).unwrap(); } } diff --git a/src/expr/src/expr/expr_unary.rs b/src/expr/src/expr/expr_unary.rs index 1697e604ebe85..b469fa5200dcf 100644 --- a/src/expr/src/expr/expr_unary.rs +++ b/src/expr/src/expr/expr_unary.rs @@ -165,30 +165,34 @@ pub fn new_unary_expr( ($( { $input:ident, $cast:ident, $func:expr } ),*) => { match (child_expr.return_type(), return_type.clone()) { $( - ($input! { type_match_pattern }, $cast! { type_match_pattern }) => Box::new( - UnaryExpression::< $input! { type_array }, $cast! { type_array }, _>::new( - child_expr, - return_type.clone(), - $func - ) - ), + ($input! { type_match_pattern }, $cast! { type_match_pattern }) => gen_cast_impl!(arm: $input, $cast, $func), )* _ => { return Err(ExprError::UnsupportedCast(child_expr.return_type(), return_type)); } } }; + (arm: $input:ident, varchar, $func:expr) => { + UnaryBytesExpression::< $input! { type_array }, _>::new( + child_expr, + return_type.clone(), + $func + ).boxed() + }; + (arm: $input:ident, $cast:ident, $func:expr) => { + UnaryExpression::< $input! { type_array }, $cast! { type_array }, _>::new( + child_expr, + return_type.clone(), + $func + ).boxed() + }; } for_all_cast_variants! { gen_cast_impl } } - (ProstType::BoolOut, _, DataType::Boolean) => { - Box::new(UnaryExpression::::new( - child_expr, - return_type, - bool_out, - )) - } + (ProstType::BoolOut, _, DataType::Boolean) => Box::new( + UnaryBytesExpression::::new(child_expr, return_type, bool_out), + ), (ProstType::Not, _, _) => Box::new(BooleanUnaryExpression::new( child_expr, |a| BoolArray::new(!a.data() & a.null_bitmap(), a.null_bitmap().clone()), diff --git a/src/expr/src/expr/template.rs b/src/expr/src/expr/template.rs index 9ef29de1f3288..da6c722369ad4 100644 --- a/src/expr/src/expr/template.rs +++ b/src/expr/src/expr/template.rs @@ -19,10 +19,7 @@ use std::sync::Arc; use itertools::{multizip, Itertools}; use paste::paste; -use risingwave_common::array::{ - Array, ArrayBuilder, ArrayImpl, ArrayRef, DataChunk, StringWriter, Utf8Array, Utf8ArrayBuilder, - WrittenGuard, -}; +use risingwave_common::array::{Array, ArrayBuilder, ArrayImpl, ArrayRef, DataChunk, Utf8Array}; use risingwave_common::row::OwnedRow; use risingwave_common::types::{option_as_scalar_ref, DataType, Datum, Scalar}; @@ -100,12 +97,12 @@ macro_rules! eval_normal_row { } macro_rules! gen_expr_normal { - ($ty_name:ident, { $($arg:ident),* }, { $($lt:lifetime),* }) => { + ($ty_name:ident, { $($arg:ident),* }) => { paste! { pub struct $ty_name< $($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($($arg::RefItem<$lt>, )*) -> $crate::Result, + F: Fn($($arg::RefItem<'_>, )*) -> $crate::Result, > { $([]: BoxedExpression,)* return_type: DataType, @@ -115,7 +112,7 @@ macro_rules! gen_expr_normal { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($($arg::RefItem<$lt>, )*) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )*) -> $crate::Result + Sync + Send, > fmt::Debug for $ty_name<$($arg, )* OA, F> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($ty_name)) @@ -128,7 +125,7 @@ macro_rules! gen_expr_normal { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($($arg::RefItem<$lt>, )*) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )*) -> $crate::Result + Sync + Send, > Expression for $ty_name<$($arg, )* OA, F> where $(for<'a> &'a $arg: std::convert::From<&'a ArrayImpl>,)* @@ -143,7 +140,7 @@ macro_rules! gen_expr_normal { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($($arg::RefItem<$lt>, )*) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )*) -> $crate::Result + Sync + Send, > $ty_name<$($arg, )* OA, F> { #[allow(dead_code)] pub fn new( @@ -167,8 +164,9 @@ macro_rules! eval_bytes { ($self:ident, $output_array:ident, $($arg:ident,)*) => { if let ($(Some($arg), )*) = ($($arg, )*) { { - let writer = $output_array.writer(); - let _guard = ($self.func)($($arg, )* writer)?; + let mut writer = $output_array.writer().begin(); + ($self.func)($($arg, )* &mut writer)?; + writer.finish(); } } else { $output_array.append(None); @@ -178,11 +176,9 @@ macro_rules! eval_bytes { macro_rules! eval_bytes_row { ($self:ident, $($arg:ident,)*) => { if let ($(Some($arg), )*) = ($($arg, )*) { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = ($self.func)($($arg, )* writer)?; - let array = builder.finish(); - array.into_single_value() // take the single value from the array + let mut writer = String::new(); + ($self.func)($($arg, )* &mut writer)?; + Some(Box::::from(writer)) } else { None } @@ -190,11 +186,11 @@ macro_rules! eval_bytes_row { } macro_rules! gen_expr_bytes { - ($ty_name:ident, { $($arg:ident),* }, { $($lt:lifetime),* }) => { + ($ty_name:ident, { $($arg:ident),* }) => { paste! { pub struct $ty_name< $($arg: Array, )* - F: for<$($lt),*, 'writer> Fn($($arg::RefItem<$lt>, )* StringWriter<'writer>) -> $crate::Result, + F: Fn($($arg::RefItem<'_>, )* &mut dyn std::fmt::Write) -> $crate::Result<()>, > { $([]: BoxedExpression,)* return_type: DataType, @@ -203,7 +199,7 @@ macro_rules! gen_expr_bytes { } impl<$($arg: Array, )* - F: for<$($lt),*, 'writer> Fn($($arg::RefItem<$lt>, )* StringWriter<'writer>) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )* &mut dyn std::fmt::Write) -> $crate::Result<()> + Sync + Send, > fmt::Debug for $ty_name<$($arg, )* F> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($ty_name)) @@ -215,7 +211,7 @@ macro_rules! gen_expr_bytes { } impl<$($arg: Array, )* - F: for<$($lt),*, 'writer> Fn($($arg::RefItem<$lt>, )* StringWriter<'writer>) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )* &mut dyn std::fmt::Write) -> $crate::Result<()> + Sync + Send, > Expression for $ty_name<$($arg, )* F> where $(for<'a> &'a $arg: std::convert::From<&'a ArrayImpl>,)* @@ -228,7 +224,7 @@ macro_rules! gen_expr_bytes { } impl<$($arg: Array, )* - F: for<$($lt),*, 'writer> Fn($($arg::RefItem<$lt>, )* StringWriter<'writer>) -> $crate::Result + Sized + Sync + Send, + F: Fn($($arg::RefItem<'_>, )* &mut dyn std::fmt::Write) -> $crate::Result<()> + Sync + Send, > $ty_name<$($arg, )* F> { pub fn new( $([]: BoxedExpression, )* @@ -262,12 +258,12 @@ macro_rules! eval_nullable_row { } macro_rules! gen_expr_nullable { - ($ty_name:ident, { $($arg:ident),* }, { $($lt:lifetime),* }) => { + ($ty_name:ident, { $($arg:ident),* }) => { paste! { pub struct $ty_name< $($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($(Option<$arg::RefItem<$lt>>, )*) -> $crate::Result>, + F: Fn($(Option<$arg::RefItem<'_>>, )*) -> $crate::Result>, > { $([]: BoxedExpression,)* return_type: DataType, @@ -277,7 +273,7 @@ macro_rules! gen_expr_nullable { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($(Option<$arg::RefItem<$lt>>, )*) -> $crate::Result> + Sized + Sync + Send, + F: Fn($(Option<$arg::RefItem<'_>>, )*) -> $crate::Result> + Sync + Send, > fmt::Debug for $ty_name<$($arg, )* OA, F> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct(stringify!($ty_name)) @@ -290,7 +286,7 @@ macro_rules! gen_expr_nullable { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($(Option<$arg::RefItem<$lt>>, )*) -> $crate::Result> + Sized + Sync + Send, + F: Fn($(Option<$arg::RefItem<'_>>, )*) -> $crate::Result> + Sync + Send, > Expression for $ty_name<$($arg, )* OA, F> where $(for<'a> &'a $arg: std::convert::From<&'a ArrayImpl>,)* @@ -305,7 +301,7 @@ macro_rules! gen_expr_nullable { impl<$($arg: Array, )* OA: Array, - F: for<$($lt),*> Fn($(Option<$arg::RefItem<$lt>>, )*) -> $crate::Result> + Sized + Sync + Send, + F: Fn($(Option<$arg::RefItem<'_>>, )*) -> $crate::Result> + Sync + Send, > $ty_name<$($arg, )* OA, F> { // Compile failed due to some GAT lifetime issues so make this field private. // Check issues #742. @@ -326,16 +322,16 @@ macro_rules! gen_expr_nullable { } } -gen_expr_normal!(UnaryExpression, { IA1 }, { 'ia1 }); -gen_expr_normal!(BinaryExpression, { IA1, IA2 }, { 'ia1, 'ia2 }); -gen_expr_normal!(TernaryExpression, { IA1, IA2, IA3 }, { 'ia1, 'ia2, 'ia3 }); +gen_expr_normal!(UnaryExpression, { IA1 }); +gen_expr_normal!(BinaryExpression, { IA1, IA2 }); +gen_expr_normal!(TernaryExpression, { IA1, IA2, IA3 }); -gen_expr_bytes!(UnaryBytesExpression, { IA1 }, { 'ia1 }); -gen_expr_bytes!(BinaryBytesExpression, { IA1, IA2 }, { 'ia1, 'ia2 }); -gen_expr_bytes!(TernaryBytesExpression, { IA1, IA2, IA3 }, { 'ia1, 'ia2, 'ia3 }); -gen_expr_bytes!(QuaternaryBytesExpression, { IA1, IA2, IA3, IA4 }, { 'ia1, 'ia2, 'ia3, 'ia4 }); +gen_expr_bytes!(UnaryBytesExpression, { IA1 }); +gen_expr_bytes!(BinaryBytesExpression, { IA1, IA2 }); +gen_expr_bytes!(TernaryBytesExpression, { IA1, IA2, IA3 }); +gen_expr_bytes!(QuaternaryBytesExpression, { IA1, IA2, IA3, IA4 }); -gen_expr_nullable!(BinaryNullableExpression, { IA1, IA2 }, { 'ia1, 'ia2 }); +gen_expr_nullable!(BinaryNullableExpression, { IA1, IA2 }); /// `for_all_cmp_types` helps in matching and casting types when building comparison expressions /// such as <= or IS DISTINCT FROM. diff --git a/src/expr/src/vector_op/cast.rs b/src/expr/src/vector_op/cast.rs index 90ff5538090a9..956792da5dd3a 100644 --- a/src/expr/src/vector_op/cast.rs +++ b/src/expr/src/vector_op/cast.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::any::type_name; +use std::fmt::Write; use std::str::FromStr; use chrono::{DateTime, NaiveDate, NaiveDateTime, NaiveTime, Utc}; @@ -248,9 +249,10 @@ pub fn parse_bytes_traditional(s: &str) -> Result> { Ok(out) } -pub fn timestampz_to_utc_string(elem: i64) -> Box { - elem.to_text_with_type(&DataType::Timestampz) - .into_boxed_str() +pub fn timestampz_to_utc_string(elem: i64, mut writer: &mut dyn Write) -> Result<()> { + elem.write_with_type(&DataType::Timestampz, &mut writer) + .unwrap(); + Ok(()) } #[inline(always)] @@ -376,18 +378,23 @@ pub fn int32_to_bool(input: i32) -> Result { // For most of the types, cast them to varchar is similar to return their text format. // So we use this function to cast type to varchar. -pub fn general_to_text(elem: T) -> Result> { - Ok(elem.to_text().into_boxed_str()) +pub fn general_to_text(elem: impl ToText, mut writer: &mut dyn Write) -> Result<()> { + elem.write(&mut writer).unwrap(); + Ok(()) } -pub fn bool_to_varchar(input: bool) -> Result> { - Ok(if input { "true" } else { "false" }.into()) +pub fn bool_to_varchar(input: bool, writer: &mut dyn Write) -> Result<()> { + writer + .write_str(if input { "true" } else { "false" }) + .unwrap(); + Ok(()) } /// `bool_out` is different from `general_to_string` to produce a single char. `PostgreSQL` /// uses different variants of bool-to-string in different situations. -pub fn bool_out(input: bool) -> Result> { - Ok(if input { "t" } else { "f" }.into()) +pub fn bool_out(input: bool, writer: &mut dyn Write) -> Result<()> { + writer.write_str(if input { "t" } else { "f" }).unwrap(); + Ok(()) } /// It accepts a macro whose input is `{ $input:ident, $cast:ident, $func:expr }` tuples @@ -426,8 +433,8 @@ macro_rules! for_all_cast_variants { { interval, varchar, general_to_text }, { date, varchar, general_to_text }, { timestamp, varchar, general_to_text }, - { timestampz, varchar, |x| Ok(timestampz_to_utc_string(x)) }, - { list, varchar, |x| general_to_text(x) }, + { timestampz, varchar, timestampz_to_utc_string }, + { list, varchar, |x, w| general_to_text(x, w) }, { boolean, int32, general_cast }, { int32, boolean, int32_to_bool }, @@ -628,17 +635,28 @@ fn scalar_cast( ($( { $input:ident, $cast:ident, $func:expr } ),*) => { match (source_type, target_type) { $( - ($input! { type_match_pattern }, $cast! { type_match_pattern }) => { - let source: <$input! { type_array } as Array>::RefItem<'_> = source.try_into()?; - let target: Result<<$cast! { type_array } as Array>::OwnedItem> = $func(source); - target.map(Scalar::to_scalar_value) - } + ($input! { type_match_pattern }, $cast! { type_match_pattern }) => gen_cast_impl!(arm: $input, $cast, $func), )* _ => { return Err(ExprError::UnsupportedCast(source_type.clone(), target_type.clone())); } } }; + (arm: $input:ident, varchar, $func:expr) => { + { + let source: <$input! { type_array } as Array>::RefItem<'_> = source.try_into()?; + let mut writer = String::new(); + let target: Result<()> = $func(source, &mut writer); + target.map(|_| Scalar::to_scalar_value(writer.into_boxed_str())) + } + }; + (arm: $input:ident, $cast:ident, $func:expr) => { + { + let source: <$input! { type_array } as Array>::RefItem<'_> = source.try_into()?; + let target: Result<<$cast! { type_array } as Array>::OwnedItem> = $func(source); + target.map(Scalar::to_scalar_value) + } + }; } for_all_cast_variants!(gen_cast_impl) } @@ -703,8 +721,10 @@ mod tests { use super::*; macro_rules! test { - ($expr:expr, $right:literal) => { - assert_eq!($expr.unwrap().as_ref(), $right); + ($fn:ident($value:expr), $right:literal) => { + let mut writer = String::new(); + $fn($value, &mut writer).unwrap(); + assert_eq!(writer, $right); }; } @@ -970,14 +990,19 @@ mod tests { #[test] fn test_timestampz() { let str1 = "0001-11-15 15:35:40.999999+08:00"; - let str1_utc0 = "0001-11-15 07:35:40.999999+00:00"; let timestampz1 = str_to_timestampz(str1).unwrap(); assert_eq!(timestampz1, -62108094259000001); - assert_eq!(timestampz_to_utc_string(timestampz1).as_ref(), str1_utc0); + + let mut writer = String::new(); + timestampz_to_utc_string(timestampz1, &mut writer).unwrap(); + assert_eq!(writer, "0001-11-15 07:35:40.999999+00:00"); let str2 = "1969-12-31 23:59:59.999999+00:00"; let timestampz2 = str_to_timestampz(str2).unwrap(); assert_eq!(timestampz2, -1); - assert_eq!(timestampz_to_utc_string(timestampz2).as_ref(), str2); + + let mut writer = String::new(); + timestampz_to_utc_string(timestampz2, &mut writer).unwrap(); + assert_eq!(writer, str2); } } diff --git a/src/expr/src/vector_op/concat_op.rs b/src/expr/src/vector_op/concat_op.rs index d10fa0844d910..3c16d6ffc8fbd 100644 --- a/src/expr/src/vector_op/concat_op.rs +++ b/src/expr/src/vector_op/concat_op.rs @@ -12,31 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; #[inline(always)] -pub fn concat_op(left: &str, right: &str, writer: StringWriter<'_>) -> Result { - let mut writer = writer.begin(); - writer.write_ref(left); - writer.write_ref(right); - Ok(writer.finish()) +pub fn concat_op(left: &str, right: &str, writer: &mut dyn Write) -> Result<()> { + writer.write_str(left).unwrap(); + writer.write_str(right).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] fn test_concat_op() { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = concat_op("114", "514", writer).unwrap(); - let array = builder.finish(); - - assert_eq!(array.value_at(0).unwrap(), "114514".to_owned()) + let mut s = String::new(); + concat_op("114", "514", &mut s).unwrap(); + assert_eq!(s, "114514") } } diff --git a/src/expr/src/vector_op/lower.rs b/src/expr/src/vector_op/lower.rs index 81e6410065de7..349a522524a7f 100644 --- a/src/expr/src/vector_op/lower.rs +++ b/src/expr/src/vector_op/lower.rs @@ -14,23 +14,18 @@ use std::fmt::Write; -use risingwave_common::array::{StringWriter, WrittenGuard}; - use crate::Result; #[inline(always)] -pub fn lower(s: &str, writer: StringWriter<'_>) -> Result { - let mut writer = writer.begin(); +pub fn lower(s: &str, writer: &mut dyn Write) -> Result<()> { for c in s.chars() { writer.write_char(c.to_ascii_lowercase()).unwrap(); } - Ok(writer.finish()) + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -42,12 +37,9 @@ mod tests { ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = lower(s, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + lower(s, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/ltrim.rs b/src/expr/src/vector_op/ltrim.rs index d7911edab3e22..465ccfb7cf0a1 100644 --- a/src/expr/src/vector_op/ltrim.rs +++ b/src/expr/src/vector_op/ltrim.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; @@ -20,13 +20,13 @@ use crate::Result; /// are actually different when the string is in right-to-left languages like Arabic or Hebrew. /// Since we would like to simplify the implementation, currently we omit this case. #[inline(always)] -pub fn ltrim(s: &str, writer: StringWriter<'_>) -> Result { - Ok(writer.write_ref(s.trim_start())) +pub fn ltrim(s: &str, writer: &mut dyn Write) -> Result<()> { + writer.write_str(s.trim_start()).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; use super::*; @@ -38,12 +38,9 @@ mod tests { ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = ltrim(s, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + ltrim(s, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/md5.rs b/src/expr/src/vector_op/md5.rs index a0dbceae2616c..271618a3d85e4 100644 --- a/src/expr/src/vector_op/md5.rs +++ b/src/expr/src/vector_op/md5.rs @@ -14,21 +14,16 @@ use std::fmt::Write; -use risingwave_common::array::{StringWriter, WrittenGuard}; - use crate::Result; #[inline(always)] -pub fn md5(s: &str, writer: StringWriter<'_>) -> Result { - let mut writer = writer.begin(); +pub fn md5(s: &str, writer: &mut dyn Write) -> Result<()> { write!(writer, "{:x}", ::md5::compute(s)).unwrap(); - Ok(writer.finish()) + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -43,12 +38,9 @@ mod tests { ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = md5(s, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + md5(s, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/overlay.rs b/src/expr/src/vector_op/overlay.rs index d59ce07b90b99..19ac95e9c4aef 100644 --- a/src/expr/src/vector_op/overlay.rs +++ b/src/expr/src/vector_op/overlay.rs @@ -12,17 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; #[inline(always)] -pub fn overlay( - s: &str, - new_sub_str: &str, - start: i32, - writer: StringWriter<'_>, -) -> Result { +pub fn overlay(s: &str, new_sub_str: &str, start: i32, writer: &mut dyn Write) -> Result<()> { // If count is omitted, it defaults to the length of new_sub_str. overlay_for(s, new_sub_str, start, new_sub_str.len() as i32, writer) } @@ -33,8 +28,8 @@ pub fn overlay_for( new_sub_str: &str, start: i32, count: i32, - writer: StringWriter<'_>, -) -> Result { + writer: &mut dyn Write, +) -> Result<()> { let count = count.max(0) as usize; // If start is out of range, attach it to the end. @@ -43,21 +38,18 @@ pub fn overlay_for( let remaining = start + count; - let mut writer = writer.begin(); - writer.write_ref(&s[..start]); - writer.write_ref(new_sub_str); + writer.write_str(&s[..start]).unwrap(); + writer.write_str(new_sub_str).unwrap(); if remaining < s.len() { - writer.write_ref(&s[remaining..]); + writer.write_str(&s[remaining..]).unwrap(); } - Ok(writer.finish()) + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -82,16 +74,13 @@ mod tests { ]; for (s, new_sub_str, start, count, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = match count { - None => overlay(s, new_sub_str, start, writer), - Some(count) => overlay_for(s, new_sub_str, start, count, writer), + let mut writer = String::new(); + match count { + None => overlay(s, new_sub_str, start, &mut writer), + Some(count) => overlay_for(s, new_sub_str, start, count, &mut writer), } .unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + assert_eq!(writer, expected); } } } diff --git a/src/expr/src/vector_op/repeat.rs b/src/expr/src/vector_op/repeat.rs index c1ea57a694d45..60861f7844c64 100644 --- a/src/expr/src/vector_op/repeat.rs +++ b/src/expr/src/vector_op/repeat.rs @@ -12,23 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; #[inline(always)] -pub fn repeat(s: &str, count: i32, writer: StringWriter<'_>) -> Result { - let mut writer = writer.begin(); +pub fn repeat(s: &str, count: i32, writer: &mut dyn Write) -> Result<()> { for _ in 0..count { - writer.write_ref(s); + writer.write_str(s).unwrap(); } - Ok(writer.finish()) + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -41,12 +38,9 @@ mod tests { ]; for (s, count, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = repeat(s, count, writer).unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + repeat(s, count, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/replace.rs b/src/expr/src/vector_op/replace.rs index 3f636c9173670..75c6acc3bc20b 100644 --- a/src/expr/src/vector_op/replace.rs +++ b/src/expr/src/vector_op/replace.rs @@ -12,40 +12,33 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; #[inline(always)] -pub fn replace( - s: &str, - from_str: &str, - to_str: &str, - writer: StringWriter<'_>, -) -> Result { +pub fn replace(s: &str, from_str: &str, to_str: &str, writer: &mut dyn Write) -> Result<()> { if from_str.is_empty() { - return Ok(writer.write_ref(s)); + writer.write_str(s).unwrap(); + return Ok(()); } let mut last = 0; - let mut writer = writer.begin(); while let Some(mut start) = s[last..].find(from_str) { start += last; - writer.write_ref(&s[last..start]); - writer.write_ref(to_str); + writer.write_str(&s[last..start]).unwrap(); + writer.write_str(to_str).unwrap(); last = start + from_str.len(); } - writer.write_ref(&s[last..]); - Ok(writer.finish()) + writer.write_str(&s[last..]).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] - fn test_replace() { + fn test_replace() -> Result<()> { let cases = vec![ ("hello, word", "我的", "world", "hello, word"), ("hello, word", "", "world", "hello, word"), @@ -56,12 +49,10 @@ mod tests { ]; for (s, from_str, to_str, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = replace(s, from_str, to_str, writer).unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + replace(s, from_str, to_str, &mut writer)?; + assert_eq!(writer, expected); } + Ok(()) } } diff --git a/src/expr/src/vector_op/rtrim.rs b/src/expr/src/vector_op/rtrim.rs index fafd98e63783b..fbef0dc451c7f 100644 --- a/src/expr/src/vector_op/rtrim.rs +++ b/src/expr/src/vector_op/rtrim.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; @@ -20,14 +20,13 @@ use crate::Result; /// are actually different when the string is in right-to-left languages like Arabic or Hebrew. /// Since we would like to simplify the implementation, currently we omit this case. #[inline(always)] -pub fn rtrim(s: &str, writer: StringWriter<'_>) -> Result { - Ok(writer.write_ref(s.trim_end())) +pub fn rtrim(s: &str, writer: &mut dyn Write) -> Result<()> { + writer.write_str(s.trim_end()).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -38,12 +37,9 @@ mod tests { ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = rtrim(s, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + rtrim(s, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/split_part.rs b/src/expr/src/vector_op/split_part.rs index c3f173bca9302..9aa13733c462a 100644 --- a/src/expr/src/vector_op/split_part.rs +++ b/src/expr/src/vector_op/split_part.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::{ExprError, Result}; @@ -21,8 +21,8 @@ pub fn split_part( string_expr: &str, delimiter_expr: &str, nth_expr: i32, - writer: StringWriter<'_>, -) -> Result { + writer: &mut dyn Write, +) -> Result<()> { if nth_expr == 0 { return Err(ExprError::InvalidParam { name: "data", @@ -61,14 +61,12 @@ pub fn split_part( } } }; - - Ok(writer.write_ref(nth_val)) + writer.write_str(nth_val).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::split_part; #[test] @@ -101,22 +99,12 @@ mod tests { for (i, case @ (string_expr, delimiter_expr, nth_expr, expected)) in cases.iter().enumerate() { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let actual = split_part(string_expr, delimiter_expr, *nth_expr, writer); - - match actual { - Ok(_guard) => { - let expected = expected.unwrap(); - let array = builder.finish(); - let actual = array.value_at(0).unwrap(); - - assert_eq!(expected, actual, "\nat case {i}: {:?}\n", case) - } - Err(_err) => { - assert!(expected.is_none()); - } + let mut writer = String::new(); + let actual = match split_part(string_expr, delimiter_expr, *nth_expr, &mut writer) { + Ok(_) => Some(writer.as_str()), + Err(_) => None, }; + assert_eq!(&actual, expected, "\nat case {i}: {:?}\n", case); } } } diff --git a/src/expr/src/vector_op/substr.rs b/src/expr/src/vector_op/substr.rs index 69c9fe660078c..1499c84af7455 100644 --- a/src/expr/src/vector_op/substr.rs +++ b/src/expr/src/vector_op/substr.rs @@ -13,42 +13,37 @@ // limitations under the License. use std::cmp::{max, min}; - -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::{bail, Result}; #[inline(always)] -pub fn substr_start(s: &str, start: i32, writer: StringWriter<'_>) -> Result { +pub fn substr_start(s: &str, start: i32, writer: &mut dyn Write) -> Result<()> { let start = min(max(start - 1, 0) as usize, s.len()); - Ok(writer.write_ref(&s[start..])) + writer.write_str(&s[start..]).unwrap(); + Ok(()) } #[inline(always)] -pub fn substr_for(s: &str, count: i32, writer: StringWriter<'_>) -> Result { +pub fn substr_for(s: &str, count: i32, writer: &mut dyn Write) -> Result<()> { let end = min(count as usize, s.len()); - Ok(writer.write_ref(&s[..end])) + writer.write_str(&s[..end]).unwrap(); + Ok(()) } #[inline(always)] -pub fn substr_start_for( - s: &str, - start: i32, - count: i32, - writer: StringWriter<'_>, -) -> Result { +pub fn substr_start_for(s: &str, start: i32, count: i32, writer: &mut dyn Write) -> Result<()> { if count < 0 { bail!("length in substr should be non-negative: {}", count); } let begin = max(start - 1, 0) as usize; let end = min(max(start - 1 + count, 0) as usize, s.len()); - Ok(writer.write_ref(&s[begin..end])) + writer.write_str(&s[begin..end]).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -65,27 +60,22 @@ mod tests { ]; for (s, off, len, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let _guard = { - let writer = builder.writer(); - match (off, len) { - (Some(off), Some(len)) => { - let result = substr_start_for(&s, off, len, writer); - if len < 0 { - assert!(result.is_err()); - continue; - } else { - result? - } + let mut writer = String::new(); + match (off, len) { + (Some(off), Some(len)) => { + let result = substr_start_for(&s, off, len, &mut writer); + if len < 0 { + assert!(result.is_err()); + continue; + } else { + result? } - (Some(off), None) => substr_start(&s, off, writer)?, - (None, Some(len)) => substr_for(&s, len, writer)?, - _ => unreachable!(), } - }; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + (Some(off), None) => substr_start(&s, off, &mut writer)?, + (None, Some(len)) => substr_for(&s, len, &mut writer)?, + _ => unreachable!(), + } + assert_eq!(writer, expected); } Ok(()) } diff --git a/src/expr/src/vector_op/to_char.rs b/src/expr/src/vector_op/to_char.rs index 695440b2bc0a5..bc538a2d5906f 100644 --- a/src/expr/src/vector_op/to_char.rs +++ b/src/expr/src/vector_op/to_char.rs @@ -12,13 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt::Debug; +use std::fmt::{Debug, Write}; use std::sync::LazyLock; use aho_corasick::{AhoCorasick, AhoCorasickBuilder}; use chrono::format::StrftimeItems; use ouroboros::self_referencing; -use risingwave_common::array::{StringWriter, WrittenGuard}; use risingwave_common::types::NaiveDateTimeWrapper; use crate::Result; @@ -75,12 +74,14 @@ pub fn compile_pattern_to_chrono(tmpl: &str) -> ChronoPattern { pub fn to_char_timestamp( data: NaiveDateTimeWrapper, tmpl: &str, - writer: StringWriter<'_>, -) -> Result { + writer: &mut dyn Write, +) -> Result<()> { let pattern = compile_pattern_to_chrono(tmpl); - let res = data - .0 - .format_with_items(pattern.borrow_items().iter()) - .to_string(); - Ok(writer.write_ref(&res)) + write!( + writer, + "{}", + data.0.format_with_items(pattern.borrow_items().iter()) + ) + .unwrap(); + Ok(()) } diff --git a/src/expr/src/vector_op/translate.rs b/src/expr/src/vector_op/translate.rs index 0ecb8279f99f4..d617168aa2ad9 100644 --- a/src/expr/src/vector_op/translate.rs +++ b/src/expr/src/vector_op/translate.rs @@ -13,8 +13,7 @@ // limitations under the License. use std::collections::HashMap; - -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; @@ -23,8 +22,8 @@ pub fn translate( s: &str, match_str: &str, replace_str: &str, - writer: StringWriter<'_>, -) -> Result { + writer: &mut dyn Write, +) -> Result<()> { let mut char_map = HashMap::new(); let mut match_chars = match_str.chars(); let mut replace_chars = replace_str.chars(); @@ -44,14 +43,14 @@ pub fn translate( Some(None) => None, None => Some(c), }); - - Ok(writer.write_from_char_iter(iter)) + for c in iter { + writer.write_char(c).unwrap(); + } + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -73,14 +72,10 @@ mod tests { ]; for (s, match_str, replace_str, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = translate(s, match_str, replace_str, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + translate(s, match_str, replace_str, &mut writer)?; + assert_eq!(writer, expected); } - Ok(()) } } diff --git a/src/expr/src/vector_op/trim.rs b/src/expr/src/vector_op/trim.rs index 4e8262e1d743f..63c50158eb081 100644 --- a/src/expr/src/vector_op/trim.rs +++ b/src/expr/src/vector_op/trim.rs @@ -12,37 +12,32 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; #[inline(always)] -pub fn trim(s: &str, writer: StringWriter<'_>) -> Result { - Ok(writer.write_ref(s.trim())) +pub fn trim(s: &str, writer: &mut dyn Write) -> Result<()> { + writer.write_str(s.trim()).unwrap(); + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] - fn test_trim() { + fn test_trim() -> Result<()> { let cases = [ (" Hello\tworld\t", "Hello\tworld"), (" 空I ❤️ databases空 ", "空I ❤️ databases空"), ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - { - let writer = builder.writer(); - let _guard = trim(s, writer).unwrap(); - } - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + trim(s, &mut writer)?; + assert_eq!(writer, expected); } + Ok(()) } } diff --git a/src/expr/src/vector_op/trim_characters.rs b/src/expr/src/vector_op/trim_characters.rs index e0ad4a5ea096d..95492e28c71c1 100644 --- a/src/expr/src/vector_op/trim_characters.rs +++ b/src/expr/src/vector_op/trim_characters.rs @@ -12,19 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. -use risingwave_common::array::{StringWriter, WrittenGuard}; +use std::fmt::Write; use crate::Result; macro_rules! gen_trim { ($( { $func_name:ident, $method:ident }),*) => { $(#[inline(always)] - pub fn $func_name(s: &str, characters: &str, writer: StringWriter<'_>) -> Result { + pub fn $func_name(s: &str, characters: &str, writer: &mut dyn Write) -> Result<()> { let pattern = |c| characters.chars().any(|ch| ch == c); // We remark that feeding a &str and a slice of chars into trim_left/right_matches // means different, one is matching with the entire string and the other one is matching // with any char in the slice. - Ok(writer.write_ref(s.$method(pattern))) + Ok(writer.write_str(s.$method(pattern)).unwrap()) })* } } @@ -37,12 +37,10 @@ gen_trim! { #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] - fn test_trim_characters() { + fn test_trim_characters() -> Result<()> { let cases = [ ("Hello world", "Hdl", "ello wor"), ("abcde", "aae", "bcd"), @@ -50,17 +48,15 @@ mod tests { ]; for (s, characters, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = trim_characters(s, characters, writer).unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + trim_characters(s, characters, &mut writer)?; + assert_eq!(writer, expected); } + Ok(()) } #[test] - fn test_ltrim_characters() { + fn test_ltrim_characters() -> Result<()> { let cases = [ ("Hello world", "Hdl", "ello world"), ("abcde", "aae", "bcde"), @@ -68,17 +64,15 @@ mod tests { ]; for (s, characters, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = ltrim_characters(s, characters, writer).unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + ltrim_characters(s, characters, &mut writer)?; + assert_eq!(writer, expected); } + Ok(()) } #[test] - fn test_rtrim_characters() { + fn test_rtrim_characters() -> Result<()> { let cases = [ ("Hello world", "Hdl", "Hello wor"), ("abcde", "aae", "abcd"), @@ -86,12 +80,10 @@ mod tests { ]; for (s, characters, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = rtrim_characters(s, characters, writer).unwrap(); - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + rtrim_characters(s, characters, &mut writer)?; + assert_eq!(writer, expected); } + Ok(()) } } diff --git a/src/expr/src/vector_op/upper.rs b/src/expr/src/vector_op/upper.rs index 5a22b886585f0..117a52dd81eeb 100644 --- a/src/expr/src/vector_op/upper.rs +++ b/src/expr/src/vector_op/upper.rs @@ -14,23 +14,18 @@ use std::fmt::Write; -use risingwave_common::array::{StringWriter, WrittenGuard}; - use crate::Result; #[inline(always)] -pub fn upper(s: &str, writer: StringWriter<'_>) -> Result { - let mut writer = writer.begin(); +pub fn upper(s: &str, writer: &mut dyn Write) -> Result<()> { for c in s.chars() { writer.write_char(c.to_ascii_uppercase()).unwrap(); } - Ok(writer.finish()) + Ok(()) } #[cfg(test)] mod tests { - use risingwave_common::array::{Array, ArrayBuilder, Utf8ArrayBuilder}; - use super::*; #[test] @@ -42,12 +37,9 @@ mod tests { ]; for (s, expected) in cases { - let mut builder = Utf8ArrayBuilder::new(1); - let writer = builder.writer(); - let _guard = upper(s, writer)?; - let array = builder.finish(); - let v = array.value_at(0).unwrap(); - assert_eq!(v, expected); + let mut writer = String::new(); + upper(s, &mut writer)?; + assert_eq!(writer, expected); } Ok(()) }