Skip to content
This repository has been archived by the owner on May 10, 2020. It is now read-only.

Add BigChar support and iterator for columns #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Licenser
Copy link

  • Add BigChar decoder support
  • Add iter() to QueryRow to allow itterating over the columns
  • Export needed structs and enums to iterate over a column set
    without knowing it's types ahead of time

* Add BigChar decoder support
* Add iter() to QueryRow to allow itterating over the columns
* Export needed structs and enums to iterate over a column set
  without knowing it's types ahead of time
Copy link
Owner

@steffengy steffengy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If rustfmt(?) goes havoc, please split formatting into a separate commit so changes are better visible.

Also I haven't spotted a test actually covering the BigChar case.

@@ -252,7 +256,7 @@ impl<I: BoxableIo> StmtResult<I> for ExecFuture<I> {

/// A row in one resultset of a query
#[derive(Debug)]
pub struct QueryRow(TokenRow);
pub struct QueryRow(pub TokenRow);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks BC and also I'd like this not exposed since it's protocol internals.
(e.g. in new TDS versions row might change)

@@ -298,6 +299,11 @@ impl QueryRow {
let col_data = &self.0.columns[idx];
R::from_column_data(col_data).map(Some)
}
pub fn iter(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exposes MetaDataColumn, which is an implementation specific representation.
Same as above applies.
Exposing this should be done using a NewType wrapper instead that exposes methods for data that we'll want and won't change (e.g. name).
Also I think we should use a slice for that instead of an iterator (similar to the postgres/mysql implementation).

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

Successfully merging this pull request may close these issues.

2 participants