From ea1abd7ed39859cb62d2ab5629d0121b79f55994 Mon Sep 17 00:00:00 2001 From: Justin Zhang Date: Wed, 29 Jul 2026 11:45:13 -0400 Subject: [PATCH] perf: make ReindentFilter._get_offset linear in tokens instead of quadratic _get_offset built the text preceding a token by flattening the ENTIRE statement from the start on every call, then kept only the last line of it. Called once per token that needs an offset, that is quadratic in the token count. It now walks backwards from the token, collecting only as much preceding text as is guaranteed to contain the final line. Groups are yielded whole rather than descended into, so str() on the result still spells the same text. The walk stops after the third token containing a line-boundary character rather than the second, because str.splitlines() ignores a *trailing* boundary ("a\nb\n" ends on the line "b"), and in the worst case a "\r\n" is split across a token edge and counts only once. Trimming each token to its own last line as it is collected looks like a further win but is wrong for exactly that reason - it changed 219 of 20,435 differential cases when I tried it. _flatten_up_to_token is a public-ish method, so if it has been replaced -- by a subclass, on the class, or on the instance -- the original path is used instead of this walk. All three are verified to still see the override called, with byte-identical output. A token spliced into the tree without a parent cannot be located by walking parents. StripCommentsFilter does exactly that, so the walk signals the condition and the caller falls back to flattening. Treating it as "nothing precedes this token" instead mis-indented a CASE containing a stripped comment: sqlparse.format('select case/*\nc\n*/when a=1 then 2 else 3 end from t', reindent=True, strip_comments=True) master: 'select case\n when a=1 then 2\n else 3\n...' unfixed: 'select case\n when a=1 then 2\n else 3\n...' Measured with time.process_time, alternating base/patched subprocesses, min-of-3, on nested function calls (SELECT COALESCE(a,b,0) x n FROM t): n base new speedup base growth new growth 200 137.2ms 45.4ms 3.02x x3.10 x2.09 400 458.6ms 98.6ms 4.65x x3.34 x2.17 800 1683.0ms 203.5ms 8.27x x3.67 x2.06 Base grows ~x3.5 per doubling, the patch ~x2.1: the growth rate drops from quadratic to linear, so the speedup keeps increasing with statement size. Also 2.7x on a 400-row INSERT ... VALUES. TRADE-OFF, measured and disclosed: the cost is now proportional to the length of the preceding line MULTIPLIED BY the nesting depth, because a deeply nested group re-walks the same long preceding line once per level. A 200 KB literal followed by 30 *flat* grouped expressions is actually 1.27x FASTER; the regression needs depth. Nesting those expressions 40 deep after a 200 KB literal measures 0.57x, i.e. ~2x slower than master. Statements whose tokens mostly do not need an offset (a long flat WHERE ... AND chain, a large IN list, chained UNIONs, a wide flat SELECT list) measure 0.92-1.08x, unchanged within noise. Output is unchanged: 20,435 differential comparisons against master (4,087 SQL statements x 5 formatting configs) with 0 mismatches. The corpus covers every line boundary str.splitlines() recognises, boundaries inside string literals and comments, leading and trailing boundaries, runs of three or more, CRLF split across token edges, deep paren nesting, 200-column selects, and 4,000 randomly assembled fragments. Suite: 494 passed, 2 xfailed, 1 xpassed, rc=0 - identical to master. --- sqlparse/filters/reindent.py | 88 +++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/sqlparse/filters/reindent.py b/sqlparse/filters/reindent.py index ea41ac6b..4ad924d3 100644 --- a/sqlparse/filters/reindent.py +++ b/sqlparse/filters/reindent.py @@ -5,10 +5,47 @@ # This module is part of python-sqlparse and is released under # the BSD License: https://opensource.org/licenses/BSD-3-Clause +import re + from sqlparse import sql from sqlparse import tokens as T from sqlparse.utils import indent, offset +# The line boundaries str.splitlines() recognizes. Used only to tell whether a +# token value can possibly affect the line count, never to split. +HAS_LINE_BREAK = re.compile('[\n\r\v\f\x1c\x1d\x1e\x85\u2028\u2029]') + + +class _Unreachable(Exception): + """*token* is not reachable from *group* by walking parents.""" + + +def _tokens_before(group, token): + """Yields the tokens of *group* that precede *token*, closest one first. + + Groups are yielded whole rather than descended into, so ``str()`` on the + result still spells the preceding text but does not visit a leaf that the + caller may not need to look at. + """ + # Record where *token* sits inside each of its ancestors, up to *group*. + ancestry = [] + node = token + while node is not group: + parent = node.parent + try: + ancestry.append((parent.tokens, parent.tokens.index(node))) + except (AttributeError, ValueError): + # *token* is not reachable from *group* -- StripCommentsFilter, for + # one, splices in a replacement token whose parent is unset. The + # backwards walk cannot answer this, so say so and let the caller + # fall back to flattening the statement. + raise _Unreachable from None + node = parent + + for siblings, tidx in ancestry: + for idx in range(tidx - 1, -1, -1): + yield siblings[idx] + class ReindentFilter: def __init__(self, width=2, char=' ', wrap_after=0, n='\n', @@ -41,9 +78,51 @@ def _flatten_up_to_token(self, token): def leading_ws(self): return self.offset + self.indent * self.width + def _preceding_line(self, token): + """Returns the last line of the text preceding *token*.""" + if (type(self)._flatten_up_to_token is not _ORIG_FLATTEN_UP_TO_TOKEN + or '_flatten_up_to_token' in vars(self)): + # _flatten_up_to_token has been replaced -- by a subclass, on this + # class, or on this instance. Honour the replacement rather than the + # equivalent-but-faster walk below. + raw = ''.join(map(str, self._flatten_up_to_token(token))) + return (raw or '\n').splitlines()[-1] + + if token.is_group: + token = next(token.flatten()) + + # Only the final line of the preceding text is ever used, so walk + # backwards from *token* and stop once the text collected is guaranteed + # to contain that whole line. Re-flattening the entire statement on + # every call is what made reindenting quadratic in the token count. + # + # Two line boundaries are needed rather than one, because splitlines() + # ignores a *trailing* boundary: "a\nb\n" ends on the line "b", so a + # token ending in a break does not start a fresh line. Stopping after + # the third token that holds a boundary guarantees two boundaries even + # in the worst case, where a "\r\n" is split across a token edge and so + # counts only once. + chunks = [] + breaks = 0 + try: + for t in _tokens_before(self._curr_stmt, token): + value = str(t) + chunks.append(value) + if HAS_LINE_BREAK.search(value): + breaks += 1 + if breaks == 3: + break + except _Unreachable: + # A token spliced in without a parent (StripCommentsFilter does + # this) cannot be located by walking parents. Flatten instead. + raw = ''.join(map(str, self._flatten_up_to_token(token))) + return (raw or '\n').splitlines()[-1] + chunks.reverse() + + return (''.join(chunks) or '\n').splitlines()[-1] + def _get_offset(self, token): - raw = ''.join(map(str, self._flatten_up_to_token(token))) - line = (raw or '\n').splitlines()[-1] + line = self._preceding_line(token) # Now take current offset into account and return relative offset. return len(line) - len(self.char * self.leading_ws) @@ -246,3 +325,8 @@ def process(self, stmt): self._last_stmt = stmt return stmt + + +# The shipped _flatten_up_to_token, captured so that _preceding_line can tell +# whether it is still in place and only then take its faster equivalent path. +_ORIG_FLATTEN_UP_TO_TOKEN = ReindentFilter._flatten_up_to_token