Implement query function on connection #8
Conversation
This commit adds the ability to cast the query types to the right Rust native type.
monetdb/src/connection.rs
Outdated
|
|
||
| let response_lines = resp.lines(); | ||
|
|
||
| let response_header: Vec<String> = response_lines.clone().skip(3).map(|x| self.sanitize(x)).collect(); |
There was a problem hiding this comment.
If I understand correctly here you ignore the first 3 lines from the response. Why is that? Take a look at this: https://github.com/MonetDB/MonetDB-PHP/tree/master/protocol_doc#52-query-response---
monetdb/src/connection.rs
Outdated
|
|
||
| #[inline] | ||
| fn sanitize(&self, line: &str) -> String { | ||
| line.replace(&['\t', '%', '[', ']', ' '], " ") |
There was a problem hiding this comment.
This does not look correct. What happens for example if the database has the string
'[%]' stored in a column?
| use mapi::errors::MonetDBError; | ||
|
|
||
| #[derive(Debug, PartialEq)] | ||
| pub enum MonetType { |
There was a problem hiding this comment.
Do we need any more types here (e.g. dates)?
There was a problem hiding this comment.
Correct. I just wanted a initial implementation out. We can add more types later.
| impl MonetType { | ||
| pub fn parse(_type: &String, s: &str) -> Result<Self, MonetDBError> { | ||
| match _type.as_str() { | ||
| "double" => Ok(MonetType::Double(s.parse::<f32>().unwrap())), |
There was a problem hiding this comment.
Please do not use unwrap because, as we have discussed in the past, it might crash the program ("abc".parse::<i32>().unwrap).
This is a good explanation about error handling in Rust: https://blog.burntsushi.net/rust-error-handling/
There was a problem hiding this comment.
It gets the type from the header, so something like "abc".parse::<i32>().unwrap() cannot happen. It always has to be one of the options. It'll otherwise return an UnimplementedError
monetdb/src/row.rs
Outdated
| match _type.as_str() { | ||
| "double" => Ok(MonetType::Double(s.parse::<f32>().unwrap())), | ||
| "int" => Ok(MonetType::Int(s.parse::<i32>().unwrap())), | ||
| "string" => Ok(MonetType::MapiString(String::from(s).replace('"', ""))), |
There was a problem hiding this comment.
This does not take into account legitimate uses of the quote character. See: https://github.com/MonetDB/MonetDB-PHP/tree/master/protocol_doc#61-escaping
This commit splits up the implementation for the parsing of the response. This makes it a little bit more testable.
| let input1 = MonetType::parse(&String::from("clob"), "bar").unwrap(); | ||
| let input2 = MonetType::parse(&String::from("clob"), "999.9").unwrap(); | ||
| let input3 = MonetType::parse(&String::from("clob"), "foo bar with a lot of spaces").unwrap(); | ||
| let input4 = MonetType::parse(&String::from("clob"), "'''foo bar with a lot of backticks'''").unwrap(); |
There was a problem hiding this comment.
The single quote character ' is significant in SQL, so we probably need a couple more tests about it.
In SQL the single quote character is itself quoted by repeating it. That is, to represent the string he said, 'what a nice rust driver'. we would write: 'he said, ''what a nice rust driver''.'.
We need to see how mapi returns strings with single quotes and adjust our tests.
|
|
||
| let metadata_header = match response_lines.next() { | ||
| Some(s) => s, | ||
| None => return Err(MonetDBError::UnimplementedError(String::from("Received no metadata"))) |
There was a problem hiding this comment.
This is not an unimplemented error. That implies that we will write some code to handle this in the future. If this ever happens it is probably a faulty server and we cannot do much about it.
|
|
||
| let metadata = QueryResponse::parse_metadata_header(&metadata_header); | ||
|
|
||
| response_lines.next(); |
There was a problem hiding this comment.
Why are these lines ignored?
| } | ||
|
|
||
| #[inline] | ||
| fn sanitize(line: &str) -> String { |
There was a problem hiding this comment.
what is this for? is it used anywhere?
| } | ||
|
|
||
| #[inline] | ||
| fn remove_first_and_last_quote( line: &str) -> String { |
There was a problem hiding this comment.
I have not actually tested this, but it looks to me that it only removes the first quote, not the last. Also this function crashes if it is given an empty string.
Implements the query function on the connection object, so users can get results back. Fixes #3
Checklist: