GreptimeTeam / GreptimeTeam/promql-parser
Support mutation and rewriting during AST traversal
- Dominant language
- Rust
- Stars
- 104
- Forks
- 29
- Avg merge
- 15h 27m
- Merged PRs (30d)
- 1
Description
First of all, thank you for maintaining this project! From my experience, this crate is the only reliable PromQL parser available in Rust right now.
**Problem:**
Currently, [`walk_expr`](https://github.com/GreptimeTeam/promql-parser/blob/c4bebc845fc4b127dffc3392fbb9bd024770c33b/src/util/visitor.rs#L41) provides a great way to traverse the PromQL AST immutably. In the official Go implementation of the PromQL parser, traversal functions like [`Walk`](https://github.com/prometheus/prometheus/blob/d6c18a4d6292ec58da677984a2c810e35b6f1e2c/promql/parser/ast.go#L344-L368) and [`Inspect`](https://github.com/prometheus/prometheus/blob/d6c18a4d6292ec58da677984a2c810e35b6f1e2c/promql/parser/ast.go#L392-L399) naturally allow for AST mutation because Go passes pointers without strict mutability constraints.
However, `walk_expr` relies entirely on shared references (`&Expr`). This means if we need to manipulate the AST, such as injecting labels into selectors or optimizing queries, we currently lack built-in traversal patterns for mutation and restructuring.
---
Here are some thoughts on how to implement this feature:
**1. Mutation**
To support modifying internal fields in-place without allocating a new tree, we can introduce an `ExprMutVisitor` trait and a `walk_expr_mut` function (which takes `&mut Expr`).
However, duplicating the traversal logic across `walk_expr` and `walk_expr_mut` creates a maintenance burden, as they easily fall out of sync when new variants are added to `Expr`. Instead, we can write a single declarative macro that defines the AST destructuring logic once and generates both the `&` and `&mut` walker functions.
**2. Rewriting**
For structural changes where we need to replace or delete entire nodes, we can introduce an `ExprRewriter` trait and a `walk_and_rewrite` function (which takes ownership of `Expr` and returns a new one).
Unlike the reference walkers, the restructuring logic here is fundamentally different, making it impractical to generate all three walking functions with a single macro. Writing a separate, complex macro just for one single function definition feels like overkill, so it would likely be better to just implement `walk_and_rewrite` manually.
---
**Questions for maintainers:**
- Are you open to adding an `ExprMutVisitor` and an `ExprRewriter` trait?
- How do you feel about using a macro to deduplicate implementations?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.