Add from_json to compiler, add from_json and to_json to prql-js #782
Conversation
| @@ -293,6 +293,11 @@ impl Display for Item { | |||
| UnOp::Not => write!(f, "not {}", expr.item)?, | |||
| }, | |||
| Item::FuncCall(func_call) => { | |||
| match func_call.name.as_str() { | |||
There was a problem hiding this comment.
When generating a group, we would have had something like:
group [foo, bar]
aggregate [baz = sum foobar]
There were missing parentheses around aggregate, which led to invalid PRQL. With this code, we write an opening parenthesis before aggregate.
Another block of code writes the closing parenthesis, so that we get:
group [foo, bar] (
aggregate [baz = sum foobar]
)
There was a problem hiding this comment.
Actually this is wrong, as groups do not only have aggregates, eg.:
group employee_id (
sort month
window rolling:12 (
derive [trail_12_m_comp = sum paycheck]
)
)
We should really put parentheses around the group's "content" and not around aggregate.
There was a problem hiding this comment.
Great!
And it's fine to leave a comment in the code for something that's not quite there yet!
| @@ -7,13 +7,16 @@ mod semantic; | |||
| mod sql; | |||
| mod utils; | |||
|
|
|||
| use crate::{ast::Item}; | |||
There was a problem hiding this comment.
I don't know if you have a conventions (eg. should you put imports in alphabetical order, separate blocks of imports by line breaks, etc.)
Plus, I didn't research the difference between use and pub use.
There was a problem hiding this comment.
cargo fmt will organize these!
|
@florent-martineau this is really good. Congratulations on your first PR! As discussed on Discord, I'm happy to help finish this off — your call on how much you want to do yourself vs ask for help. To merge, we need a couple of additions:
|
|
@florent-martineau FYI I merged the latest main into your branch, so run |
|
@florent-martineau thanks for kicking this off! We merged #829 so I'll close this. But congrats on your first open-source PR, and we'd be happy to work together on another. |
The goal of this PR is:
Please note that it's my first PR on an open source repository, plus I had never seen a line of Rust until yesterday, so please be indulgent 😅