fix(function): bound group_concat result and clamp stream out columns#35408
fix(function): bound group_concat result and clamp stream out columns#35408bestdo77 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds bounds checking to prevent buffer overflow in group_concat (returning TSDB_CODE_PAR_VALUE_TOO_LONG if the limit is exceeded), registers a cleanup function to prevent memory leaks, and adds integration tests for overflow safety. The review feedback points out a potential null pointer dereference of pRes in the error handling block of gconcatFunction when code != TSDB_CODE_SUCCESS.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
ab4f331 to
03be346
Compare
Fixes #34504.
group_concat has two problems:
The result is accumulated into a fixed 65519-byte buffer with no bounds check (
gconcatHelperin builtinsimpl.c). Once a group's payload grows past that, the memcpy corrupts the heap and taosd aborts (malloc(): corrupted top size, signal 6). I reproduced this on current main by running group_concat over ~6k rows (~102 KB payload) — the server dies immediately.translateGroupConcatalways estimates the result width as 65515, so a stream output row containing any other column exceeds the 65531 limit andCREATE STREAMfails withRow length exceeds max length 65531 [0x8000263E]— the error from the issue.What this PR does:
gconcatHelpernow checks the length before every append and returns "Value too long for column/tag" when the payload would exceed the limit. The query fails, the server stays up. AcleanupFuncis registered for group_concat and all buffer frees are idempotent, so the error path releases the accumulation buffer instead of leaking it.createStreamReqSetDefaultOutCols). The stream in the issue can be created now. If a real value is still longer than the clamped column, the write is rejected with a normal error instead of crashing.Reproduced/tested with:
Added
test_group_concat_overflowtotest_agg_gconcat.pycovering both cases.One note for the reporter: the SQL in the issue has no window constraint (
ts >= _twstart and ts <= _twend), so each trigger scans the whole table — that is how the calc query behaves without it; the stream cases in this repo always include it.Tested on Debian 10 x86_64, community build 3.4.1.13.alpha based on main@df445db5:
Performance impact should be negligible: one integer comparison per appended value in group_concat, and the clamp runs once per stream creation only when the row is oversized. No other code paths change. The only behavior change is that an oversized group_concat now returns an error instead of crashing the server.