Merge pull request #1623 from ignatz/align_row_and_rows

Breaking change: Make Rows and Row API more consistent.
tursodatabase · Aug 8, 2024 · 9120ce6 · 9120ce6
2 parents b97d37d + f510359
commit 9120ce6
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 55 deletions.
diff --git a/libsql/src/de.rs b/libsql/src/de.rs
@@ -68,7 +68,7 @@ impl<'de> Deserializer<'de> for RowDeserializer<'de> {
 
         visitor.visit_map(RowMapAccess {
             row: self.row,
-            idx: 0..self.row.inner.column_count(),
+            idx: 0..(self.row.inner.column_count() as usize),
             value: None,
         })
     }

diff --git a/libsql/src/hrana/mod.rs b/libsql/src/hrana/mod.rs
@@ -24,7 +24,7 @@ use std::pin::Pin;
 use std::sync::Arc;
 use std::task::{Context, Poll};
 
-use super::rows::{RowInner, RowsInner};
+use super::rows::{ColumnsInner, RowInner, RowsInner};
 
 pub(crate) type Result<T> = std::result::Result<T, HranaError>;
 
@@ -261,7 +261,12 @@ where
     async fn next(&mut self) -> crate::Result<Option<super::Row>> {
         self.next().await
     }
+}
 
+impl<S> ColumnsInner for HranaRows<S>
+where
+    S: Stream<Item = std::io::Result<Bytes>> + Send + Sync + Unpin,
+{
     fn column_count(&self) -> i32 {
         self.column_count()
     }
@@ -303,13 +308,6 @@ impl RowInner for Row {
         Ok(into_value2(v))
     }
 
-    fn column_name(&self, idx: i32) -> Option<&str> {
-        self.cols
-            .get(idx as usize)
-            .and_then(|c| c.name.as_ref())
-            .map(|s| s.as_str())
-    }
-
     fn column_str(&self, idx: i32) -> crate::Result<&str> {
         if let Some(value) = self.inner.get(idx as usize) {
             if let proto::Value::Text { value } = value {
@@ -321,6 +319,15 @@ impl RowInner for Row {
             Err(crate::Error::ColumnNotFound(idx))
         }
     }
+}
+
+impl ColumnsInner for Row {
+    fn column_name(&self, idx: i32) -> Option<&str> {
+        self.cols
+            .get(idx as usize)
+            .and_then(|c| c.name.as_ref())
+            .map(|s| s.as_str())
+    }
 
     fn column_type(&self, idx: i32) -> crate::Result<ValueType> {
         if let Some(value) = self.inner.get(idx as usize) {
@@ -337,8 +344,8 @@ impl RowInner for Row {
         }
     }
 
-    fn column_count(&self) -> usize {
-        self.cols.len()
+    fn column_count(&self) -> i32 {
+        self.cols.len() as i32
     }
 }
 
@@ -417,7 +424,9 @@ impl RowsInner for StmtResultRows {
             inner: Box::new(row),
         }))
     }
+}
 
+impl ColumnsInner for StmtResultRows {
     fn column_count(&self) -> i32 {
         self.cols.len() as i32
     }

diff --git a/libsql/src/local/impls.rs b/libsql/src/local/impls.rs
@@ -5,7 +5,7 @@ use crate::connection::BatchRows;
 use crate::{
     connection::Conn,
     params::Params,
-    rows::{RowInner, RowsInner},
+    rows::{ColumnsInner, RowInner, RowsInner},
     statement::Stmt,
     transaction::Tx,
     Column, Connection, Result, Row, Rows, Statement, Transaction, TransactionBehavior, Value,
@@ -159,7 +159,9 @@ impl RowsInner for LibsqlRows {
 
         Ok(row)
     }
+}
 
+impl ColumnsInner for LibsqlRows {
     fn column_count(&self) -> i32 {
         self.0.column_count()
     }
@@ -180,20 +182,22 @@ impl RowInner for LibsqlRow {
         self.0.get_value(idx)
     }
 
-    fn column_name(&self, idx: i32) -> Option<&str> {
-        self.0.column_name(idx)
-    }
-
     fn column_str(&self, idx: i32) -> Result<&str> {
         self.0.get::<&str>(idx)
     }
+}
+
+impl ColumnsInner for LibsqlRow {
+    fn column_name(&self, idx: i32) -> Option<&str> {
+        self.0.column_name(idx)
+    }
 
     fn column_type(&self, idx: i32) -> Result<ValueType> {
         self.0.column_type(idx).map(ValueType::from)
     }
 
-    fn column_count(&self) -> usize {
-        self.0.stmt.column_count()
+    fn column_count(&self) -> i32 {
+        self.0.stmt.column_count() as i32
     }
 }
 

diff --git a/libsql/src/local/rows.rs b/libsql/src/local/rows.rs
@@ -1,6 +1,6 @@
 use crate::local::{Connection, Statement};
 use crate::params::Params;
-use crate::rows::{RowInner, RowsInner};
+use crate::rows::{ColumnsInner, RowInner, RowsInner};
 use crate::{errors, Error, Result};
 use crate::{Value, ValueRef};
 use libsql_sys::ValueType;
@@ -213,7 +213,9 @@ impl RowsInner for BatchedRows {
             Ok(None)
         }
     }
+}
 
+impl ColumnsInner for BatchedRows {
     fn column_count(&self) -> i32 {
         self.cols.len() as i32
     }
@@ -244,10 +246,6 @@ impl RowInner for BatchedRow {
             .ok_or(Error::InvalidColumnIndex)
     }
 
-    fn column_name(&self, idx: i32) -> Option<&str> {
-        self.cols.get(idx as usize).map(|c| c.0.as_str())
-    }
-
     fn column_str(&self, idx: i32) -> Result<&str> {
         self.row
             .get(idx as usize)
@@ -258,9 +256,15 @@ impl RowInner for BatchedRow {
                     .ok_or(Error::InvalidColumnType)
             })
     }
+}
+
+impl ColumnsInner for BatchedRow {
+    fn column_name(&self, idx: i32) -> Option<&str> {
+        self.cols.get(idx as usize).map(|c| c.0.as_str())
+    }
 
-    fn column_count(&self) -> usize {
-        self.cols.len()
+    fn column_count(&self) -> i32 {
+        self.cols.len() as i32
     }
 
     fn column_type(&self, idx: i32) -> Result<crate::value::ValueType> {

diff --git a/libsql/src/local/statement.rs b/libsql/src/local/statement.rs
@@ -250,15 +250,15 @@ impl Statement {
     /// sure that current statement has already been stepped once before
     /// calling this method.
     pub fn column_names(&self) -> Vec<&str> {
-        let n = self.column_count();
-        let mut cols = Vec::with_capacity(n);
-        for i in 0..n {
-            let s = self.column_name(i);
-            if let Some(s) = s {
-                cols.push(s);
-            }
-        }
-        cols
+       let n = self.column_count();
+       let mut cols = Vec::with_capacity(n);
+       for i in 0..n {
+           let s = self.column_name(i);
+           if let Some(s) = s {
+               cols.push(s);
+           }
+       }
+       cols
     }
 
     /// Return the number of columns in the result set returned by the prepared
@@ -314,12 +314,11 @@ impl Statement {
     /// the specified `name`.
     pub fn column_index(&self, name: &str) -> Result<usize> {
         let bytes = name.as_bytes();
-        let n = self.column_count() as i32;
+        let n = self.column_count();
         for i in 0..n {
             // Note: `column_name` is only fallible if `i` is out of bounds,
             // which we've already checked.
             let col_name = self
-                .inner
                 .column_name(i)
                 .ok_or_else(|| Error::InvalidColumnName(name.to_string()))?;
             if bytes.eq_ignore_ascii_case(col_name.as_bytes()) {

diff --git a/libsql/src/replication/connection.rs b/libsql/src/replication/connection.rs
@@ -11,7 +11,7 @@ use parking_lot::Mutex;
 
 use crate::parser;
 use crate::parser::StmtKind;
-use crate::rows::{RowInner, RowsInner};
+use crate::rows::{ColumnsInner, RowInner, RowsInner};
 use crate::statement::Stmt;
 use crate::transaction::Tx;
 use crate::{
@@ -780,7 +780,9 @@ impl RowsInner for RemoteRows {
         let row = RemoteRow(values, self.0.column_descriptions.clone());
         Ok(Some(row).map(Box::new).map(|inner| Row { inner }))
     }
+}
 
+impl ColumnsInner for RemoteRows {
     fn column_count(&self) -> i32 {
         self.0.column_descriptions.len() as i32
     }
@@ -813,10 +815,6 @@ impl RowInner for RemoteRow {
             .ok_or(Error::InvalidColumnIndex)
     }
 
-    fn column_name(&self, idx: i32) -> Option<&str> {
-        self.1.get(idx as usize).map(|s| s.name.as_str())
-    }
-
     fn column_str(&self, idx: i32) -> Result<&str> {
         let value = self.0.get(idx as usize).ok_or(Error::InvalidColumnIndex)?;
 
@@ -825,6 +823,12 @@ impl RowInner for RemoteRow {
             _ => Err(Error::InvalidColumnType),
         }
     }
+}
+
+impl ColumnsInner for RemoteRow {
+    fn column_name(&self, idx: i32) -> Option<&str> {
+        self.1.get(idx as usize).map(|s| s.name.as_str())
+    }
 
     fn column_type(&self, idx: i32) -> Result<ValueType> {
         let col = self.1.get(idx as usize).unwrap();
@@ -835,8 +839,8 @@ impl RowInner for RemoteRow {
             .ok_or(Error::InvalidColumnType)
     }
 
-    fn column_count(&self) -> usize {
-        self.1.len()
+    fn column_count(&self) -> i32 {
+        self.1.len() as i32
     }
 }
 

diff --git a/libsql/src/rows.rs b/libsql/src/rows.rs
@@ -38,14 +38,8 @@ impl Column<'_> {
 }
 
 #[async_trait::async_trait]
-pub(crate) trait RowsInner {
+pub(crate) trait RowsInner: ColumnsInner {
     async fn next(&mut self) -> Result<Option<Row>>;
-
-    fn column_count(&self) -> i32;
-
-    fn column_name(&self, idx: i32) -> Option<&str>;
-
-    fn column_type(&self, idx: i32) -> Result<ValueType>;
 }
 
 /// A set of rows returned from a connection.
@@ -131,7 +125,7 @@ impl Row {
     }
 
     /// Get the count of columns in this set of rows.
-    pub fn column_count(&self) -> usize {
+    pub fn column_count(&self) -> i32 {
         self.inner.column_count()
     }
 
@@ -284,12 +278,15 @@ where
 }
 impl<T> Sealed for Option<T> {}
 
-pub(crate) trait RowInner: fmt::Debug {
-    fn column_value(&self, idx: i32) -> Result<Value>;
-    fn column_str(&self, idx: i32) -> Result<&str>;
+pub(crate) trait ColumnsInner {
     fn column_name(&self, idx: i32) -> Option<&str>;
     fn column_type(&self, idx: i32) -> Result<ValueType>;
-    fn column_count(&self) -> usize;
+    fn column_count(&self) -> i32;
+}
+
+pub(crate) trait RowInner: ColumnsInner + fmt::Debug {
+    fn column_value(&self, idx: i32) -> Result<Value>;
+    fn column_str(&self, idx: i32) -> Result<&str>;
 }
 
 mod sealed {