search

mit-pdos / noria

Fast web applications through dynamic, partially-stateful dataflow
Apache License 2.0
4.97k stars 241 forks source link

Filtered aggregate #150

Closed jmbredenberg closed 4 years ago

jmbredenberg commented 4 years ago

Automatically merges adjacent filters and aggregations into a single operator when possible, so that a single traversal of the data performs both functions. Goes with the nom-sql commit adding syntax for COUNT/SUM(CASE WHEN x THEN y ELSE z END).

Adds various tests for the operator and for the automatic merging. Also refactors the way we store condition vectors, and adds support for generating such a vector for the AND of two condition trees.

jonhoo commented 4 years ago

@jmbredenberg

I read your comments -- thank you so much for the incredibly detailed feedback! I haven't used Rust enough to be familiar with the conventions, so it's very helpful -- but I probably won't get a chance to make the changes until after Weds.

Happy to! I hope it doesn't come across as too overwhelming. This is good work. I just wanted to make sure we didn't have anything slip through the cracks :) No rush in making the changes. Also, separately, I highly recommend cargo clippy — it's great at pointing out non-idiomatic Rust code!

jonhoo commented 4 years ago

As an aside, as you fix things, feel free to mark comments that have been addressed fully as "resolved" in the GitHub UI :)

tomharmon commented 4 years ago

Are there any things left that need finishing up that I could help with? I saw @ms705 's approval for the PR but still some open comments and am willing to help fix up any last issues

jmbredenberg commented 4 years ago

Are there any things left that need finishing up that I could help with? I saw @ms705 's approval for the PR but still some open comments and am willing to help fix up any last issues

Thanks for the offer! I was working on something else and got a bit distracted from finishing this up, but it should be done soon.