remove the 1024bytes limit for group_concat result.

[m-schen]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

https://github.com/matrixorigin/MO-Cloud/issues/4084

What this PR does / why we need it:

移除了group_concat每行结果不能超过1024的限制

---

PR Type

Bug fix

---

Description

• Removed the 1024-byte length limit for each row content in the group_concat function, allowing for longer concatenated strings.
• Simplified the code by removing unnecessary checks related to the previous length limit.

---

Changes walkthrough 📝

	Relevant files
Bug fix	concat.go Remove length limit for group_concat results                          pkg/sql/colexec/aggexec/concat.go Removed the 1024-byte length limit for group_concat results. Simplified logic by eliminating checks against the removed limit. +1/-8

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Potential Memory Issue Removing the 1024-byte limit for group_concat results could lead to excessive memory usage if large strings are concatenated.

[qodo-merge-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Add error handling for potential memory allocation failures during string concatenation Consider adding error handling for the case when appending the separator or the new value causes the slice to grow beyond its capacity, which could lead to memory allocation failures. pkg/sql/colexec/aggexec/concat.go [115-118] if len(r) > 0 { - r = append(r, exec.separator...) + r, err := safeAppend(r, exec.separator) + if err != nil { + return err + } } -r = append(r, v...) +r, err := safeAppend(r, v) +if err != nil { + return err +} exec.ret.aggSet(r) Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion improves robustness by adding error handling for potential memory allocation failures during string concatenation, which is a valuable enhancement for ensuring stability and reliability of the code.	8
Possible issue	Add a check for maximum allowed length of the concatenated string to prevent potential memory issues Consider adding a check for the maximum allowed length of the concatenated string to prevent potential memory issues. You could use a configurable maximum length or a system-defined limit. pkg/sql/colexec/aggexec/concat.go [104-118] func (exec *groupConcatExec) Fill(groupIndex int, row int, vectors []*vector.Vector) error { ... r := exec.ret.aggGet() + if len(r) > exec.maxLength { + return fmt.Errorf("group_concat result exceeds maximum allowed length") + } if len(r) > 0 { r = append(r, exec.separator...) } Suggestion importance[1-10]: 7 Why: The suggestion to add a configurable maximum length check is reasonable for preventing potential memory issues, especially since the previous hardcoded limit was removed. However, the suggestion does not address how this maximum length should be configured or enforced, which is a crucial part of the implementation.	7
	Add a length check in the merge function to prevent exceeding a maximum allowed length Consider adding a check in the merge function to ensure that the combined length of v1 and v2 doesn't exceed a maximum allowed length, similar to the suggestion for the Fill function. pkg/sql/colexec/aggexec/concat.go [166-172] v1 := exec.ret.aggGet() v2 := other.ret.aggGet() if len(v2) == 0 { return nil } +if len(v1)+len(v2)+len(exec.separator) > exec.maxLength { + return fmt.Errorf("merged group_concat result exceeds maximum allowed length") +} if len(v1) > 0 && len(v2) > 0 { v1 = append(v1, exec.separator...) } Apply this suggestion Suggestion importance[1-10]: 7 Why: Similar to the first suggestion, this adds a safeguard against excessive memory usage by checking the combined length of concatenated strings. It is a good practice, but the implementation details regarding the maximum length configuration are not provided, which limits its completeness.	7