Skip to content

[3.15] gh-109638: Avoid pathological backtracking in csv.Sniffer (GH-153694) - #154865

Closed
Punisheroot wants to merge 1 commit into
python:3.15from
Punisheroot:perf/js/avoid-csv-sniffer-backtracking
Closed

[3.15] gh-109638: Avoid pathological backtracking in csv.Sniffer (GH-153694)#154865
Punisheroot wants to merge 1 commit into
python:3.15from
Punisheroot:perf/js/avoid-csv-sniffer-backtracking

Conversation

@Punisheroot

@Punisheroot Punisheroot commented Jul 29, 2026

Copy link
Copy Markdown

Summary

Avoid pathological backtracking in the regular expression used by
csv.Sniffer to detect doubled quote characters.

This is a narrow maintenance-branch replacement for the gh-109638 portion of
GH-153694. The full rewrite cannot be backported because it changes behavior
on valid inputs, while GH-153694 explicitly identifies a targeted regex fix as
the appropriate approach for maintenance branches.

Root cause

The first negated character class in the doublequote pattern excluded the
delimiter and newline, but not the detected quote character. On inputs with
long runs of quotes, the regex engine could repeatedly repartition those
quotes between the character class and the following quote tokens.

The change excludes the quote character from that first character class. This
has the same intent as the lookahead originally proposed in gh-109638, without
introducing another capture group. It deliberately does not use the atomic
group from GH-109639: that version consumed the quote characters needed by the
rest of the pattern and could silently disable doublequote detection.

The regression test is the pathological sample already used by the rewritten
3.16 implementation. Existing positive and negative doublequote tests remain
unchanged and pass.

Performance

Measured with a Windows x64 release build of the current 3.15 branch and the
60-iteration reproducer from gh-109638:

Version Time
Before 7.592759 s
After, median of 5 runs 0.187145 s

This is approximately a 40.6x speedup. Both versions detected , as the
delimiter and left doublequote false for this sample.

Validation

  • Windows x64 Debug build: succeeded with no warnings or errors
  • Windows x64 Release build: succeeded with no warnings or errors
  • Regression test: passed
  • TestSniffer: 14 tests passed
  • Full test_csv, Debug: 135 tests passed
  • Full test_csv, Release: 135 tests passed, 4 skipped
  • Tools/patchcheck/patchcheck.py: passed and recognized the NEWS entry
  • git diff --check: passed

@Punisheroot Punisheroot changed the title gh-109638: Avoid pathological backtracking in csv.Sniffer [3.15] gh-109638: Avoid pathological backtracking in csv.Sniffer (GH-153694) Jul 29, 2026
@Punisheroot
Punisheroot marked this pull request as ready for review July 29, 2026 10:58
@serhiy-storchaka

Copy link
Copy Markdown
Member

Excluding the quote character from the first run lowers the degree, but the second run can still take each quote either as its own content or as one of the matched quotes, so the search stays polynomial.

Measured on the reproducer from the issue:

n sample 3.15 this PR #154868
50 252 B 2.32 s 0.07 s 0.0001 s
100 502 B 76.28 s 1.05 s 0.0001 s
200 1002 B -- 17.04 s 0.0001 s
400 2002 B -- 266.90 s 0.0001 s

O(n^5) before, O(n^4) after: a 2 KB sample still takes four and a half minutes, and the test added here takes 1.05 s. #154869 proposed the same change and was closed in favour of #154868, reporting the same limitation.

Excluding the quote character from both runs and matching them possessively makes it linear, at the cost of changing results on 42 of 560 files in the CSVsniffer corpora, where this PR changes none.

3.15 is still in beta, so there is also a third option: backporting the new engine from main, which fixes this issue along with seven others.

@Punisheroot

Copy link
Copy Markdown
Author

Thanks for the analysis. I implemented the quote-exclusion approach suggested in the July 21 issue comment, but I have now reproduced the residual scaling locally on the current 3.15 branch: n=50 took 0.122 s, n=100 1.598 s, and n=150 7.856 s.
I agree that this reduces but does not eliminate the pathological behavior, and that #154868 is the stronger fix. I’m happy to close this in its favor. If useful, I can contribute the Windows validation or help with follow-up work for the maintenance branches.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants