feat: establish RockQL compiler foundation - #1
Conversation
📝 WalkthroughWalkthroughThe PR establishes a Rust workspace for RockQL, adds AST, parser, SQL compiler, and CLI crates, documents syntax and contribution practices, includes an example query, and configures CI and security workflows. ChangesRockQL compiler foundation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant rockql_cli
participant rockql_parser
participant rockql_sql
User->>rockql_cli: Submit RockQL source and dialect
rockql_cli->>rockql_parser: Parse source
rockql_parser-->>rockql_cli: Return Query or diagnostics
rockql_cli->>rockql_sql: Compile Query for dialect
rockql_sql-->>rockql_cli: Return SQL or error
rockql_cli-->>User: Print result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
You've hit your review limit for the week, but don't worry you'll get some more next week! Contact us at hello@zenable.io if you want this rate limit to go away |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
rustfmt changes requireddiff --git a/compiler/rockql-parser/src/lib.rs b/compiler/rockql-parser/src/lib.rs
index e441dc9..6b22432 100644
--- a/compiler/rockql-parser/src/lib.rs
+++ b/compiler/rockql-parser/src/lib.rs
@@ -72,7 +72,12 @@ fn split_segments(source: &str) -> Vec<Segment> {
for (byte_index, character) in line.char_indices() {
if character == '|' {
- push_segment(&mut segments, &line[start..byte_index], line_index + 1, start);
+ push_segment(
+ &mut segments,
+ &line[start..byte_index],
+ line_index + 1,
+ start,
+ );
start = byte_index + character.len_utf8();
}
}
@@ -157,10 +162,7 @@ fn parse_select(rest: &str, span: Span) -> Result<Transform, Diagnostic> {
fn parse_derive(rest: &str, span: Span) -> Result<Transform, Diagnostic> {
let Some((name, expression)) = rest.split_once('=') else {
- return Err(Diagnostic::new(
- "expected `derive name = expression`",
- span,
- ));
+ return Err(Diagnostic::new("expected `derive name = expression`", span));
};
let name = name.trim();
@@ -181,12 +183,7 @@ fn parse_sort(rest: &str, span: Span) -> Result<Transform, Diagnostic> {
let inner = match (rest.strip_prefix('{'), rest.strip_suffix('}')) {
(Some(without_open), Some(_)) => &without_open[..without_open.len().saturating_sub(1)],
(None, None) => rest,
- _ => {
- return Err(Diagnostic::new(
- "sort braces must be balanced",
- span,
- ))
- }
+ _ => return Err(Diagnostic::new("sort braces must be balanced", span)),
};
let items = inner
@@ -217,9 +214,10 @@ fn parse_sort(rest: &str, span: Span) -> Result<Transform, Diagnostic> {
fn parse_take(rest: &str, span: Span) -> Result<Transform, Diagnostic> {
require_value(rest, "expected a row count after `take`", span)?;
- let count = rest.replace('_', "").parse::<u64>().map_err(|_| {
- Diagnostic::new("`take` requires a non-negative integer row count", span)
- })?;
+ let count = rest
+ .replace('_', "")
+ .parse::<u64>()
+ .map_err(|_| Diagnostic::new("`take` requires a non-negative integer row count", span))?;
Ok(Transform::Take { count })
}
@@ -253,8 +251,8 @@ mod tests {
#[test]
fn parses_pipe_separated_pipeline() {
- let query = parse("from users | filter active == true | take 10")
- .expect("query should parse");
+ let query =
+ parse("from users | filter active == true | take 10").expect("query should parse");
assert_eq!(query.transforms.len(), 3);
assert_eq!(query.transforms[1].span, Span::new(1, 14));
@@ -262,8 +260,8 @@ mod tests {
#[test]
fn reports_line_and_column() {
- let diagnostics = parse("from users\n unknown value")
- .expect_err("unknown transformation should fail");
+ let diagnostics =
+ parse("from users\n unknown value").expect_err("unknown transformation should fail");
assert_eq!(diagnostics[0].span, Span::new(2, 3));
assert!(diagnostics[0].message.contains("unknown transformation"));
@@ -271,12 +269,9 @@ mod tests {
#[test]
fn formats_to_canonical_multiline_source() {
- let formatted = format_source("from users | sort {-created_at} | take 5")
- .expect("query should format");
+ let formatted =
+ format_source("from users | sort {-created_at} | take 5").expect("query should format");
- assert_eq!(
- formatted,
- "from users\nsort {-created_at}\ntake 5\n"
- );
+ assert_eq!(formatted, "from users\nsort {-created_at}\ntake 5\n");
}
}
diff --git a/compiler/rockql-sql/src/lib.rs b/compiler/rockql-sql/src/lib.rs
index 8d56f24..ed1f931 100644
--- a/compiler/rockql-sql/src/lib.rs
+++ b/compiler/rockql-sql/src/lib.rs
@@ -39,7 +39,9 @@ impl FromStr for Dialect {
pub enum SqlError {
#[error("query must start with a `from` transformation")]
MissingFrom,
- #[error("only one `from` transformation is currently supported (line {line}, column {column})")]
+ #[error(
+ "only one `from` transformation is currently supported (line {line}, column {column})"
+ )]
MultipleFrom { line: usize, column: usize },
}
@@ -245,21 +247,18 @@ mod tests {
#[test]
fn compiles_boolean_filter_for_sqlite() {
- let query = parse("from users | filter active == true | take 10")
- .expect("query should parse");
+ let query =
+ parse("from users | filter active == true | take 10").expect("query should parse");
let sql = compile(&query, Dialect::Sqlite).expect("query should compile");
- assert_eq!(
- sql,
- "SELECT *\nFROM users\nWHERE active = TRUE\nLIMIT 10;"
- );
+ assert_eq!(sql, "SELECT *\nFROM users\nWHERE active = TRUE\nLIMIT 10;");
}
#[test]
fn preserves_keywords_inside_strings() {
- let query = parse("from events | filter label == \"true and false\"")
- .expect("query should parse");
+ let query =
+ parse("from events | filter label == \"true and false\"").expect("query should parse");
let sql = compile(&query, Dialect::Generic).expect("query should compile");
|
|
You've hit your review limit for the week, but don't worry you'll get some more next week! Contact us at hello@zenable.io if you want this rate limit to go away |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 20-21: Update the actions/checkout@v4 steps in both the CI and
security workflows to disable credential persistence, while leaving the existing
checkout behavior unchanged. Set persist-credentials to false for each checkout
action.
In @.github/workflows/security.yml:
- Around line 35-36: Update the Checkout step in the codeql job to disable
persisted credentials, matching the shared fix applied in ci.yml. Configure the
existing actions/checkout@v4 step with the appropriate persist-credentials
setting.
- Around line 22-23: Update the Checkout step in the dependency-review job of
security.yml to disable persisted credentials, matching the shared fix applied
in the CI workflow. Configure the existing actions/checkout@v4 step with the
appropriate persist-credentials setting while leaving the rest of the job
unchanged.
In `@compiler/rockql-parser/src/lib.rs`:
- Around line 67-102: Update split_segments to track whether scanning is inside
a quoted string, following the quote-handling pattern used by
normalize_keywords, and only treat | as a separator outside string literals.
Preserve push_segment trimming and span calculations, including support for
escaped quote characters as applicable to the existing literal syntax.
In `@compiler/rockql-sql/src/lib.rs`:
- Around line 154-179: Update normalize_expression and the normalization helpers
so numeric-separator removal and !=/== operator replacement occur within the
same quote-aware character walk used by normalize_keywords. Preserve all
characters inside quoted spans—including underscores and operator
sequences—while applying these transformations only outside quotes; remove or
bypass the current raw-string processing in remove_numeric_separators and
normalize_expression.
In `@README.md`:
- Line 141: Update the v0.3 roadmap entry in README.md to remove PostgreSQL if
it refers to already implemented output, or qualify it as expanded PostgreSQL
support; keep only genuinely future features such as MySQL alongside the
existing roadmap items.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 793dd4d3-a058-4c73-8d90-254abbef0a37
📒 Files selected for processing (16)
.github/workflows/ci.yml.github/workflows/security.yml.gitignoreCONTRIBUTING.mdCargo.tomlREADME.mdcompiler/rockql-ast/Cargo.tomlcompiler/rockql-ast/src/lib.rscompiler/rockql-cli/Cargo.tomlcompiler/rockql-cli/src/main.rscompiler/rockql-parser/Cargo.tomlcompiler/rockql-parser/src/lib.rscompiler/rockql-sql/Cargo.tomlcompiler/rockql-sql/src/lib.rsexamples/employees.rockqlrust-toolchain.toml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f110ccaf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Creates the first functional RockQL compiler foundation in Rust.
Included
from,filter,select,derive,sort, andtake.|-separated pipeline syntax.rockql compile,check,ast, andformatcommands.Example
Scope boundaries
This PR intentionally does not add joins, grouping, aggregation, type resolution, relational IR, WebAssembly, the web playground, Android, desktop, or release packaging. Those should build on this reviewed compiler boundary rather than being added as placeholders.
Validation
The branch uses Rust 1.97.1 because current Clap releases require stable Rust 2024 Cargo support.
Summary by CodeRabbit
New Features
Documentation
Quality & Security