[sql] Cleanup arc-strings.

Leonid Ryzhyk · Leonid Ryzhyk · commit 97bae96ec00d · 2025-02-04T00:03:57.000-08:00
* Fixup benchmarks
* Implement rkyv traits for SqlString (the Arc version only)
* Feature-gate the internment dependency.
* Make the `Arc&lt;&gt;` version the default. The interned version is enabled
  by the `interned_string` feature.

Signed-off-by: Leonid Ryzhyk &lt;leonid@feldera.com&gt;
diff --git a/benchmark/feldera-sql/benchmarks/nexmark/queries/q21.rs b/benchmark/feldera-sql/benchmarks/nexmark/queries/q21.rs
@@ -1,26 +1,27 @@
 use dashmap::DashMap;
 use regex::Regex;
 use std::sync::LazyLock;
+use feldera_sqllib::*;
 
-static REGEXS: LazyLock<DashMap<String, Regex>> = LazyLock::new(|| DashMap::new());
+static REGEXS: LazyLock<DashMap<SqlString, Regex>> = LazyLock::new(|| DashMap::new());
 
 pub fn re_extract(
-    s: Option<String>,
-    p: Option<String>,
+    s: Option<SqlString>,
+    p: Option<SqlString>,
     group: Option<i32>,
-) -> Result<Option<String>, Box<dyn std::error::Error>> {
+) -> Result<Option<SqlString>, Box<dyn std::error::Error>> {
     Ok(do_re_extract(s, p, group))
 }
 
-fn do_re_extract(s: Option<String>, p: Option<String>, group: Option<i32>) -> Option<String> {
+fn do_re_extract(s: Option<SqlString>, p: Option<SqlString>, group: Option<i32>) -> Option<SqlString> {
     let s = s?;
     let p = p?;
     let group = group?;
 
     let re = REGEXS
         .entry(p.clone())
-        .or_try_insert_with(|| Regex::new(&p))
+        .or_try_insert_with(|| Regex::new(p.str()))
         .ok()?;
 
-    Some(re.captures(&s)?.get(group as usize)?.as_str().to_string())
+    Some(SqlString::from_ref(re.captures(s.str())?.get(group as usize)?.as_str()))
 }
diff --git a/crates/dbsp/src/hash.rs b/crates/dbsp/src/hash.rs
@@ -4,7 +4,7 @@ use std::hash::{Hash, Hasher};
 use xxhash_rust::xxh3::Xxh3Default;
 
 /// Default hashing function used to shard records across workers.
-pub fn default_hash<T: Hash>(x: &T) -> u64 {
+pub fn default_hash<T: Hash + ?Sized>(x: &T) -> u64 {
     let mut hasher = Xxh3Default::new();
     x.hash(&mut hasher);
     hasher.finish()
diff --git a/crates/sqllib/Cargo.toml b/crates/sqllib/Cargo.toml
@@ -11,7 +11,8 @@ categories = ["database", "api-bindings", "network-programming"]
 publish = true
 
 [features]
-arcstring = []
+default = []
+interned_string = ["internment"]
 
 [dependencies]
 thiserror = "1.0"
@@ -37,7 +38,7 @@ flate2 = "1.0.28"
 metrics = { version = "0.23.0" }
 base64 = "0.22.1"
 uuid = { version = "1.11.0", features = ["v4", "std"] }
-internment = { version = "0.8.5", features = ["arc"] }
+internment = { version = "0.8.5", features = ["arc"], optional = true }
 
 [dev-dependencies]
 serde_json = "1.0.107"
diff --git a/crates/sqllib/src/string.rs b/crates/sqllib/src/string.rs
@@ -1,4 +1,10 @@
-//! SQL String operations
+//! SQL String operations.
+//!
+
+// This module contains two implementations of an immutable reference-counted
+// string type: a version wrapped in `Arc` and an interned string.  The former
+// is enabled by default.  The latter misses rkyv trait implementations and
+// has poor performance in preliminary benchmarks.
 
 #![allow(non_snake_case)]
 use crate::{
@@ -8,47 +14,62 @@ use crate::{
 
 use core::fmt::Error;
 use feldera_types::{deserialize_without_context, serialize_without_context};
-#[cfg(not(feature = "arcstring"))]
+#[cfg(feature = "interned_string")]
 use internment::ArcIntern;
 use like::{Escape, Like};
 use regex::Regex;
-use rkyv::Fallible;
-use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Serialize};
 use size_of::{Context, SizeOf};
 use std::{
-    borrow::Cow,
     cmp::{max, min},
     fmt::{Display, Formatter},
     sync::Arc,
 };
 
-#[cfg(not(feature = "arcstring"))]
+#[cfg(feature = "interned_string")]
 type StringRef = ArcIntern<String>;
 
-#[cfg(feature = "arcstring")]
+#[cfg(not(feature = "interned_string"))]
 type StringRef = Arc<String>;
 
-#[derive(Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
+/// An immutable reference counted string.
+#[derive(
+    Clone,
+    Default,
+    Debug,
+    Eq,
+    Hash,
+    Ord,
+    PartialEq,
+    PartialOrd,
+    Serialize,
+    Deserialize,
+    rkyv::Archive,
+    rkyv::Serialize,
+    rkyv::Deserialize,
+)]
+#[archive_attr(derive(Ord, Eq, PartialEq, PartialOrd))]
+#[serde(transparent)]
 pub struct SqlString(StringRef);
 
 /// String representation used by the Feldera SQL runtime
 impl SqlString {
-    #[cfg(feature = "arcstring")]
+    #[cfg(not(feature = "interned_string"))]
     pub fn new() -> Self {
-        SqlString(StringRef::new("".to_string()))
+        Self::default()
     }
 
-    #[cfg(feature = "arcstring")]
+    #[cfg(not(feature = "interned_string"))]
     pub fn from_ref(value: &str) -> Self {
-        SqlString(StringRef::new(value))
+        SqlString(StringRef::from(value.to_string()))
     }
 
-    #[cfg(not(feature = "arcstring"))]
+    #[cfg(feature = "interned_string")]
     pub fn new() -> Self {
-        SqlString(StringRef::new("".to_string()))
+        Self::default()
     }
 
-    #[cfg(not(feature = "arcstring"))]
+    #[cfg(feature = "interned_string")]
     pub fn from_ref(value: &str) -> Self {
         SqlString(StringRef::from_ref(value))
     }
@@ -66,25 +87,6 @@ impl SqlString {
     }
 }
 
-impl Serialize for SqlString {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        self.str().serialize(serializer)
-    }
-}
-
-impl<'de> Deserialize<'de> for SqlString {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let str: Cow<'de, str> = Deserialize::deserialize(deserializer)?;
-        Ok(Self::from_ref(&str))
-    }
-}
-
 serialize_without_context!(SqlString);
 deserialize_without_context!(SqlString);
 
@@ -118,41 +120,6 @@ impl SizeOf for SqlString {
     }
 }
 
-impl rkyv::Archive for SqlString {
-    type Archived = ();
-    type Resolver = ();
-    unsafe fn resolve(&self, _pos: usize, _resolver: Self::Resolver, _out: *mut Self::Archived) {
-        todo!()
-    }
-}
-
-impl<D> rkyv::Deserialize<SqlString, D> for ()
-where
-    D: Fallible + ?Sized,
-{
-    fn deserialize(&self, _deserializer: &mut D) -> Result<SqlString, D::Error> {
-        todo!()
-    }
-}
-
-impl<D> rkyv::Deserialize<SqlString, D> for SqlString
-where
-    D: Fallible + ?Sized,
-{
-    fn deserialize(&self, _deserializer: &mut D) -> Result<SqlString, D::Error> {
-        todo!()
-    }
-}
-
-impl<S> rkyv::Serialize<S> for SqlString
-where
-    S: Fallible + ?Sized,
-{
-    fn serialize(&self, _serializer: &mut S) -> Result<Self::Resolver, S::Error> {
-        todo!()
-    }
-}
-
 ///////////////////////////////
 
 #[doc(hidden)]
diff --git a/crates/sqllib/src/timestamp.rs b/crates/sqllib/src/timestamp.rs
@@ -1244,7 +1244,7 @@ some_polymorphic_function2!(datediff_day, Date, Date, Date, Date, i32);
 
 #[doc(hidden)]
 pub fn format_date__(format: SqlString, date: Date) -> SqlString {
-    return SqlString::from(date.to_dateTime().format(format.str()).to_string());
+    SqlString::from(date.to_dateTime().format(format.str()).to_string())
 }
 
 some_function2!(format_date, SqlString, Date, SqlString);
diff --git a/sql-to-dbsp-compiler/temp/Cargo.toml b/sql-to-dbsp-compiler/temp/Cargo.toml
@@ -6,7 +6,7 @@ publish = false
 
 [features]
 default = []
-arcstring = ["feldera-sqllib/arcstring"]
+interned_string = ["feldera-sqllib/interned_string"]
 
 [dependencies]
 paste = { version = "1.0.12" }

Original file line number	Diff line number	Diff line change
`@@ -1244,7 +1244,7 @@ some_polymorphic_function2!(datediff_day, Date, Date, Date, Date, i32);`
`1244`	`1244`
`1245`	`1245`	`#[doc(hidden)]`
`1246`	`1246`	`pub fn format_date__(format: SqlString, date: Date) -> SqlString {`
`1247`		`- return SqlString::from(date.to_dateTime().format(format.str()).to_string());`
	`1247`	`+ SqlString::from(date.to_dateTime().format(format.str()).to_string())`
`1248`	`1248`	`}`
`1249`	`1249`
`1250`	`1250`	`some_function2!(format_date, SqlString, Date, SqlString);`