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

Bump to Rust 1.83, fix clippy suggestions #649

Merged
merged 2 commits into from
Dec 2, 2024
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
4 changes: 3 additions & 1 deletion .github/workflows/schema-definitions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ jobs:
shared-key: "build" # share the cache across jobs
save-if: false

# currently there are no tests in here, leaving to ensure any that are
# added are run
- name: OpenAPI Definitions
run: cargo nextest run --no-fail-fast --release --filter-expr='package(openapi-generator)'
run: cargo nextest run --no-tests=pass --no-fail-fast --release --filter-expr='package(openapi-generator)'
5 changes: 2 additions & 3 deletions crates/query-engine/sql/src/sql/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,8 @@ impl Select {

sql.append_syntax(" ");

match &self.from {
Some(from) => from.to_sql(sql),
None => (),
if let Some(from) = &self.from {
from.to_sql(sql);
}

for join in &self.joins {
Expand Down
21 changes: 10 additions & 11 deletions crates/query-engine/translation/src/translation/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,17 +548,16 @@ impl State {
&mut self,
variables: &Option<Vec<BTreeMap<models::VariableName, serde_json::Value>>>,
) -> Option<(sql::ast::From, sql::ast::TableReference)> {
match variables {
None => None,
Some(_) => {
let variables_table_alias = self.make_table_alias("%variables_table".to_string());
let table_reference =
sql::ast::TableReference::AliasedTable(variables_table_alias.clone());
Some((
sql::helpers::from_variables(variables_table_alias),
table_reference,
))
}
if variables.is_none() {
None
} else {
let variables_table_alias = self.make_table_alias("%variables_table".to_string());
let table_reference =
sql::ast::TableReference::AliasedTable(variables_table_alias.clone());
Some((
sql::helpers::from_variables(variables_table_alias),
table_reference,
))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn generate(env: &Env) -> BTreeMap<models::ProcedureName, Mutation> {
.map(|(name, mutation)| (name, Mutation::V1(mutation)))
.collect(),
Some(mutations::MutationsVersion::V2) => {
v2::generate(&env.metadata.tables, &env.mutations_prefix)
v2::generate(&env.metadata.tables, env.mutations_prefix.as_ref())
.into_iter()
.map(|(name, mutation)| (name, Mutation::V2(mutation)))
.collect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ fn translate_mutation(
name: return_collection,
reference: sql::ast::TableReference::AliasedTable(cte_table_alias.clone()),
},
&None,
None,
&query,
)?;

Expand Down Expand Up @@ -227,7 +227,7 @@ fn translate_native_query(
name: procedure_name.to_string().into(),
reference: table_reference,
},
&None,
None,
&query,
)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ pub fn default_constraint() -> serde_json::Value {

// the old default was to prefix generated mutations with `v2_` or `v1_`
// but now we are able to override this
pub fn get_version_prefix(mutations_prefix: &Option<String>) -> String {
pub fn get_version_prefix(mutations_prefix: Option<&String>) -> String {
match mutations_prefix {
None => format!("{}_", super::VERSION),
Some(str) => match str.as_str() {
Expand All @@ -122,10 +122,13 @@ pub fn get_version_prefix(mutations_prefix: &Option<String>) -> String {

#[test]
fn test_version_prefix() {
assert_eq!(get_version_prefix(&None), "v2_".to_string());
assert_eq!(get_version_prefix(None), "v2_".to_string());
assert_eq!(
get_version_prefix(&Some("horse".into())),
get_version_prefix(Some("horse".into()).as_ref()),
"horse_".to_string()
);
assert_eq!(get_version_prefix(&Some(String::new())), String::new());
assert_eq!(
get_version_prefix(Some(String::new()).as_ref()),
String::new()
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub struct DeleteByKey {
pub fn generate_delete_by_unique(
collection_name: &models::CollectionName,
table_info: &database::TableInfo,
mutations_prefix: &Option<String>,
mutations_prefix: Option<&String>,
) -> Vec<(models::ProcedureName, DeleteMutation)> {
table_info
.uniqueness_constraints
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub enum Mutation {
/// Given our introspection data, work out all the mutations we can generate
pub fn generate(
tables_info: &database::TablesInfo,
mutations_prefix: &Option<String>,
mutations_prefix: Option<&String>,
) -> BTreeMap<models::ProcedureName, Mutation> {
let mut mutations = BTreeMap::new();
for (collection_name, table_info) in &tables_info.0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct InsertMutation {
pub fn generate(
collection_name: &models::CollectionName,
table_info: &database::TableInfo,
mutations_prefix: &Option<String>,
mutations_prefix: Option<&String>,
) -> (models::ProcedureName, InsertMutation) {
let name = format!(
"{}insert_{collection_name}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub fn translate(
),
Error,
> {
let mutation = lookup_generated_mutation(env, procedure_name, &env.mutations_prefix)?;
let mutation = lookup_generated_mutation(env, procedure_name, env.mutations_prefix.as_ref())?;

Ok(match mutation {
super::generate::Mutation::DeleteMutation(delete) => {
Expand Down Expand Up @@ -79,7 +79,7 @@ pub fn translate(
fn lookup_generated_mutation(
env: &Env<'_>,
procedure_name: &models::ProcedureName,
mutations_prefix: &Option<String>,
mutations_prefix: Option<&String>,
) -> Result<super::generate::Mutation, Error> {
// this means we generate them on every mutation request
// i don't think this is optimal but I'd like to get this working before working out
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct UpdateByKey {
pub fn generate_update_by_unique(
collection_name: &models::CollectionName,
table_info: &database::TableInfo,
mutations_prefix: &Option<String>,
mutations_prefix: Option<&String>,
) -> Vec<(models::ProcedureName, UpdateMutation)> {
table_info
.uniqueness_constraints
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub fn translate(
name: query_request.collection.clone(),
arguments: query_request.arguments.clone(),
},
&None,
None,
&query_request.query,
)?;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ pub fn translate(
arguments,
},
// We ask to inject the join predicate into the where clause.
&Some(root::JoinPredicate {
Some(root::JoinPredicate {
join_with: current_table,
relationship,
}),
})
.as_ref(),
&join_field.query,
)?;

Expand Down
16 changes: 10 additions & 6 deletions crates/query-engine/translation/src/translation/query/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub fn translate_query(
env: &Env,
state: &mut State,
make_from: &MakeFrom,
join_predicate: &Option<JoinPredicate<'_, '_>>,
join_predicate: Option<&JoinPredicate<'_, '_>>,
query_request: &models::Query,
) -> Result<sql::helpers::SelectSet, Error> {
// translate rows selection.
Expand Down Expand Up @@ -59,7 +59,7 @@ fn translate_aggregates(
env: &Env,
state: &mut State,
make_from: &MakeFrom,
join_predicate: &Option<JoinPredicate<'_, '_>>,
join_predicate: Option<&JoinPredicate<'_, '_>>,
query: &models::Query,
) -> Result<Option<sql::ast::Select>, Error> {
// fail if no aggregates defined at all
Expand Down Expand Up @@ -111,7 +111,7 @@ fn translate_rows(
env: &Env,
state: &mut State,
make_from: &MakeFrom,
join_predicate: &Option<JoinPredicate<'_, '_>>,
join_predicate: Option<&JoinPredicate<'_, '_>>,
query: &models::Query,
) -> Result<(ReturnsFields, sql::ast::Select), Error> {
let (current_table, from_clause) = make_reference_and_from_clause(env, state, make_from)?;
Expand Down Expand Up @@ -169,7 +169,7 @@ pub fn translate_query_part(
env: &Env,
state: &mut State,
current_table: &TableSourceAndReference,
join_predicate: &Option<JoinPredicate<'_, '_>>,
join_predicate: Option<&JoinPredicate<'_, '_>>,
query: &models::Query,
select: &mut sql::ast::Select,
) -> Result<(), Error> {
Expand All @@ -180,8 +180,12 @@ pub fn translate_query_part(
};

// translate order_by
let (order_by, order_by_joins) =
sorting::translate(env, state, &root_and_current_tables, &query.order_by)?;
let (order_by, order_by_joins) = sorting::translate(
env,
state,
&root_and_current_tables,
query.order_by.as_ref(),
)?;

select.joins.extend(order_by_joins);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn translate(
env: &Env,
state: &mut State,
root_and_current_tables: &RootAndCurrentTables,
order_by: &Option<models::OrderBy>,
order_by: Option<&models::OrderBy>,
) -> Result<(sql::ast::OrderBy, Vec<sql::ast::Join>), Error> {
let mut joins: Vec<sql::ast::Join> = vec![];
// skip if there's no order by clause.
Expand Down Expand Up @@ -632,7 +632,7 @@ fn process_path_element_for_order_by_targets(
current_table: last_table,
},
relationship,
&path_element.predicate,
path_element.predicate.as_deref(),
sql::ast::SelectList::SelectList(select_cols.aliases_and_expressions()),
(table.clone(), from_clause),
)?;
Expand Down Expand Up @@ -767,7 +767,7 @@ fn select_for_path_element(
state: &mut State,
root_and_current_tables: &RootAndCurrentTables,
relationship: &models::Relationship,
predicate: &Option<Box<models::Expression>>,
predicate: Option<&models::Expression>,
select_list: sql::ast::SelectList,
(join_table, from_clause): (TableSourceAndReference, sql::ast::From),
) -> Result<sql::ast::Select, Error> {
Expand Down
24 changes: 12 additions & 12 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
pkgs.nixpkgs-fmt
pkgs.nodePackages.prettier
pkgs.moreutils
pkgs.git

# Rust
pkgs.bacon
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[toolchain]
channel = "1.81.0"
channel = "1.83.0"
profile = "default" # see https://rust-lang.github.io/rustup/concepts/profiles.html
components = ["rust-analyzer", "rust-src"] # see https://rust-lang.github.io/rustup/concepts/components.html