GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||
GeezerVison is showing github.com — [3.15] gh-109638: Fix excessive backtracking in csv.Sniffer doublequote detection by pikammmmm · Pull Request #154869 · python/cpython Conversation
|
| iterations | before | after | speedup |
|---|---|---|---|
| 40 | 1.11 s | 0.042 s | 27x |
| 60 | 9.70 s | 0.257 s | 38x |
| 80 | 43.85 s | 0.83 s | 53x |
Growth is still superlinear, as noted in the issue — this narrows the search
rather than replacing the approach — but it is far slower.
test_sniff_regex_backtracking mirrors the test added on main, with one
difference: it uses 60 iterations rather than 100. main can afford 100 because
the rewrite made the cost negligible, whereas with this targeted fix 100
iterations still takes about 1.7 s, which is too slow to put in the suite. 60 is
the size quoted in the issue and costs 0.23 s.
Whole-file test_csv.py on 3.15.0b1: 0.45 s with the fix, 11.6 s
without it, 135 passing either way — so the new test acts as the same kind of
canary it does on main rather than asserting on a timer.
Beyond the suite, I compared sniff() results between patched and unpatched
csv.py across 8726 cases — the delimiter/quote/doublequote/skipinitialspace
combinations, TestSniffer's own corpus, and 4000 randomized quote-dense
strings — with no divergence in the returned dialect or the exceptions raised.
Disclosure: prepared with AI assistance (Claude Code) — used to analyse the
backtracking behaviour, produce the measurements and differential check above,
and draft the patch and this description.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…e detection The doublequote-detection pattern used two greedy character classes that both admitted the quote character, so the engine had to try every way of splitting a run of quotes between them. Excluding the quote from the first class forces the quote that follows it to match at the first one available, which removes the ambiguity while leaving detection intact. The second class must keep admitting the quote, since that is what lets `"a""b"` match -- narrowing both classes, or making the first one atomic while it still admits the quote, silently disables the detection.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Closing this in favour of #154868, which solves the same problem more thoroughly: the possessive-quantifier rewrite also detects a doubled quote inside a field that contains the delimiter or a line break, which this patch does not. I hadn't spotted #154865 or #154868 when I opened this — sorry for the duplicate noise. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.