Repository navigation
Don't copy the type AST at every LowCardinality level - #588
Merged
Merged
Conversation
CreateColumnFromAst() binds the LowCardinality child as "const auto nested = GetASTChildElement(ast, 0)", which copies the whole subtree. With nested LowCardinality(Nullable(...)) levels every level makes another copy while the recursion still holds the earlier ones, so peak memory grows with depth times subtree width: 200 levels over an 8000-element Tuple took about 220 MiB before the type was rejected. Bind the child by reference instead.
slabko
approved these changes
Oct 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CreateColumnFromAstincolumns/factory.cppbinds the LowCardinality child withconst auto nested = GetASTChildElement(ast, 0);, which deep-copies the whole subtree at every level. All the copies stay alive down the recursion, so memory grows with depth × width. A ~110 KB type string,LowCardinality(Nullable(nested about 250 times over aTupleof a few thousandUInt8, makes the client allocate hundreds of megabytes before it reports the type as unsupported. A server can send such a type in a block header.Fix: bind the child by const reference. It was the only by-value copy of the AST on a recursive path in
factory.cpp.Test:
CreateColumnByType.NestedLowCardinalityDoesNotCopyAstbuilds 200 levels overTuple(UInt8 × 8000)and checks that peak RSS grows by less than 16 MiB. On master it grows by about 220 MiB, in the full test binary too; with the fix, about 5 MiB. It reads/proc/self/status, so it runs on Linux only and skips elsewhere. The include reorder in that test file comes from clang-format's include regrouping.