Skip to content

fix(parser): refine Python parser depth accounting#7813

Open
peterbud wants to merge 1 commit into
sqlfluff:mainfrom
peterbud:7805
Open

fix(parser): refine Python parser depth accounting#7813
peterbud wants to merge 1 commit into
sqlfluff:mainfrom
peterbud:7805

Conversation

@peterbud
Copy link
Copy Markdown
Contributor

@peterbud peterbud commented May 2, 2026 •
edited by cubic-dev-ai Bot
Loading

Brief summary of the change made

Fix a false positive max_parse_depth failure in the Python parser by excluding helper-only match wrappers from depth accounting.

Fixes #7805

What changed

  • Added track_parse_depth to ParseContext.deeper_match()
  • Stopped charging depth for helper wrappers like Ref, Sequence, AnyNumberOf, Delimited, and greedy terminator matching
  • Added test fixtures for the new behavior and the reported T-SQL case

Note: Python parser only. Rust parser unchanged.

Are there any other side effects of this change that we should be aware of?

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with tox -e generate-fixture-yml).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.

  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

Summary by cubic

Fixes false positive max_parse_depth errors in the Python parser by not counting helper-only match wrappers toward depth. Prevents unexpected depth-limit failures (e.g., in T‑SQL). Rust parser remains unchanged.

  • Bug Fixes
    • Added track_parse_depth to ParseContext.deeper_match() to control depth accounting.
    • Stopped charging depth for helper wrappers (refs, sequences, delimited groups) and greedy terminator matching.
    • Added tests for the new behavior, including the reported T‑SQL case.

Written for commit 0b67b7d. Summary will update on new commits.

cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed May 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 7 files

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 2, 2026

Coverage Results ✅

Name    Stmts   Miss  Cover   Missing
-------------------------------------
TOTAL   21667      0   100%

263 files skipped due to complete coverage.

@alanmcruickshank
Copy link
Copy Markdown
Member

alanmcruickshank commented May 8, 2026

@copilot can you compare and contrast this with how depth accounting is handled in rust?

@peterbud
Copy link
Copy Markdown
Contributor Author

peterbud commented May 17, 2026

@copilot can you compare and contrast this with how depth accounting is handled in rust?

Hi @alanmcruickshank , let me try to contrast it

In Python, the PR changes context.py:219-265 so a deeper-match scope can opt out of consuming parse-depth budget. That matters because the Python parser uses one shared mutable counter, and before this PR it charged that counter for helper-only wrappers as well as real descent. The new call sites in base.py:416-427, anyof.py:140-196, delimited.py:132-150, and sequence.py:236-558 explicitly mark wrappers like Ref, Sequence, Delimited, bracket helpers, and greedy terminator checks as non-depth-consuming. Real descent still goes through normal counted scopes such as segment matching in base.py:696.

Rust is different in two important ways. First, it enforces max depth from the explicit frame stack inside the iterative parser, not from a shared context-manager counter: see iterative.rs:58-124. Real recursive descent is represented by child frames pushed onto that stack. Second, some helper probes in Rust do not add onto the current frame depth at all because they launch a separate subparse with a fresh local stack, such as exclude checks.

So the practical contrast is this: old Python overcounted because every wrapper scope incremented one shared depth counter; this PR fixes that selectively. Rust already avoids that exact problem because depth is tied to active parse frames, not logging or dispatch scopes. But Rust is not identical to the post-PR Python behavior either, because some helper work is isolated into separate subparses, which means Rust can undercount relative to a fully cumulative notion of nested helper activity. I would not expect the exact false-positive fixed by this PR to reproduce in Rust. The Rust side already passes the same configured limit through from rust_parser.py:90-102, but it applies that limit at the frame-stack level rather than at every helper wrapper.

Does that help?

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

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tsql: Maximum parse depth exceeded (limit 255) triggers on modest nested function calls

2 participants

@peterbud @alanmcruickshank