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

fix reduce #80

Merged
merged 10 commits into from
Sep 12, 2023
Merged

fix reduce #80

merged 10 commits into from
Sep 12, 2023

Conversation

andicuko
Copy link
Contributor

@andicuko andicuko commented Aug 11, 2023

  • when the query has a group by and doesn't have aggregation functions

related to this issue.

@andicuko
Copy link
Contributor Author

Several files have been changed following cargo fmt command.

The actual changes are in src/relation/builders.rs and in src/sql/relation.rs to add a test.

@andicuko andicuko requested a review from ngrislain August 11, 2023 10:05
@andicuko andicuko marked this pull request as draft August 11, 2023 10:09
@andicuko andicuko marked this pull request as ready for review August 11, 2023 10:17
@@ -765,6 +767,7 @@ mod tests {
}

#[test]
#[ignore]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove all these

self.split = self
.split
.map_last_reduce(|reduce| reduce.and(Split::group_by(expr)));
self.split = self.split.and(Split::group_by(expr.into()).into());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure?

@@ -907,6 +907,28 @@ mod tests {
println!("query = {q}");
}

#[test]
fn test_reduce_with_only_group_by_columns() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure this is not working now without the change?

Is it working with piled map reduce: R, MR, RM, MRM, RMR

@ngrislain ngrislain merged commit a635320 into main Sep 12, 2023
1 check passed
@ngrislain ngrislain deleted the fix_reduce branch September 12, 2023 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants