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.

Sign up for GitHub

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

replace unwrap() with expect("unique message") #681

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

replace unwrap() with expect("unique message") #681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdee0b6
replace unwrap() with expect("unique message")
kvc0 Jan 13, 2023
4a84eef
cargo fmt
kvc0 Jan 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions crates/t-digest/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ impl TDigest {
max: self.max(),
centroids: centroids.to_vec(),
})
.unwrap()
.expect("could not create a ron string")
}
}

Expand Down Expand Up @@ -293,8 +293,8 @@ impl TDigest {
let mut result = TDigest::new_with_size(self.max_size());
result.count = self.count() + (sorted_values.len() as u64);

let maybe_min = OrderedFloat::from(*sorted_values.first().unwrap());
let maybe_max = OrderedFloat::from(*sorted_values.last().unwrap());
let maybe_min = OrderedFloat::from(*sorted_values.first().expect("invalid sorted pair - missing first"));
let maybe_max = OrderedFloat::from(*sorted_values.last().expect("invalid sorted pair - missing second"));

if self.count() > 0 {
result.min = std::cmp::min(self.min, maybe_min);
Expand All @@ -315,14 +315,14 @@ impl TDigest {
let mut iter_sorted_values = sorted_values.iter().peekable();

let mut curr: Centroid = if let Some(c) = iter_centroids.peek() {
let curr = **iter_sorted_values.peek().unwrap();
let curr = **iter_sorted_values.peek().expect("sorted value list must not be empty");
if c.mean() < curr {
iter_centroids.next().unwrap().clone()
iter_centroids.next().expect("next centroid to clone must exist").clone()
} else {
Centroid::new(*iter_sorted_values.next().unwrap(), 1)
Centroid::new(*iter_sorted_values.next().expect("next sorted value to use as centroid mean must exist"), 1)
}
} else {
Centroid::new(*iter_sorted_values.next().unwrap(), 1)
Centroid::new(*iter_sorted_values.next().expect("next value after an empty value must exist (oh?)"), 1)
};

let mut weight_so_far: u64 = curr.weight();
Expand All @@ -333,14 +333,14 @@ impl TDigest {
while iter_centroids.peek().is_some() || iter_sorted_values.peek().is_some() {
let next: Centroid = if let Some(c) = iter_centroids.peek() {
if iter_sorted_values.peek().is_none()
|| c.mean() < **iter_sorted_values.peek().unwrap()
|| c.mean() < **iter_sorted_values.peek().expect("this was already checked in this condition")
{
iter_centroids.next().unwrap().clone()
iter_centroids.next().expect("the next centroid after an empty sorted value must exist (but the loop says ||)").clone()
} else {
Centroid::new(*iter_sorted_values.next().unwrap(), 1)
Centroid::new(*iter_sorted_values.next().expect("this was logically guarded in the preceding condition"), 1)
}
} else {
Centroid::new(*iter_sorted_values.next().unwrap(), 1)
Centroid::new(*iter_sorted_values.next().expect("the next sorted value after a missing centroid must exist (but the loop says ||)"), 1)
};

let next_sum: f64 = next.mean() * next.weight() as f64;
Expand Down Expand Up @@ -424,7 +424,8 @@ impl TDigest {
}

// TODO should this be the smaller of the sizes?
let max_size = digests.first().unwrap().max_size;
// TODO: should this be configurable? Should it make sure it's at least some reasonable size?
let max_size = digests.first().expect("there was no digest to merge").max_size;
let mut centroids: Vec<Centroid> = Vec::with_capacity(n_centroids);
let mut starts: Vec<usize> = Vec::with_capacity(digests.len());

Expand Down Expand Up @@ -475,7 +476,7 @@ impl TDigest {
let mut q_limit_times_count: f64 = Self::k_to_q(k_limit, max_size as f64) * (count as f64);

let mut iter_centroids = centroids.iter_mut();
let mut curr = iter_centroids.next().unwrap();
let mut curr = iter_centroids.next().expect("we made a centroid list with contents from the digests list");
let mut weight_so_far: u64 = curr.weight();
let mut sums_to_merge: f64 = 0.0;
let mut weights_to_merge: u64 = 0;
Expand Down Expand Up @@ -1048,7 +1049,7 @@ mod tests {
.chain(batch4.iter())
.copied()
.collect();
master.sort_by(|a, b| a.partial_cmp(b).unwrap());
master.sort_by(|a, b| a.partial_cmp(b).expect("a comparison must exist"));

if master.len() < 100 {
return TestResult::discard();
Expand Down
27 changes: 21 additions & 6 deletions extension/src/tdigest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ pub fn tdigest_trans_inner(
}
};
let mut state = match state {
None => tdigest::Builder::with_size(size.try_into().unwrap()).into(),
None => tdigest::Builder::with_size(
size.try_into().expect("unsigned size must fit in usize"),
)
.into(),
Some(state) => state,
};
state.push(value);
Expand Down Expand Up @@ -88,7 +91,11 @@ use crate::raw::bytea;

#[pg_extern(immutable, parallel_safe, strict)]
pub fn tdigest_serialize(state: Internal) -> bytea {
let state: &mut tdigest::Builder = unsafe { state.get_mut().unwrap() };
let state: &mut tdigest::Builder = unsafe {
state
.get_mut()
.expect("state must exist to serialize tdigest")
};
// TODO this macro is really broken
let hack = state.build();
let hackref = &hack;
Expand Down Expand Up @@ -123,7 +130,7 @@ impl<'input> InOutFuncs for TDigest<'input> {
fn output(&self, buffer: &mut StringInfo) {
use crate::serialization::{str_to_db_encoding, EncodedStr::*};

let stringified = ron::to_string(&**self).unwrap();
let stringified = ron::to_string(&**self).expect("must be able to ronstring self");
match str_to_db_encoding(&stringified) {
Utf8(s) => buffer.push_str(s),
Other(s) => buffer.push_bytes(s.to_bytes()),
Expand All @@ -137,7 +144,8 @@ impl<'input> InOutFuncs for TDigest<'input> {
use crate::serialization::str_from_db_encoding;

let input = str_from_db_encoding(input);
let mut val: TDigestData = ron::from_str(input).unwrap();
let mut val: TDigestData =
ron::from_str(input).expect("must be able to read self as ronstring");
val.buckets = val
.centroids
.len()
Expand All @@ -160,7 +168,10 @@ impl<'input> TDigest<'input> {
}

fn from_internal_tdigest(digest: &InternalTDigest) -> TDigest<'static> {
let max_buckets: u32 = digest.max_size().try_into().unwrap();
let max_buckets: u32 = digest
.max_size()
.try_into()
.expect("usize must fit into u32 for max_buckets");

let centroids = digest.raw_centroids();

Expand Down Expand Up @@ -295,7 +306,11 @@ fn tdigest_compound_final(

#[pg_extern(immutable, parallel_safe)]
fn tdigest_compound_serialize(state: Internal, _fcinfo: pg_sys::FunctionCallInfo) -> bytea {
let state: Inner<InternalTDigest> = unsafe { state.to_inner().unwrap() };
let state: Inner<InternalTDigest> = unsafe {
state
.to_inner()
.expect("an internal tdigest must exist for serialization")
};
crate::do_serialize!(state)
}

Expand Down