Skip to content

⚡ Bolt: [performance improvement] Remove intermediate String allocations #9

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.

Sign up for GitHub

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

Merged
SayanthRock merged 1 commit into from
Aug 12, 2026
+41 −10
Merged

⚡ Bolt: [performance improvement] Remove intermediate String allocations #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions compiler/rockql-parser/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,36 @@ 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))?;
// ⚡ Bolt Optimization: Manually parse the integer without allocating an intermediate
// String via `.replace('_', "")`. This directly iterates over bytes, ignoring `_`.
let mut count: u64 = 0;
let mut has_digits = false;
for &byte in rest.as_bytes() {
if byte == b'_' {
continue;
} else if byte.is_ascii_digit() {
has_digits = true;
let digit = (byte - b'0') as u64;
count = count
.checked_mul(10)
.and_then(|c| c.checked_add(digit))
.ok_or_else(|| {
Diagnostic::new("`take` requires a non-negative integer row count", span)
})?;
} else {
return Err(Diagnostic::new(
"`take` requires a non-negative integer row count",
span,
));
}
Comment on lines +221 to +238

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

if ! rg -n -P 'take\s+\+[_0-9]+' compiler/rockql-parser --glob '*.rs'; then
  echo "Missing regression coverage for plus-prefixed TAKE counts." >&2
  exit 1
fi

Repository: Sayanthrock-Developer/ROCKQL

Length of output: 225

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- parser source ---'
sed -n '180,270p' compiler/rockql-parser/src/lib.rs

printf '%s\n' '--- TAKE-related tests and implementations ---'
rg -n -C 4 'take|TAKE|from_str|parse::<u64>' compiler/rockql-parser --glob '*.rs'

printf '%s\n' '--- repository status ---'
git diff --stat
git status --short

Repository: Sayanthrock-Developer/ROCKQL

Length of output: 7932

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- recent commits ---'
git log --oneline -5

printf '%s\n' '--- command parsing around TAKE dispatch ---'
sed -n '60,135p' compiler/rockql-parser/src/lib.rs

printf '%s\n' '--- prior parse_take implementation ---'
parent="$(git rev-parse HEAD^)"
git show "$parent:compiler/rockql-parser/src/lib.rs" | sed -n '205,255p'

printf '%s\n' '--- current and prior parse_take diffs ---'
git diff "$parent" HEAD -- compiler/rockql-parser/src/lib.rs | sed -n '/parse_take/,$p'

Repository: Sayanthrock-Developer/ROCKQL

Length of output: 5738

🌐 Web query:

Rust u64 FromStr leading plus sign underscores syntax "+_10"

💡 Result:

In Rust, the standard library's FromStr implementation for unsigned integer types like u64 does not support leading plus signs (+) or underscores (_) in input strings [1][2]. When you attempt to parse such strings, the operation will return an Err (specifically ParseIntError with kind: InvalidDigit) [1][2]. 1. Leading Plus Sign: While some other programming languages permit a leading + sign for positive integers, Rust's str::parse::<u64>() and u64::from_str() are strictly defined to reject them [1][3][2]. To handle strings that might contain a leading +, you must explicitly remove it before parsing, for example by using s.trim_start_matches('+').parse::<u64>() [1]. 2. Underscores: Similarly, FromStr does not support underscores (e.g., 1_000) within the string [4]. If your input data contains underscores, you must strip them before calling parse() [4]. A common approach is to use s.replace('_', "").parse::<u64>() [4]. These behaviors are intentional design choices in Rust's standard library to maintain strict, predictable parsing rules [1][2]. Top Results: [1], [3], [2], [4]

Citations:

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

if ! command -v rustc >/dev/null 2>&1; then
  echo "rustc is unavailable"
  exit 0
fi

rustc --edition=2021 -o /tmp/rockql_u64_probe - <<'RS'
fn main() {
    for input in ["+10", "+_10", "10", "1_0", "_10", "10_"] {
        let old = input.replace('_', "").parse::<u64>().ok();
        println!("{input:?}: old_parse={old:?}");
    }
}
RS

/tmp/rockql_u64_probe

Repository: Sayanthrock-Developer/ROCKQL

Length of output: 322

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rustc --edition=2021 -o /tmp/rockql_take_compat_probe - <<'RS'
fn old(input: &str) -> bool {
    input.replace('_', "").parse::<u64>().is_ok()
}

fn current(input: &str) -> bool {
    let mut count = 0u64;
    let mut has_digits = false;

    for &byte in input.as_bytes() {
        if byte == b'_' {
            continue;
        } else if byte.is_ascii_digit() {
            has_digits = true;
            let digit = (byte - b'0') as u64;
            count = match count.checked_mul(10).and_then(|c| c.checked_add(digit)) {
                Some(value) => value,
                None => return false,
            };
        } else {
            return false;
        }
    }

    has_digits
}

fn proposed(input: &str) -> bool {
    let mut count = 0u64;
    let mut has_digits = false;
    let mut has_plus = false;

    for &byte in input.as_bytes() {
        if byte == b'_' {
            continue;
        } else if byte == b'+' && !has_plus && !has_digits {
            has_plus = true;
        } else if byte.is_ascii_digit() {
            has_digits = true;
            let digit = (byte - b'0') as u64;
            count = match count.checked_mul(10).and_then(|c| c.checked_add(digit)) {
                Some(value) => value,
                None => return false,
            };
        } else {
            return false;
        }
    }

    has_digits
}

fn main() {
    let inputs = [
        "+10", "+_10", "_+_10", "++10", "10+", "_", "+", "10", "1_0",
        "_10", "10_", "1__0", "18446744073709551615",
        "18446744073709551616",
    ];

    for input in inputs {
        println!(
            "{input:?}: old={} current={} proposed={}",
            old(input),
            current(input),
            proposed(input)
        );
    }
}
RS

/tmp/rockql_take_compat_probe

Repository: Sayanthrock-Developer/ROCKQL

Length of output: 820

Preserve the existing + sign behavior.

The previous implementation accepted take +10 and take +_10. The current loop rejects b'+'. Allow one leading + before the first digit, after any skipped underscores. Add regression tests for +10 and +_10.

🤖 Prompt for 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.

In `@compiler/rockql-parser/src/lib.rs` around lines 221 - 238, The numeric
parsing loop in the take row-count handling must continue accepting a single
leading plus sign. Update the logic around the visible byte iteration so `+10`
and `+_10` are valid, while preserving underscore skipping, digit validation,
and rejection of plus signs elsewhere; add regression tests covering both forms.

Source: MCP tools

}

if !has_digits {
return Err(Diagnostic::new(
"`take` requires a non-negative integer row count",
span,
));
}

Ok(Transform::Take { count })
}
Expand Down
17 changes: 11 additions & 6 deletions compiler/rockql-sql/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,18 @@ impl FromStr for Dialect {
type Err = String;

fn from_str(value: &str) -> Result<Self, Self::Err> {
match value.to_ascii_lowercase().as_str() {
"generic" | "sql" => Ok(Self::Generic),
"sqlite" | "sqlite3" => Ok(Self::Sqlite),
"postgres" | "postgresql" => Ok(Self::Postgres),
_ => Err(format!(
// ⚡ Bolt Optimization: Use `eq_ignore_ascii_case` to avoid intermediate String allocation from `to_ascii_lowercase()`
if value.eq_ignore_ascii_case("generic") || value.eq_ignore_ascii_case("sql") {
Ok(Self::Generic)
} else if value.eq_ignore_ascii_case("sqlite") || value.eq_ignore_ascii_case("sqlite3") {
Ok(Self::Sqlite)
} else if value.eq_ignore_ascii_case("postgres") || value.eq_ignore_ascii_case("postgresql")
{
Ok(Self::Postgres)
} else {
Err(format!(
"unsupported SQL target `{value}`; expected generic, sqlite, or postgres"
)),
))
}
}
}
Expand Down
Loading