From 9e9feb4133a73e9455531ad71715f29fe71a2bba Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Thu, 16 Nov 2023 18:13:58 -0800 Subject: [PATCH] Cleanup pgrx sql entity graph more (#1387) A second pass at cleanup. While the first was simply moving code around, this flattens out significant parts of the logic using more streamlined expressions so that the rest is more readable. No functional changes. --- .../src/extension_sql/entity.rs | 20 +- pgrx-sql-entity-graph/src/lib.rs | 17 +- pgrx-sql-entity-graph/src/lifetimes.rs | 47 ++-- .../src/pg_extern/entity/mod.rs | 21 +- pgrx-sql-entity-graph/src/pg_extern/mod.rs | 3 +- .../src/pg_trigger/entity.rs | 6 +- pgrx-sql-entity-graph/src/pgrx_sql.rs | 216 ++++++------------ .../src/postgres_ord/entity.rs | 29 ++- pgrx-sql-entity-graph/src/schema/entity.rs | 5 +- 9 files changed, 129 insertions(+), 235 deletions(-) diff --git a/pgrx-sql-entity-graph/src/extension_sql/entity.rs b/pgrx-sql-entity-graph/src/extension_sql/entity.rs index 81bb926cd..a4528a38f 100644 --- a/pgrx-sql-entity-graph/src/extension_sql/entity.rs +++ b/pgrx-sql-entity-graph/src/extension_sql/entity.rs @@ -192,9 +192,9 @@ impl SqlDeclaredEntity { pub fn has_sql_declared_entity(&self, identifier: &SqlDeclared) -> bool { match (&identifier, &self) { - (SqlDeclared::Type(identifier_name), &SqlDeclaredEntity::Type(data)) - | (SqlDeclared::Enum(identifier_name), &SqlDeclaredEntity::Enum(data)) - | (SqlDeclared::Function(identifier_name), &SqlDeclaredEntity::Function(data)) => { + (SqlDeclared::Type(ident_name), &SqlDeclaredEntity::Type(data)) + | (SqlDeclared::Enum(ident_name), &SqlDeclaredEntity::Enum(data)) + | (SqlDeclared::Function(ident_name), &SqlDeclaredEntity::Function(data)) => { let matches = |identifier_name: &str| { identifier_name == &data.name || identifier_name == &data.option @@ -206,21 +206,15 @@ impl SqlDeclaredEntity { || identifier_name == &data.option_array || identifier_name == &data.varlena }; - if matches(identifier_name) || data.pg_box.contains(identifier_name) { + if matches(ident_name) || data.pg_box.contains(ident_name) { return true; } // there are cases where the identifier is // `core::option::Option` while the data stores // `Option` check again for this - let generics_start = match identifier_name.find('<') { - None => return false, - Some(idx) => idx, - }; - let qualification_end = match identifier_name[..generics_start].rfind("::") { - None => return false, - Some(idx) => idx, - }; - matches(&identifier_name[qualification_end + 2..]) + let Some(generics_start) = ident_name.find('<') else { return false }; + let Some(qual_end) = ident_name[..generics_start].rfind("::") else { return false }; + matches(&ident_name[qual_end + 2..]) } _ => false, } diff --git a/pgrx-sql-entity-graph/src/lib.rs b/pgrx-sql-entity-graph/src/lib.rs index 318ad0d98..eda2ca7f5 100644 --- a/pgrx-sql-entity-graph/src/lib.rs +++ b/pgrx-sql-entity-graph/src/lib.rs @@ -110,7 +110,7 @@ pub enum SqlGraphEntity { impl SqlGraphEntity { pub fn sql_anchor_comment(&self) -> String { let maybe_file_and_line = if let (Some(file), Some(line)) = (self.file(), self.line()) { - format!("-- {file}:{line}\n", file = file, line = line) + format!("-- {file}:{line}\n") } else { String::default() }; @@ -119,7 +119,6 @@ impl SqlGraphEntity { {maybe_file_and_line}\ -- {rust_identifier}\ ", - maybe_file_and_line = maybe_file_and_line, rust_identifier = self.rust_identifier(), ) } @@ -132,7 +131,7 @@ impl SqlGraphIdentifier for SqlGraphEntity { SqlGraphEntity::CustomSql(item) => item.dot_identifier(), SqlGraphEntity::Function(item) => item.dot_identifier(), SqlGraphEntity::Type(item) => item.dot_identifier(), - SqlGraphEntity::BuiltinType(item) => format!("preexisting type {}", item), + SqlGraphEntity::BuiltinType(item) => format!("preexisting type {item}"), SqlGraphEntity::Enum(item) => item.dot_identifier(), SqlGraphEntity::Ord(item) => item.dot_identifier(), SqlGraphEntity::Hash(item) => item.dot_identifier(), @@ -268,18 +267,16 @@ impl ToSql for SqlGraphEntity { pub fn ident_is_acceptable_to_postgres(ident: &syn::Ident) -> Result<(), syn::Error> { // Roughly `pgrx::pg_sys::NAMEDATALEN` // - // Technically it **should** be that exactly, however this is `pgrx-utils` and a this data is used at macro time. + // Technically it **should** be that, but we need to guess at build time const POSTGRES_IDENTIFIER_MAX_LEN: usize = 64; - let ident_string = ident.to_string(); - if ident_string.len() >= POSTGRES_IDENTIFIER_MAX_LEN { + let len = ident.to_string().len(); + if len >= POSTGRES_IDENTIFIER_MAX_LEN { return Err(syn::Error::new( ident.span(), format!( - "Identifier `{}` was {} characters long, PostgreSQL will truncate identifiers with less than \ - {POSTGRES_IDENTIFIER_MAX_LEN} characters, opt for an identifier which Postgres won't truncate", - ident, - ident_string.len(), + "Identifier `{ident}` was {len} characters long, PostgreSQL will truncate identifiers with less than \ + {POSTGRES_IDENTIFIER_MAX_LEN} characters, opt for an identifier which Postgres won't truncate" ) )); } diff --git a/pgrx-sql-entity-graph/src/lifetimes.rs b/pgrx-sql-entity-graph/src/lifetimes.rs index cbd36504d..0c539813b 100644 --- a/pgrx-sql-entity-graph/src/lifetimes.rs +++ b/pgrx-sql-entity-graph/src/lifetimes.rs @@ -117,38 +117,35 @@ pub fn anonymize_lifetimes(value: &mut syn::Type) { match value { syn::Type::Path(type_path) => { for segment in &mut type_path.path.segments { - match &mut segment.arguments { - syn::PathArguments::AngleBracketed(bracketed) => { - for arg in &mut bracketed.args { - match arg { - // rename lifetimes to the anonymous lifetime - syn::GenericArgument::Lifetime(lifetime) => { - lifetime.ident = syn::Ident::new("_", lifetime.ident.span()); - } + if let syn::PathArguments::AngleBracketed(bracketed) = &mut segment.arguments { + for arg in &mut bracketed.args { + match arg { + // rename lifetimes to the anonymous lifetime + syn::GenericArgument::Lifetime(lifetime) => { + lifetime.ident = syn::Ident::new("_", lifetime.ident.span()); + } - // recurse - syn::GenericArgument::Type(ty) => anonymize_lifetimes(ty), - syn::GenericArgument::Binding(binding) => { - anonymize_lifetimes(&mut binding.ty) - } - syn::GenericArgument::Constraint(constraint) => { - for bound in constraint.bounds.iter_mut() { - match bound { - syn::TypeParamBound::Lifetime(lifetime) => { - lifetime.ident = - syn::Ident::new("_", lifetime.ident.span()) - } - _ => {} + // recurse + syn::GenericArgument::Type(ty) => anonymize_lifetimes(ty), + syn::GenericArgument::Binding(binding) => { + anonymize_lifetimes(&mut binding.ty) + } + syn::GenericArgument::Constraint(constraint) => { + for bound in constraint.bounds.iter_mut() { + match bound { + syn::TypeParamBound::Lifetime(lifetime) => { + lifetime.ident = + syn::Ident::new("_", lifetime.ident.span()) } + _ => {} } } - - // nothing to do otherwise - _ => {} } + + // nothing to do otherwise + _ => {} } } - _ => {} } } } diff --git a/pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs b/pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs index a24309057..f800c1506 100644 --- a/pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs +++ b/pgrx-sql-entity-graph/src/pg_extern/entity/mod.rs @@ -273,11 +273,10 @@ impl ToSql for PgExternEntity { PgExternReturnEntity::Iterated { tys: table_items, optional: _, result: _ } => { let mut items = String::new(); let metadata_retval = self.metadata.retval.clone().ok_or_else(|| eyre!("Macro expansion time and SQL resolution time had differing opinions about the return value existing"))?; - let metadata_retval_sqls = match metadata_retval.return_sql { + let metadata_retval_sqls: Vec = match metadata_retval.return_sql { Ok(Returns::Table(variants)) => { - let mut retval_sqls = vec![]; - for (idx, variant) in variants.iter().enumerate() { - let sql = match variant { + variants.iter().enumerate().map(|(idx, variant)| { + match variant { SqlMapping::As(sql) => sql.clone(), SqlMapping::Composite { array_brackets } => { let composite = table_items[idx].ty.composite_type.unwrap(); @@ -287,10 +286,8 @@ impl ToSql for PgExternEntity { fmt::with_array_brackets(context.source_only_to_sql_type(table_items[idx].ty.ty_source).unwrap(), *array_brackets) } SqlMapping::Skip => todo!(), - }; - retval_sqls.push(sql) - } - retval_sqls + } + }).collect() }, Ok(_other) => return Err(eyre!("Got non-table return variant SQL in what macro-expansion thought was a table")), Err(err) => return Err(err).wrap_err("Error mapping return SQL"), @@ -302,11 +299,11 @@ impl ToSql for PgExternEntity { let graph_index = context.graph.neighbors_undirected(self_index).find(|neighbor| { match &context.graph[*neighbor] { - SqlGraphEntity::Type(neightbor_ty) => { - neightbor_ty.id_matches(&ty.ty_id) + SqlGraphEntity::Type(neighbor_ty) => { + neighbor_ty.id_matches(&ty.ty_id) } - SqlGraphEntity::Enum(neightbor_en) => { - neightbor_en.id_matches(&ty.ty_id) + SqlGraphEntity::Enum(neighbor_en) => { + neighbor_en.id_matches(&ty.ty_id) } SqlGraphEntity::BuiltinType(defined) => defined == ty.ty_source, _ => false, diff --git a/pgrx-sql-entity-graph/src/pg_extern/mod.rs b/pgrx-sql-entity-graph/src/pg_extern/mod.rs index aed33e8ae..a774b5eb2 100644 --- a/pgrx-sql-entity-graph/src/pg_extern/mod.rs +++ b/pgrx-sql-entity-graph/src/pg_extern/mod.rs @@ -190,12 +190,11 @@ impl PgExtern { } else if in_commented_sql_block && inner.value().trim() == "```" { in_commented_sql_block = false; } else if in_commented_sql_block { - let sql = retval.get_or_insert_with(String::default); let line = inner.value().trim_start().replace( "@FUNCTION_NAME@", &(self.func.sig.ident.to_string() + "_wrapper"), ) + "\n"; - sql.push_str(&line); + retval.get_or_insert_with(String::default).push_str(&line); } } } diff --git a/pgrx-sql-entity-graph/src/pg_trigger/entity.rs b/pgrx-sql-entity-graph/src/pg_trigger/entity.rs index 3027f4377..0d9997de8 100644 --- a/pgrx-sql-entity-graph/src/pg_trigger/entity.rs +++ b/pgrx-sql-entity-graph/src/pg_trigger/entity.rs @@ -44,6 +44,7 @@ impl ToSql for PgTriggerEntity { let self_index = context.triggers[self]; let schema = context.schema_prefix_for(&self_index); + let PgTriggerEntity { file, line, full_path, function_name, .. } = self; let sql = format!( "\n\ -- {file}:{line}\n\ @@ -52,11 +53,6 @@ impl ToSql for PgTriggerEntity { \tRETURNS TRIGGER\n\ \tLANGUAGE c\n\ \tAS 'MODULE_PATHNAME', '{wrapper_function_name}';", - schema = schema, - file = self.file, - line = self.line, - full_path = self.full_path, - function_name = self.function_name, wrapper_function_name = self.wrapper_function_name(), ); Ok(sql) diff --git a/pgrx-sql-entity-graph/src/pgrx_sql.rs b/pgrx-sql-entity-graph/src/pgrx_sql.rs index 1862358f1..d1198ea61 100644 --- a/pgrx-sql-entity-graph/src/pgrx_sql.rs +++ b/pgrx-sql-entity-graph/src/pgrx_sql.rs @@ -334,51 +334,41 @@ impl PgrxSql { .to_owned() }, &|_graph, (_index, node)| { + let dot_id = node.dot_identifier(); match node { // Colors derived from https://www.schemecolor.com/touch-of-creativity.php SqlGraphEntity::Schema(_item) => format!( - "label = \"{}\", weight = 6, shape = \"tab\"", - node.dot_identifier() + "label = \"{dot_id}\", weight = 6, shape = \"tab\"" ), SqlGraphEntity::Function(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#ADC7C6\", weight = 4, shape = \"box\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#ADC7C6\", weight = 4, shape = \"box\"", ), SqlGraphEntity::Type(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#AE9BBD\", weight = 5, shape = \"oval\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#AE9BBD\", weight = 5, shape = \"oval\"", ), SqlGraphEntity::BuiltinType(_item) => format!( - "label = \"{}\", shape = \"plain\"", - node.dot_identifier() + "label = \"{dot_id}\", shape = \"plain\"" ), SqlGraphEntity::Enum(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#C9A7C8\", weight = 5, shape = \"oval\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#C9A7C8\", weight = 5, shape = \"oval\"" ), SqlGraphEntity::Ord(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFCFD3\", weight = 5, shape = \"diamond\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFCFD3\", weight = 5, shape = \"diamond\"" ), SqlGraphEntity::Hash(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"" ), SqlGraphEntity::Aggregate(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"" ), SqlGraphEntity::Trigger(_item) => format!( - "label = \"{}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"", - node.dot_identifier() + "label = \"{dot_id}\", penwidth = 0, style = \"filled\", fillcolor = \"#FFE4E0\", weight = 5, shape = \"diamond\"" ), SqlGraphEntity::CustomSql(_item) => format!( - "label = \"{}\", weight = 3, shape = \"signature\"", - node.dot_identifier() + "label = \"{dot_id}\", weight = 3, shape = \"signature\"" ), SqlGraphEntity::ExtensionRoot(_item) => format!( - "label = \"{}\", shape = \"cylinder\"", - node.dot_identifier() + "label = \"{dot_id}\", shape = \"cylinder\"" ), } }, @@ -422,12 +412,8 @@ impl PgrxSql { })? { let step = &self.graph[step_id]; - let sql = step.to_sql(self)?; - - if !sql.is_empty() { - full_sql.push_str(&sql); - full_sql.push('\n'); - } + let sql = step.to_sql(self).map(|s| s + "\n")?; + full_sql.push_str(&sql); } Ok(full_sql) } @@ -751,19 +737,9 @@ fn initialize_externs( build_base_edges(graph, index, root, bootstrap, finalize); for arg in &item.fn_args { - let mut found = false; - for (ty_item, &_ty_index) in mapped_types { - if ty_item.id_matches(&arg.used_ty.ty_id) { - found = true; - break; - } - } - for (ty_item, &_ty_index) in mapped_enums { - if ty_item.id_matches(&arg.used_ty.ty_id) { - found = true; - break; - } - } + let found = mapped_types.keys().any(|ty_item| ty_item.id_matches(&arg.used_ty.ty_id)) + || mapped_enums.keys().any(|ty_item| ty_item.id_matches(&arg.used_ty.ty_id)); + if !found { mapped_builtin_types.entry(arg.used_ty.full_path.to_string()).or_insert_with( || { @@ -778,19 +754,9 @@ fn initialize_externs( match &item.fn_return { PgExternReturnEntity::None | PgExternReturnEntity::Trigger => (), PgExternReturnEntity::Type { ty, .. } | PgExternReturnEntity::SetOf { ty, .. } => { - let mut found = false; - for (ty_item, &_ty_index) in mapped_types { - if ty_item.id_matches(&ty.ty_id) { - found = true; - break; - } - } - for (ty_item, &_ty_index) in mapped_enums { - if ty_item.id_matches(&ty.ty_id) { - found = true; - break; - } - } + let found = mapped_types.keys().any(|ty_item| ty_item.id_matches(&ty.ty_id)) + || mapped_enums.keys().any(|ty_item| ty_item.id_matches(&ty.ty_id)); + if !found { mapped_builtin_types.entry(ty.full_path.to_string()).or_insert_with(|| { graph.add_node(SqlGraphEntity::BuiltinType(ty.full_path.to_string())) @@ -798,30 +764,14 @@ fn initialize_externs( } } PgExternReturnEntity::Iterated { tys: iterated_returns, optional: _, result: _ } => { - for PgExternReturnEntityIteratedItem { ty: return_ty_entity, .. } in - iterated_returns - { - let mut found = false; - for (ty_item, &_ty_index) in mapped_types { - if ty_item.id_matches(&return_ty_entity.ty_id) { - found = true; - break; - } - } - for (ty_item, &_ty_index) in mapped_enums { - if ty_item.id_matches(&return_ty_entity.ty_id) { - found = true; - break; - } - } + for PgExternReturnEntityIteratedItem { ty, .. } in iterated_returns { + let found = mapped_types.keys().any(|ty_item| ty_item.id_matches(&ty.ty_id)) + || mapped_enums.keys().any(|ty_item| ty_item.id_matches(&ty.ty_id)); + if !found { - mapped_builtin_types - .entry(return_ty_entity.ty_source.to_string()) - .or_insert_with(|| { - graph.add_node(SqlGraphEntity::BuiltinType( - return_ty_entity.ty_source.to_string(), - )) - }); + mapped_builtin_types.entry(ty.ty_source.to_string()).or_insert_with(|| { + graph.add_node(SqlGraphEntity::BuiltinType(ty.ty_source.to_string())) + }); } } } @@ -949,31 +899,26 @@ fn connect_externs( match &item.fn_return { PgExternReturnEntity::None | PgExternReturnEntity::Trigger => (), PgExternReturnEntity::Type { ty, .. } | PgExternReturnEntity::SetOf { ty, .. } => { - let mut found = false; - for (ty_item, &ty_index) in types { - if ty_item.id_matches(&ty.ty_id) { - graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); - found = true; - break; - } + let found_ty = types.iter().find(|(ty_item, _)| ty_item.id_matches(&ty.ty_id)); + if let Some((_, &ty_index)) = found_ty { + graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); } - if !found { - for (ty_item, &ty_index) in enums { - if ty_item.id_matches(&ty.ty_id) { - graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); - found = true; - break; - } - } + let found_enum = if found_ty.is_none() { + enums.iter().find(|(ty_item, _)| ty_item.id_matches(&ty.ty_id)) + } else { + None + }; + if let Some((_, &ty_index)) = found_enum { + graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); } - if !found { + if found_ty.is_none() && found_enum.is_none() { let builtin_index = builtin_types.get(&ty.full_path.to_string()).unwrap_or_else(|| { panic!("Could not fetch Builtin Type {}.", ty.full_path) }); graph.add_edge(*builtin_index, index, SqlGraphRelationship::RequiredByReturn); } - if !found { + if found_ty.is_none() && found_enum.is_none() { for (ext_item, ext_index) in extension_sqls { if ext_item .has_sql_declared_entity(&SqlDeclared::Type(ty.full_path.to_string())) @@ -991,28 +936,20 @@ fn connect_externs( } PgExternReturnEntity::Iterated { tys: iterated_returns, optional: _, result: _ } => { for PgExternReturnEntityIteratedItem { ty: type_entity, .. } in iterated_returns { - let mut found = false; - for (ty_item, &ty_index) in types { - if ty_item.id_matches(&type_entity.ty_id) { - graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); - found = true; - break; - } + let found_ty = + types.iter().find(|(ty_item, _)| ty_item.id_matches(&type_entity.ty_id)); + if let Some((_, &ty_index)) = found_ty { + graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); } - if !found { - for (ty_item, &ty_index) in enums { - if ty_item.id_matches(&type_entity.ty_id) { - graph.add_edge( - ty_index, - index, - SqlGraphRelationship::RequiredByReturn, - ); - found = true; - break; - } - } + let found_enum = if found_ty.is_none() { + enums.iter().find(|(ty_item, _)| ty_item.id_matches(&type_entity.ty_id)) + } else { + None + }; + if let Some((_, &ty_index)) = found_enum { + graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredByReturn); } - if !found { + if found_ty.is_none() && found_enum.is_none() { let builtin_index = builtin_types .get(&type_entity.ty_source.to_string()) .unwrap_or_else(|| { @@ -1024,7 +961,7 @@ fn connect_externs( SqlGraphRelationship::RequiredByReturn, ); } - if !found { + if found_ty.is_none() && found_enum.is_none() { for (ext_item, ext_index) in extension_sqls { if ext_item .has_sql_declared_entity(&SqlDeclared::Type( @@ -1171,15 +1108,10 @@ fn connect_hashes( enums, ); - for (extern_item, &extern_index) in externs { - let hash_fn_name = item.fn_name(); - let hash_fn_matches = - item.module_path == extern_item.module_path && extern_item.name == hash_fn_name; - - if hash_fn_matches { - graph.add_edge(extern_index, index, SqlGraphRelationship::RequiredBy); - break; - } + if let Some((_, extern_index)) = externs.into_iter().find(|(extern_item, _)| { + item.module_path == extern_item.module_path && extern_item.name == item.fn_name() + }) { + graph.add_edge(*extern_index, index, SqlGraphRelationship::RequiredBy); } } } @@ -1482,17 +1414,12 @@ fn make_extern_connection( full_path: &str, externs: &HashMap, ) -> eyre::Result<()> { - let mut found = false; - for (extern_item, &extern_index) in externs { - if full_path == extern_item.full_path { - graph.add_edge(extern_index, index, SqlGraphRelationship::RequiredBy); - found = true; - break; + match externs.into_iter().find(|(extern_item, _)| full_path == extern_item.full_path) { + Some((_, extern_index)) => { + graph.add_edge(*extern_index, index, SqlGraphRelationship::RequiredBy); + Ok(()) } - } - match found { - true => Ok(()), - false => Err(eyre!("Did not find connection `{full_path}` in {:#?}", { + None => Err(eyre!("Did not find connection `{full_path}` in {:#?}", { let mut paths = externs.iter().map(|(v, _)| v.full_path).collect::>(); paths.sort(); paths @@ -1509,21 +1436,14 @@ fn make_type_or_enum_connection( types: &HashMap, enums: &HashMap, ) -> bool { - let mut found = false; - for (ty_item, &ty_index) in types { - if ty_item.id_matches(ty_id) { - graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredBy); - found = true; - break; - } + let found_ty = types.into_iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); + if let Some((_, ty_index)) = found_ty { + graph.add_edge(*ty_index, index, SqlGraphRelationship::RequiredBy); } - for (ty_item, &ty_index) in enums { - if ty_item.id_matches(ty_id) { - graph.add_edge(ty_index, index, SqlGraphRelationship::RequiredBy); - found = true; - break; - } + let found_enum = enums.into_iter().find(|(ty_item, _)| ty_item.id_matches(ty_id)); + if let Some((_, ty_index)) = found_enum { + graph.add_edge(*ty_index, index, SqlGraphRelationship::RequiredBy); } - found + found_ty.is_some() || found_enum.is_some() } diff --git a/pgrx-sql-entity-graph/src/postgres_ord/entity.rs b/pgrx-sql-entity-graph/src/postgres_ord/entity.rs index c2feef97d..d573e3454 100644 --- a/pgrx-sql-entity-graph/src/postgres_ord/entity.rs +++ b/pgrx-sql-entity-graph/src/postgres_ord/entity.rs @@ -83,23 +83,20 @@ impl SqlGraphIdentifier for PostgresOrdEntity { impl ToSql for PostgresOrdEntity { fn to_sql(&self, _context: &PgrxSql) -> eyre::Result { + let PostgresOrdEntity { name, full_path, file, line, .. } = self; let sql = format!("\n\ - -- {file}:{line}\n\ - -- {full_path}\n\ - CREATE OPERATOR FAMILY {name}_btree_ops USING btree;\n\ - CREATE OPERATOR CLASS {name}_btree_ops DEFAULT FOR TYPE {name} USING btree FAMILY {name}_btree_ops AS\n\ - \tOPERATOR 1 <,\n\ - \tOPERATOR 2 <=,\n\ - \tOPERATOR 3 =,\n\ - \tOPERATOR 4 >=,\n\ - \tOPERATOR 5 >,\n\ - \tFUNCTION 1 {cmp_fn_name}({name}, {name});\ - ", - name = self.name, - full_path = self.full_path, - file = self.file, - line = self.line, - cmp_fn_name = self.cmp_fn_name(), + -- {file}:{line}\n\ + -- {full_path}\n\ + CREATE OPERATOR FAMILY {name}_btree_ops USING btree;\n\ + CREATE OPERATOR CLASS {name}_btree_ops DEFAULT FOR TYPE {name} USING btree FAMILY {name}_btree_ops AS\n\ + \tOPERATOR 1 <,\n\ + \tOPERATOR 2 <=,\n\ + \tOPERATOR 3 =,\n\ + \tOPERATOR 4 >=,\n\ + \tOPERATOR 5 >,\n\ + \tFUNCTION 1 {cmp_fn_name}({name}, {name});\ + ", + cmp_fn_name = self.cmp_fn_name(), ); Ok(sql) } diff --git a/pgrx-sql-entity-graph/src/schema/entity.rs b/pgrx-sql-entity-graph/src/schema/entity.rs index 9f6a92afd..516eb8748 100644 --- a/pgrx-sql-entity-graph/src/schema/entity.rs +++ b/pgrx-sql-entity-graph/src/schema/entity.rs @@ -53,15 +53,12 @@ impl SqlGraphIdentifier for SchemaEntity { impl ToSql for SchemaEntity { fn to_sql(&self, _context: &PgrxSql) -> eyre::Result { + let SchemaEntity { name, file, line, module_path } = self; let sql = format!( "\n\ -- {file}:{line}\n\ CREATE SCHEMA IF NOT EXISTS {name}; /* {module_path} */\ ", - name = self.name, - file = self.file, - line = self.line, - module_path = self.module_path, ); Ok(sql) }