Skip to content
This repository has been archived by the owner on Apr 15, 2023. It is now read-only.

Commit

Permalink
Mass clippy fixes now that I know how to run clippy on save
Browse files Browse the repository at this point in the history
  • Loading branch information
chotchki committed Sep 4, 2021
1 parent 2370aaa commit d95965e
Show file tree
Hide file tree
Showing 23 changed files with 85 additions and 116 deletions.
14 changes: 9 additions & 5 deletions benches/feophant_benchmark.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use criterion::BenchmarkId;
use criterion::Criterion;
use criterion::{criterion_group, criterion_main};
use feophantlib::constants::Nullable;
use feophantlib::engine::get_row;
use feophantlib::engine::get_table;
use feophantlib::engine::io::row_formats::RowData;
Expand Down Expand Up @@ -47,10 +46,15 @@ async fn row_manager_mass_insert(row_count: usize) -> Result<(), Box<dyn std::er
.await;

assert_eq!(result_rows.len(), row_count);
for i in 0..row_count {
let sample_row = get_row(i.to_string());
assert_eq!(result_rows[i].user_data, sample_row);
}
result_rows
.iter()
.enumerate()
.take(row_count)
.map(|(i, row)| {
let sample_row = get_row(i.to_string());
assert_eq!(row.user_data, sample_row);
})
.for_each(drop);

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/constants/system_tables/pg_attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn get_columns() -> Vec<Attribute> {
]
}

pub fn get_index(attrs: &Vec<Attribute>) -> Arc<Index> {
pub fn get_index(attrs: &[Attribute]) -> Arc<Index> {
Arc::new(Index {
id: Uuid::from_bytes(hex!("516B20412CF145A2AD9E39A8BDEB30A8")),
name: NAME.to_string() + "_name_index",
Expand Down
2 changes: 1 addition & 1 deletion src/constants/system_tables/pg_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub fn get_columns() -> Vec<Attribute> {
]
}

pub fn get_index(attrs: &Vec<Attribute>) -> Arc<Index> {
pub fn get_index(attrs: &[Attribute]) -> Arc<Index> {
Arc::new(Index {
id: Uuid::from_bytes(hex!("516B20412CF145A2AD9E39A8BDEB30A8")),
name: NAME.to_string() + "_name_index",
Expand Down
2 changes: 1 addition & 1 deletion src/constants/system_tables/pg_constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn get_columns() -> Vec<Attribute> {
]
}

pub fn get_index(attrs: &Vec<Attribute>) -> Arc<Index> {
pub fn get_index(attrs: &[Attribute]) -> Arc<Index> {
Arc::new(Index {
id: Uuid::from_bytes(hex!("27182DE783AB42D8B5DD43BFC0154F0F")),
name: NAME.to_string() + "_name_index",
Expand Down
2 changes: 1 addition & 1 deletion src/constants/system_tables/pg_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn get_columns() -> Vec<Attribute> {
]
}

pub fn get_index(attrs: &Vec<Attribute>) -> Arc<Index> {
pub fn get_index(attrs: &[Attribute]) -> Arc<Index> {
Arc::new(Index {
id: Uuid::from_bytes(hex!("5F59466782874C568F1C0C09E99C9249")),
name: NAME.to_string() + "_name_index",
Expand Down
2 changes: 1 addition & 1 deletion src/engine/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl Analyzer {
//Insert columns will be the target
targets: Arc::new(output_type),
range_tables: vec![target_tbl.clone(), anon_tbl.clone()],
joins: vec![((JoinType::Inner, target_tbl, anon_tbl))],
joins: vec![(JoinType::Inner, target_tbl, anon_tbl)],
})
}

Expand Down
20 changes: 8 additions & 12 deletions src/engine/analyzer/definition_lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl DefinitionLookup {
let row_stream = self
.vis_row_man
.clone()
.get_stream(tran_id, SystemTables::PgClass.value().clone());
.get_stream(tran_id, SystemTables::PgClass.value());
pin!(row_stream);
while let Some(row_res) = row_stream.next().await {
let row = row_res?;
Expand Down Expand Up @@ -175,13 +175,13 @@ impl DefinitionLookup {
&self,
tran_id: TransactionId,
class_id: Uuid,
attributes: &Vec<Attribute>,
attributes: &[Attribute],
) -> Result<Vec<Arc<Index>>, DefinitionLookupError> {
let mut rows = vec![];
let row_stream = self
.vis_row_man
.clone()
.get_stream(tran_id, SystemTables::PgIndex.value().clone());
.get_stream(tran_id, SystemTables::PgIndex.value());
pin!(row_stream);
while let Some(row_res) = row_stream.next().await {
let row = row_res?;
Expand Down Expand Up @@ -214,9 +214,7 @@ impl DefinitionLookup {
cols.push(
attributes
.get(i_usize)
.ok_or_else(|| {
DefinitionLookupError::WrongColumnIndex(i_usize)
})?
.ok_or(DefinitionLookupError::WrongColumnIndex(i_usize))?
.clone(),
);
}
Expand Down Expand Up @@ -249,13 +247,13 @@ impl DefinitionLookup {
&self,
tran_id: TransactionId,
class_id: Uuid,
indexes: &Vec<Arc<Index>>,
indexes: &[Arc<Index>],
) -> Result<Vec<Constraint>, DefinitionLookupError> {
let mut rows = vec![];
let row_stream = self
.vis_row_man
.clone()
.get_stream(tran_id, SystemTables::PgConstraint.value().clone());
.get_stream(tran_id, SystemTables::PgConstraint.value());
pin!(row_stream);
while let Some(row_res) = row_stream.next().await {
let row = row_res?;
Expand Down Expand Up @@ -368,9 +366,8 @@ mod tests {
.get_definition(tran_id, "something_random".to_string())
.await;
match pg_class_def {
Ok(_) => assert!(false),
Err(DefinitionLookupError::TableDoesNotExist(_)) => assert!(true),
_ => assert!(false),
Err(DefinitionLookupError::TableDoesNotExist(_)) => {}
_ => panic!("Should not have gotten here!"),
}
Ok(())
}
Expand All @@ -397,7 +394,6 @@ mod tests {
dl.get_definition(tran, "foo".to_string()).await?;
tm.commit_trans(tran).await?;

assert!(true);
Ok(())
}
}
1 change: 0 additions & 1 deletion src/engine/io/constraint_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::{
};

use super::{
page_formats::PageData,
row_formats::{ItemPointer, RowData},
VisibleRowManager, VisibleRowManagerError,
};
Expand Down
2 changes: 0 additions & 2 deletions src/engine/io/file_manager.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
//! This is a different approach than I had done before. This file manager runs its own loop based on a spawned task
//! since the prior approach was too lock heavy and I couldn't figure out an approach that didn't starve resources.
use super::page_formats::{PageId, PageOffset, UInt12, UInt12Error};
use async_stream::try_stream;
use bytes::{Bytes, BytesMut};
use futures::Stream;
use std::convert::TryFrom;
use std::ffi::OsString;
use std::num::TryFromIntError;
Expand Down
9 changes: 4 additions & 5 deletions src/engine/io/file_manager/file_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::constants::PAGE_SIZE;
use crate::engine::io::file_manager::ResourceFormatter;
use crate::engine::io::page_formats::{PageId, PageOffset};
use bytes::{Bytes, BytesMut};
use futures::SinkExt;
use lru::LruCache;
use std::collections::{HashMap, VecDeque};
use std::convert::TryFrom;
Expand Down Expand Up @@ -628,9 +627,9 @@ mod tests {
let mut test_file =
FileOperations::open_path(tmp_dir, &page_id, PageOffset(0).get_file_number()).await?;

let mut test_page = get_test_page(1);
let test_page = get_test_page(1);

test_file.write_all(&mut test_page).await?;
test_file.write_all(&test_page).await?;
drop(test_file);

//Now let's test add
Expand Down Expand Up @@ -672,9 +671,9 @@ mod tests {
)
.await?;

let mut test_page = get_test_page(1);
let test_page = get_test_page(1);

test_file.write_all(&mut test_page).await?;
test_file.write_all(&test_page).await?;
drop(test_file);

//Now let's test add
Expand Down
49 changes: 20 additions & 29 deletions src/engine/io/free_space_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,22 +116,6 @@ impl FreeSpaceManager {
None
}

/// Gets the status of a field inside a page, you MUST pass an offset
/// that fits in the buffer.
//TODO Decide if I end up keeping this or maybe move it to the unit test section?
fn get_status_inside_page(buffer: &BytesMut, offset: usize) -> FreeStat {
let offset_index = offset / 8;
let offset_subindex = offset % 8;

let offset_value = buffer[offset_index];
let bit_value = (offset_value >> offset_subindex) & 0x1;
if bit_value == 0 {
FreeStat::Free
} else {
FreeStat::InUse
}
}

/// Sets the status of a field inside a page, you MUST pass an offset
/// that fits in the buffer.
fn set_status_inside_page(buffer: &mut BytesMut, offset: usize, status: FreeStat) {
Expand Down Expand Up @@ -181,27 +165,34 @@ mod tests {

use super::*;

/// Gets the status of a field inside a page, you MUST pass an offset
/// that fits in the buffer.
//This was in the implementation, I just only needed it for unit tests
fn get_status_inside_page(buffer: &BytesMut, offset: usize) -> FreeStat {
let offset_index = offset / 8;
let offset_subindex = offset % 8;

let offset_value = buffer[offset_index];
let bit_value = (offset_value >> offset_subindex) & 0x1;
if bit_value == 0 {
FreeStat::Free
} else {
FreeStat::InUse
}
}

///This test works by toggling each bit repeatedly and making sure it gives the correct result each time.
#[test]
fn test_get_and_set() -> Result<(), Box<dyn std::error::Error>> {
let mut test = BytesMut::with_capacity(2);
test.put_u16(0x0);

for i in 0..test.capacity() * 8 {
assert_eq!(
FreeSpaceManager::get_status_inside_page(&test, i),
FreeStat::Free
);
assert_eq!(get_status_inside_page(&test, i), FreeStat::Free);
FreeSpaceManager::set_status_inside_page(&mut test, i, FreeStat::InUse);
assert_eq!(
FreeSpaceManager::get_status_inside_page(&test, i),
FreeStat::InUse
);
assert_eq!(get_status_inside_page(&test, i), FreeStat::InUse);
FreeSpaceManager::set_status_inside_page(&mut test, i, FreeStat::Free);
assert_eq!(
FreeSpaceManager::get_status_inside_page(&test, i),
FreeStat::Free
);
assert_eq!(get_status_inside_page(&test, i), FreeStat::Free);
}

Ok(())
Expand All @@ -220,7 +211,7 @@ mod tests {
FreeSpaceManager::set_status_inside_page(&mut test, i, FreeStat::InUse);
}
assert_eq!(
FreeSpaceManager::find_first_free_page_in_page(&mut test.clone()),
FreeSpaceManager::find_first_free_page_in_page(&mut test),
None
);

Expand Down
11 changes: 4 additions & 7 deletions src/engine/io/lock_cache_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl LockCacheManager {
) -> Result<Arc<RwLock<Option<BytesMut>>>, LockCacheManagerError> {
let mut cache = self.cache.lock().await;
match cache.get(&(page_id, offset)) {
Some(s) => return Ok(s.clone()),
Some(s) => Ok(s.clone()),
None => {
//Cache miss, let's make the RwLock and drop the mutex
let page_lock = Arc::new(RwLock::new(None));
Expand All @@ -70,12 +70,9 @@ impl LockCacheManager {
drop(cache);

//Now we can load the underlying page without blocking everyone
match self.file_manager.get_page(&page_id, &offset).await? {
Some(s) => {
page_lock_write.replace(s);
}
None => {}
};
if let Some(s) = self.file_manager.get_page(&page_id, &offset).await? {
page_lock_write.replace(s);
}
drop(page_lock_write);

Ok(page_lock)
Expand Down
7 changes: 2 additions & 5 deletions src/engine/io/page_formats/page_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,11 @@ pub enum PageDataError {

#[cfg(test)]
mod tests {
use crate::constants::{Nullable, PAGE_SIZE};
use crate::constants::PAGE_SIZE;
use crate::engine::get_table;
use crate::engine::objects::SqlTuple;

use super::super::super::super::objects::{
types::{BaseSqlTypes, BaseSqlTypesMapper},
Attribute, Table,
};
use super::super::super::super::objects::types::BaseSqlTypes;
use super::super::super::super::transactions::TransactionId;
use super::*;
use bytes::BytesMut;
Expand Down
6 changes: 6 additions & 0 deletions src/engine/io/page_formats/page_header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ impl PageHeader {
}
}

impl Default for PageHeader {
fn default() -> Self {
Self::new()
}
}

impl ConstEncodedSize for PageHeader {
fn encoded_size() -> usize {
3
Expand Down
24 changes: 6 additions & 18 deletions src/engine/io/page_formats/page_offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,27 +203,15 @@ mod tests {

#[test]
fn test_is_same_file() -> Result<(), Box<dyn std::error::Error>> {
assert_eq!(
PageOffset(0).is_same_file(&PageOffset(PAGES_PER_FILE)),
false
);
assert_eq!(PageOffset(0).is_same_file(&PageOffset(0)), true);
assert_eq!(
PageOffset(0).is_same_file(&PageOffset(PAGES_PER_FILE - 1)),
true
);

assert_eq!(
PageOffset(PAGES_PER_FILE).is_same_file(&PageOffset(0)),
false
);
assert_eq!(
PageOffset(PAGES_PER_FILE - 1).is_same_file(&PageOffset(0)),
true
);
assert!(!PageOffset(0).is_same_file(&PageOffset(PAGES_PER_FILE)));
assert!(PageOffset(0).is_same_file(&PageOffset(0)));
assert!(PageOffset(0).is_same_file(&PageOffset(PAGES_PER_FILE - 1)));
assert!(!PageOffset(PAGES_PER_FILE).is_same_file(&PageOffset(0)));
assert!(PageOffset(PAGES_PER_FILE - 1).is_same_file(&PageOffset(0)));

Ok(())
}

#[test]
fn test_increment_and_hash_map() -> Result<(), Box<dyn std::error::Error>> {
let test = PageOffset(0);
Expand Down
2 changes: 1 addition & 1 deletion src/engine/io/row_formats/row_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ mod tests {
let mut buffer = buffer.freeze();

let test_parse = RowData::parse(table, &mut buffer)?;
assert_eq!(test, test_parse.clone());
assert_eq!(test, test_parse);

let column_val = test_parse.get_column_not_null("header")?;
assert_eq!(column_val, BaseSqlTypes::Text("this is a test".to_string()));
Expand Down
Loading

0 comments on commit d95965e

Please sign in to comment.