Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic interaction with query results #278

Open
bittrance opened this issue Mar 14, 2023 · 1 comment
Open

Dynamic interaction with query results #278

bittrance opened this issue Mar 14, 2023 · 1 comment

Comments

@bittrance
Copy link
Contributor

bittrance commented Mar 14, 2023

It is harder than it needs to be to handle query results where you do not know or do not care about the data types of returned values. I first met this difficulty when trying to query arbitrary changetables where the primary key column(s) are included in the output. However, having encountered the difficulty, it seems to me that any code that does not know the exact query asked, will require needlessly complex code.

There are two main issues:

  • receiving and storing an arbitrary value (i.e. holding the ColumnData<'_> rather than forcing it into another type through FromSql)
  • iterating over columns and values in parallel

Working with arbitrary values

Consider:

 let mut stream = client.simple_query(q).await?.into_row_stream();
 fn f<'a>(_col: &Column, _val: impl FromSql<'a>) { /* ... */ };
 while let Some(row) = stream.try_next().await? {
     for (n, col) in row.columns().iter().enumerate() {
        let val = row.get(n).unwrap();
        f(col, val);
    }
}

This results in

error[E0282]: type annotations needed
   --> src/db.rs:117:21
    |
117 |                 let val = row.get(n).unwrap();
    |                     ^^^
    |

To my knowledge, there is no agnostic way to express the type of val so as to delegate the conversion?

Iterating over columns and values

Row implements IntoIterator so at first glance, it should be possible to do stuff. However, the borrow checker will not appreciate us looking at row.columns() after we row.into_iter(), so we would have to clone the columns first.

Proposal

I think many dynamic cases could be addressed by allowing iterating over the "cells" (i.e. key/value pairs) in a row. Consider the following diff:

diff --git a/src/row.rs b/src/row.rs
index 34c591d..3a0a450 100644
--- a/src/row.rs
+++ b/src/row.rs
@@ -291,6 +291,11 @@ impl Row {
         &self.columns
     }

+    /// Return an iterator over row values.
+    pub fn cells(&self) -> impl Iterator<Item = (&Column, &ColumnData<'static>)> {
+        self.columns().iter().zip(self.data.iter())
+    }
+
     /// The result set number, starting from zero and increasing if the stream
     /// has results from more than one query.
     pub fn result_index(&self) -> usize {
diff --git a/src/tds/codec/token/token_row.rs b/src/tds/codec/token/token_row.rs
index d724e81..b1ff16b 100644
--- a/src/tds/codec/token/token_row.rs
+++ b/src/tds/codec/token/token_row.rs
@@ -71,6 +71,11 @@ impl<'a> TokenRow<'a> {
         self.data.len()
     }

+    /// Returns an iterator over column values.
+    pub fn iter(&self) -> std::slice::Iter<'_, ColumnData<'a>> {
+        self.data.iter()
+    }
+
     /// True if row has no columns.
     pub fn is_empty(&self) -> bool {
         self.data.is_empty()

With this, you can easily construct (in my case):

let mut stream = client.simple_query(q).await?.into_row_stream();
while let Some(row) = stream.try_next().await? {
    let entries = row
        .cells()
        .filter(|(c, _)| !c.name().starts_with("SYS_"));
    handler(&table, entries).await?;
}

This also gives access directly to the ColumnData object and so allows us to postpone converting it into a "native" data type.

I'm not in the least wed to this exact signature. You may argue the implementation should return Cell objects, for example.

If you think something like this sounds useful, I'll be happy to provide a PR once we've figured out the exact signature.

@bittrance
Copy link
Contributor Author

Assuming we expose Column in a signature, we need to expose a public constructor for it so that you can write unit tests for your own code. It may be that it should only be exposed under a feature testing or something similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant