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.

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

chore: deny unused results #1825

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ rustflags = [
"-Wclippy::print_stderr",
"-Wclippy::implicit_clone",
"-Aclippy::items_after_test_module",
"-Wunused_results",
]
31 changes: 15 additions & 16 deletions benchmarks/src/bin/nyc-taxi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ async fn write_data(
};

let now = Instant::now();
db.insert(requests).await.unwrap();
let _ = db.insert(requests).await.unwrap();
let elapsed = now.elapsed();
total_rpc_elapsed_ms += elapsed.as_millis();
progress_bar.inc(row_count as _);
Expand Down Expand Up @@ -377,19 +377,16 @@ fn create_table_expr() -> CreateTableExpr {
}

fn query_set() -> HashMap<String, String> {
let mut ret = HashMap::new();

ret.insert(
"count_all".to_string(),
format!("SELECT COUNT(*) FROM {TABLE_NAME};"),
);

ret.insert(
"fare_amt_by_passenger".to_string(),
format!("SELECT passenger_count, MIN(fare_amount), MAX(fare_amount), SUM(fare_amount) FROM {TABLE_NAME} GROUP BY passenger_count")
);

ret
HashMap::from([
(
"count_all".to_string(),
format!("SELECT COUNT(*) FROM {TABLE_NAME};"),
),
(
"fare_amt_by_passenger".to_string(),
format!("SELECT passenger_count, MIN(fare_amount), MAX(fare_amount), SUM(fare_amount) FROM {TABLE_NAME} GROUP BY passenger_count"),
)
])
}

async fn do_write(args: &Args, db: &Database) {
Expand All @@ -414,7 +411,8 @@ async fn do_write(args: &Args, db: &Database) {
let db = db.clone();
let mpb = multi_progress_bar.clone();
let pb_style = progress_bar_style.clone();
write_jobs.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
let _ = write_jobs
.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
}
}
while write_jobs.join_next().await.is_some() {
Expand All @@ -423,7 +421,8 @@ async fn do_write(args: &Args, db: &Database) {
let db = db.clone();
let mpb = multi_progress_bar.clone();
let pb_style = progress_bar_style.clone();
write_jobs.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
let _ = write_jobs
.spawn(async move { write_data(batch_size, &db, path, mpb, pb_style).await });
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/catalog/src/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,6 @@ mod tests {
#[test]
fn test_table_global_value_compatibility() {
let s = r#"{"node_id":1,"regions_id_map":{"1":[0]},"table_info":{"ident":{"table_id":1098,"version":1},"name":"container_cpu_limit","desc":"Created on insertion","catalog_name":"greptime","schema_name":"dd","meta":{"schema":{"column_schemas":[{"name":"container_id","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"container_name","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"docker_image","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"host","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"image_name","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"image_tag","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"interval","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"runtime","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"short_image","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"type","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"dd_value","data_type":{"Float64":{}},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"ts","data_type":{"Timestamp":{"Millisecond":null}},"is_nullable":false,"is_time_index":true,"default_constraint":null,"metadata":{"greptime:time_index":"true"}},{"name":"git.repository_url","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}}],"timestamp_index":11,"version":1},"primary_key_indices":[0,1,2,3,4,5,6,7,8,9,12],"value_indices":[10,11],"engine":"mito","next_column_id":12,"region_numbers":[],"engine_options":{},"options":{},"created_on":"1970-01-01T00:00:00Z"},"table_type":"Base"}}"#;
TableGlobalValue::parse(s).unwrap();
assert!(TableGlobalValue::parse(s).is_ok());
killme2008 marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion src/catalog/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub(crate) async fn handle_system_table_request<'a, M: CatalogManager>(
table_name,
),
})?;
manager
let _ = manager
.register_table(RegisterTableRequest {
catalog: catalog_name.clone(),
schema: schema_name.clone(),
Expand Down
33 changes: 20 additions & 13 deletions src/catalog/src/local/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ impl LocalCatalogManager {

async fn init_system_catalog(&self) -> Result<()> {
// register SystemCatalogTable
self.catalogs
let _ = self
.catalogs
.register_catalog_sync(SYSTEM_CATALOG_NAME.to_string())?;
self.catalogs.register_schema_sync(RegisterSchemaRequest {
let _ = self.catalogs.register_schema_sync(RegisterSchemaRequest {
catalog: SYSTEM_CATALOG_NAME.to_string(),
schema: INFORMATION_SCHEMA_NAME.to_string(),
})?;
Expand All @@ -131,12 +132,13 @@ impl LocalCatalogManager {
table_id: SYSTEM_CATALOG_TABLE_ID,
table: self.system.information_schema.system.clone(),
};
self.catalogs.register_table(register_table_req).await?;
let _ = self.catalogs.register_table(register_table_req).await?;

// register default catalog and default schema
self.catalogs
let _ = self
.catalogs
.register_catalog_sync(DEFAULT_CATALOG_NAME.to_string())?;
self.catalogs.register_schema_sync(RegisterSchemaRequest {
let _ = self.catalogs.register_schema_sync(RegisterSchemaRequest {
catalog: DEFAULT_CATALOG_NAME.to_string(),
schema: DEFAULT_SCHEMA_NAME.to_string(),
})?;
Expand All @@ -151,7 +153,8 @@ impl LocalCatalogManager {
table: numbers_table,
};

self.catalogs
let _ = self
.catalogs
.register_table(register_number_table_req)
.await?;

Expand Down Expand Up @@ -226,7 +229,8 @@ impl LocalCatalogManager {
for entry in entries {
match entry {
Entry::Catalog(c) => {
self.catalogs
let _ = self
.catalogs
.register_catalog_if_absent(c.catalog_name.clone());
info!("Register catalog: {}", c.catalog_name);
}
Expand All @@ -235,7 +239,7 @@ impl LocalCatalogManager {
catalog: s.catalog_name.clone(),
schema: s.schema_name.clone(),
};
self.catalogs.register_schema_sync(req)?;
let _ = self.catalogs.register_schema_sync(req)?;
info!("Registered schema: {:?}", s);
}
Entry::Table(t) => {
Expand Down Expand Up @@ -297,7 +301,7 @@ impl LocalCatalogManager {
table_id: t.table_id,
table: table_ref,
};
self.catalogs.register_table(register_request).await?;
let _ = self.catalogs.register_table(register_request).await?;

Ok(())
}
Expand Down Expand Up @@ -389,8 +393,9 @@ impl CatalogManager for LocalCatalogManager {
let engine = request.table.table_info().meta.engine.to_string();
let table_name = request.table_name.clone();
let table_id = request.table_id;
self.catalogs.register_table(request).await?;
self.system
let _ = self.catalogs.register_table(request).await?;
let _ = self
.system
.register_table(
catalog_name.clone(),
schema_name.clone(),
Expand Down Expand Up @@ -438,7 +443,8 @@ impl CatalogManager for LocalCatalogManager {

let engine = old_table.table_info().meta.engine.to_string();
// rename table in system catalog
self.system
let _ = self
.system
.register_table(
catalog_name.clone(),
schema_name.clone(),
Expand Down Expand Up @@ -499,7 +505,8 @@ impl CatalogManager for LocalCatalogManager {
schema: schema_name,
}
);
self.system
let _ = self
.system
.register_schema(request.catalog.clone(), schema_name.clone())
.await?;
self.catalogs.register_schema_sync(request)
Expand Down
35 changes: 19 additions & 16 deletions src/catalog/src/local/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,8 @@ impl Default for MemoryCatalogManager {
catalogs: Default::default(),
};

let mut catalog = HashMap::with_capacity(1);
catalog.insert(DEFAULT_SCHEMA_NAME.to_string(), HashMap::new());
manager
let catalog = HashMap::from([(DEFAULT_SCHEMA_NAME.to_string(), HashMap::new())]);
let _ = manager
.catalogs
.write()
.unwrap()
Expand Down Expand Up @@ -115,7 +114,7 @@ impl CatalogManager for MemoryCatalogManager {
}

let table = schema.remove(&request.table_name).unwrap();
schema.insert(request.new_table_name, table);
let _ = schema.insert(request.new_table_name, table);

Ok(true)
}
Expand Down Expand Up @@ -144,9 +143,11 @@ impl CatalogManager for MemoryCatalogManager {
}

async fn register_schema(&self, request: RegisterSchemaRequest) -> Result<bool> {
self.register_schema_sync(request)?;
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0);
Ok(true)
let registered = self.register_schema_sync(request)?;
if registered {
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0);
}
Ok(registered)
}

async fn register_system_table(&self, _request: RegisterSystemTableRequest) -> Result<()> {
Expand Down Expand Up @@ -234,9 +235,11 @@ impl CatalogManager for MemoryCatalogManager {
}

async fn register_catalog(&self, name: String) -> Result<bool> {
self.register_catalog_sync(name)?;
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0);
Ok(true)
let registered = self.register_catalog_sync(name)?;
if registered {
increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0);
}
Ok(registered)
}

fn as_any(&self) -> &dyn Any {
Expand All @@ -252,7 +255,7 @@ impl MemoryCatalogManager {
match entry {
Entry::Occupied(_) => true,
Entry::Vacant(v) => {
v.insert(HashMap::new());
let _ = v.insert(HashMap::new());
false
}
}
Expand All @@ -273,7 +276,7 @@ impl MemoryCatalogManager {
if catalog.contains_key(&request.schema) {
return Ok(false);
}
catalog.insert(request.schema, HashMap::new());
let _ = catalog.insert(request.schema, HashMap::new());
Ok(true)
}

Expand Down Expand Up @@ -310,7 +313,7 @@ impl MemoryCatalogManager {
table_id: table.table_info().ident.table_id,
table,
};
manager.register_table_sync(request).unwrap();
let _ = manager.register_table_sync(request).unwrap();
manager
}
}
Expand Down Expand Up @@ -341,7 +344,7 @@ mod tests {
table: Arc::new(NumbersTable::default()),
};

catalog_list.register_table(register_request).await.unwrap();
assert!(catalog_list.register_table(register_request).await.is_ok());
let table = catalog_list
.table(
DEFAULT_CATALOG_NAME,
Expand Down Expand Up @@ -390,7 +393,7 @@ mod tests {
new_table_name: new_table_name.to_string(),
table_id,
};
catalog.rename_table(rename_request).await.unwrap();
assert!(catalog.rename_table(rename_request).await.is_ok());

// test old table name not exist
assert!(!catalog
Expand Down Expand Up @@ -492,7 +495,7 @@ mod tests {
table_id: 2333,
table: Arc::new(NumbersTable::default()),
};
catalog.register_table(register_table_req).await.unwrap();
assert!(catalog.register_table(register_table_req).await.is_ok());
assert!(catalog
.table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name)
.await
Expand Down
2 changes: 1 addition & 1 deletion src/catalog/src/remote/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl KvBackend for MetaKvBackend {

async fn move_value(&self, from_key: &[u8], to_key: &[u8]) -> Result<()> {
let req = MoveValueRequest::new(from_key, to_key);
self.client.move_value(req).await.context(MetaSrvSnafu)?;
let _ = self.client.move_value(req).await.context(MetaSrvSnafu)?;
Ok(())
}

Expand Down
26 changes: 14 additions & 12 deletions src/catalog/src/remote/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl RemoteCatalogManager {
joins.push(self.initiate_schemas(node_id, backend, engine_manager, catalog_name));
}

futures::future::try_join_all(joins).await?;
let _ = futures::future::try_join_all(joins).await?;

Ok(())
}
Expand Down Expand Up @@ -623,13 +623,14 @@ impl CatalogManager for RemoteCatalogManager {
self.check_catalog_schema_exist(&catalog_name, &schema_name)
.await?;

self.register_table(
catalog_name.clone(),
schema_name.clone(),
request.table_name,
request.table.clone(),
)
.await?;
let _ = self
.register_table(
catalog_name.clone(),
schema_name.clone(),
request.table_name,
request.table.clone(),
)
.await?;

let table_info = request.table.table_info();
let table_ident = TableIdent {
Expand Down Expand Up @@ -680,7 +681,8 @@ impl CatalogManager for RemoteCatalogManager {
table_id: table_info.ident.table_id,
engine: table_info.meta.engine.clone(),
};
self.region_alive_keepers
let _ = self
.region_alive_keepers
.deregister_table(&table_ident)
.await;
}
Expand Down Expand Up @@ -846,7 +848,7 @@ impl CatalogManager for RemoteCatalogManager {
let catalog_key = String::from_utf8_lossy(&catalog.0);

if let Ok(key) = CatalogKey::parse(&catalog_key) {
catalogs.insert(key.catalog_name);
let _ = catalogs.insert(key.catalog_name);
}
}
}
Expand All @@ -865,7 +867,7 @@ impl CatalogManager for RemoteCatalogManager {
let schema_key = String::from_utf8_lossy(&schema.0);

if let Ok(key) = SchemaKey::parse(&schema_key) {
schemas.insert(key.schema_name);
let _ = schemas.insert(key.schema_name);
}
}
}
Expand All @@ -886,7 +888,7 @@ impl CatalogManager for RemoteCatalogManager {
let table_key = String::from_utf8_lossy(&table.0);

if let Ok(key) = TableRegionalKey::parse(&table_key) {
tables.insert(key.table_name);
let _ = tables.insert(key.table_name);
}
}
}
Expand Down
Loading