Skip to content

Commit

Permalink
Optimize the performance of filter operator
Browse files Browse the repository at this point in the history
  • Loading branch information
AmrDeveloper committed Dec 18, 2024
1 parent fa423d8 commit 5018222
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 40 deletions.
8 changes: 4 additions & 4 deletions crates/gitql-core/src/combinations_generator.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/// Return a list of all non empty and unique combinations
pub fn generate_list_of_all_combinations(n: usize) -> Vec<Vec<usize>> {
let mut result = Vec::with_capacity((2 << n) - 1);
let mut current = Vec::new();
generate_indeses_combination(n, 0, &mut current, &mut result);
let mut current = Vec::with_capacity(n);
generate_indices_combination(n, 0, &mut current, &mut result);
result
}

fn generate_indeses_combination(
fn generate_indices_combination(
n: usize,
start: usize,
current: &mut Vec<usize>,
Expand All @@ -18,7 +18,7 @@ fn generate_indeses_combination(

for i in start..n {
current.push(i);
generate_indeses_combination(n, i + 1, current, result);
generate_indices_combination(n, i + 1, current, result);
current.pop();
}
}
36 changes: 20 additions & 16 deletions crates/gitql-engine/src/engine_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,22 @@ fn execute_where_statement(
statement: &WhereStatement,
gitql_object: &mut GitQLObject,
) -> Result<(), String> {
if gitql_object.is_empty() {
return Ok(());
}

if gitql_object.len() > 1 {
gitql_object.flat()
}

// Perform where command only on the first group
// because group by command not executed yet
let condition = &statement.condition;
let main_group = &gitql_object.groups.first().unwrap().rows;
let rows = apply_filter_operation(env, condition, &gitql_object.titles, main_group)?;
let filtered_group: Group = Group { rows };

// Update the main group with the filtered data
gitql_object.groups.remove(0);
gitql_object.groups.push(filtered_group);
apply_filter_operation(
env,
&statement.condition,
&gitql_object.titles,
&mut gitql_object.groups[0].rows,
)?;

Ok(())
}
Expand All @@ -321,14 +327,12 @@ fn execute_having_statement(

// Perform where command only on the first group
// because group by command not executed yet
let condition = &statement.condition;
let main_group = &gitql_object.groups.first().unwrap().rows;
let rows = apply_filter_operation(env, condition, &gitql_object.titles, main_group)?;
let filtered_group: Group = Group { rows };

// Update the main group with the filtered data
gitql_object.groups.remove(0);
gitql_object.groups.push(filtered_group);
apply_filter_operation(
env,
&statement.condition,
&gitql_object.titles,
&mut gitql_object.groups[0].rows,
)?;

Ok(())
}
Expand Down
23 changes: 12 additions & 11 deletions crates/gitql-engine/src/engine_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@ pub(crate) fn apply_filter_operation(
env: &mut Environment,
condition: &Box<dyn Expr>,
titles: &[String],
objects: &Vec<Row>,
) -> Result<Vec<Row>, String> {
let mut rows: Vec<Row> = vec![];

for object in objects {
let expression = evaluate_expression(env, condition, titles, &object.values)?;
rows: &mut Vec<Row>,
) -> Result<(), String> {
let mut positions_to_delete = vec![];
for (index, row) in rows.iter().enumerate() {
let expression = evaluate_expression(env, condition, titles, &row.values)?;
if let Some(bool_value) = expression.as_any().downcast_ref::<BoolValue>() {
if bool_value.value {
rows.push(Row {
values: object.values.clone(),
});
if !bool_value.value {
positions_to_delete.push(index);
}
}
}

Ok(rows)
for position in positions_to_delete.iter().rev() {
rows.remove(*position);
}

Ok(())
}
22 changes: 13 additions & 9 deletions crates/gitql-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,7 @@ fn parse_select_query(
.as_boxed());
}

context.inside_having = true;
let statement = parse_having_statement(&mut context, env, tokens, position)?;
context.inside_having = false;
statements.insert("having", statement);
}
TokenKind::Limit => {
Expand All @@ -301,7 +299,7 @@ fn parse_select_query(
.as_boxed());
}

// Consume Comma
// Consume `,``
*position += 1;

if *position >= len || !matches!(tokens[*position].kind, TokenKind::Integer(_))
Expand Down Expand Up @@ -362,10 +360,7 @@ fn parse_select_query(
.as_boxed());
}

context.inside_order_by = true;
let statement = parse_order_by_statement(&mut context, env, tokens, position)?;
context.inside_order_by = false;

statements.insert("order", statement);
}
TokenKind::Into => {
Expand Down Expand Up @@ -905,7 +900,7 @@ fn parse_from_option(
// Make sure user set predicate condition for LEFT or RIGHT JOIN
if predicate.is_none() && matches!(join_kind, JoinKind::Right | JoinKind::Left) {
return Err(Diagnostic::error(
"You must set predicate condition using `ON` Keyword for LEFT OR RIHTH JOINS",
"You must set predicate condition using `ON` Keyword for `LEFT` OR `RIGHT` JOINS",
)
.with_location(join_location)
.as_boxed());
Expand Down Expand Up @@ -999,7 +994,7 @@ fn parse_group_by_statement(
tokens,
position,
TokenKind::By,
"Expect keyword `by` after keyword `group`",
"Expect keyword `BY` after keyword `group`",
)?;

// Parse one or more expression
Expand Down Expand Up @@ -1043,7 +1038,11 @@ fn parse_having_statement(
tokens: &[Token],
position: &mut usize,
) -> Result<Box<dyn Statement>, Box<Diagnostic>> {
context.inside_having = true;

// Consume `HAVING` token
*position += 1;

if *position >= tokens.len() {
return Err(
Diagnostic::error("Expect expression after `HAVING` keyword")
Expand Down Expand Up @@ -1079,6 +1078,7 @@ fn parse_having_statement(
})
}

context.inside_having = false;
Ok(Box::new(HavingStatement { condition }))
}

Expand Down Expand Up @@ -1163,6 +1163,8 @@ pub(crate) fn parse_order_by_statement(
// Consume `ORDER` keyword
*position += 1;

context.inside_order_by = true;

// Consume `BY` keyword
consume_token_or_error(
tokens,
Expand Down Expand Up @@ -1192,6 +1194,8 @@ pub(crate) fn parse_order_by_statement(
}
}

context.inside_order_by = false;

Ok(Box::new(OrderByStatement {
arguments,
sorting_orders,
Expand Down Expand Up @@ -1287,7 +1291,7 @@ fn parse_into_statement(
// Consume `INTO` keyword
*position += 1;

// Make sure user define explicity the into type
// Make sure user define explicitly the into type
if *position >= tokens.len()
|| (tokens[*position].kind != TokenKind::Outfile
&& tokens[*position].kind != TokenKind::Dumpfile)
Expand Down

0 comments on commit 5018222

Please sign in to comment.