Review comments for #418.

This commit is contained in:
Richard Newman 2017-04-18 13:50:58 -07:00
parent aa14a71019
commit bffefe7e6b
4 changed files with 26 additions and 15 deletions

View file

@ -61,13 +61,13 @@ impl ValueType {
impl fmt::Display for ValueType { impl fmt::Display for ValueType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", match *self { write!(f, "{}", match *self {
ValueType::Ref => "db.type/ref", ValueType::Ref => ":db.type/ref",
ValueType::Boolean => "db.type/boolean", ValueType::Boolean => ":db.type/boolean",
ValueType::Instant => "db.type/instant", ValueType::Instant => ":db.type/instant",
ValueType::Long => "db.type/long", ValueType::Long => ":db.type/long",
ValueType::Double => "db.type/double", ValueType::Double => ":db.type/double",
ValueType::String => "db.type/string", ValueType::String => ":db.type/string",
ValueType::Keyword => "db.type/keyword", ValueType::Keyword => ":db.type/keyword",
}) })
} }
} }

View file

@ -19,8 +19,7 @@ use mentat_query::{
Variable, Variable,
}; };
pub use errors::{ use errors::{
Error,
ErrorKind, ErrorKind,
Result, Result,
}; };
@ -32,13 +31,17 @@ pub use errors::{
/// When built correctly, `types` is guaranteed to contain the types of `values` -- use /// When built correctly, `types` is guaranteed to contain the types of `values` -- use
/// `QueryInputs::new` or `QueryInputs::with_values` to construct an instance. /// `QueryInputs::new` or `QueryInputs::with_values` to construct an instance.
pub struct QueryInputs { pub struct QueryInputs {
// These should be crate-private.
pub types: BTreeMap<Variable, ValueType>, pub types: BTreeMap<Variable, ValueType>,
pub values: BTreeMap<Variable, TypedValue>, pub values: BTreeMap<Variable, TypedValue>,
} }
impl Default for QueryInputs { impl Default for QueryInputs {
fn default() -> Self { fn default() -> Self {
QueryInputs { types: BTreeMap::default(), values: BTreeMap::default() } QueryInputs {
types: BTreeMap::default(),
values: BTreeMap::default(),
}
} }
} }

View file

@ -8,6 +8,8 @@
// CONDITIONS OF ANY KIND, either express or implied. See the License for the // CONDITIONS OF ANY KIND, either express or implied. See the License for the
// specific language governing permissions and limitations under the License. // specific language governing permissions and limitations under the License.
use std::cmp;
use std::collections::{ use std::collections::{
BTreeMap, BTreeMap,
BTreeSet, BTreeSet,
@ -116,14 +118,20 @@ impl<K: Clone + Ord, V: Clone> Intersection<K> for BTreeMap<K, V> {
/// Remove all keys from the map that are not present in `ks`. /// Remove all keys from the map that are not present in `ks`.
/// This implementation is terrible because there's no mutable iterator for BTreeMap. /// This implementation is terrible because there's no mutable iterator for BTreeMap.
fn keep_intersected_keys(&mut self, ks: &BTreeSet<K>) { fn keep_intersected_keys(&mut self, ks: &BTreeSet<K>) {
let mut to_remove = Vec::with_capacity(self.len() - ks.len()); if self.is_empty() {
{ return;
}
if ks.is_empty() {
self.clear();
}
let expected_remaining = cmp::max(0, self.len() - ks.len());
let mut to_remove = Vec::with_capacity(expected_remaining);
for k in self.keys() { for k in self.keys() {
if !ks.contains(k) { if !ks.contains(k) {
to_remove.push(k.clone()) to_remove.push(k.clone())
} }
} }
}
for k in to_remove.into_iter() { for k in to_remove.into_iter() {
self.remove(&k); self.remove(&k);
} }

View file

@ -43,8 +43,8 @@ error_chain! {
errors { errors {
UnboundVariables(names: BTreeSet<String>) { UnboundVariables(names: BTreeSet<String>) {
description("unbound variables at execution time") description("unbound variables at query execution time")
display("variables {:?} unbound at execution time", names) display("variables {:?} unbound at query execution time", names)
} }
InvalidArgumentName(name: String) { InvalidArgumentName(name: String) {