Skip to content

feat: add parser validation for fixed SQL in rule fixtures#2121

Open
benfdking wants to merge 1 commit intomainfrom
claude/add-sqruff-parser-validation-TnXdM
Open

feat: add parser validation for fixed SQL in rule fixtures#2121
benfdking wants to merge 1 commit intomainfrom
claude/add-sqruff-parser-validation-TnXdM

Conversation

@benfdking
Copy link
Collaborator

Add validation that all fix_str values in rule fixtures can be parsed
without errors. This ensures that rule fixes produce valid SQL that the
parser can process.

The validation:

  • Parses the fix_str after a fix is applied
  • Checks for any Unparsable segments in the parse tree
  • Fails the test if unparsable sections are found

Skips validation for cases with Jinja template syntax ({{ or {%)
when the jinja templater is not configured, as these would be
unparsable anyway without template expansion.

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Benchmark for ae4bac3

Click to view benchmark
Test Base PR %
DepthMap::from_parent 53.8±0.50µs 54.0±1.04µs +0.37%
fix_complex_query 11.9±0.05ms 11.9±0.22ms 0.00%
fix_superlong 173.6±29.49ms 169.1±25.82ms -2.59%
parse_complex_query 4.2±0.05µs 4.2±0.07µs 0.00%
parse_expression_recursion 7.2±0.07µs 7.1±0.09µs -1.39%
parse_simple_query 1059.7±19.91ns 1073.8±16.61ns +1.33%

Add validation that all fix_str values in rule fixtures can be parsed
without errors. This ensures that rule fixes produce valid SQL that the
parser can process.

The validation:
- Parses the fix_str after a fix is applied
- Checks for any Unparsable segments in the parse tree
- Fails the test if unparsable sections are found

Skips validation for cases with Jinja template syntax ({{ or {%)
when the jinja templater is not configured, as these would be
unparsable anyway without template expansion.
@benfdking benfdking force-pushed the claude/add-sqruff-parser-validation-TnXdM branch from e5685c2 to 38ffd57 Compare January 8, 2026 07:56
@github-actions
Copy link

github-actions bot commented Jan 8, 2026

Benchmark for 2cbde0d

Click to view benchmark
Test Base PR %
DepthMap::from_parent 54.2±0.79µs 54.3±0.84µs +0.18%
fix_complex_query 12.2±0.09ms 11.9±0.09ms -2.46%
fix_superlong 150.3±19.99ms 147.3±18.31ms -2.00%
parse_complex_query 4.1±0.04µs 4.1±0.05µs 0.00%
parse_expression_recursion 7.3±0.10µs 7.4±0.06µs +1.37%
parse_simple_query 1075.2±14.52ns 1074.1±16.20ns -0.10%

@openhands-ai
Copy link

openhands-ai bot commented Jan 8, 2026

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • PR Checks

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #2121 at branch `claude/add-sqruff-parser-validation-TnXdM`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants