fix: honor text collation in min and max

[iamlinjunhong]

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #3344

What this PR does / why we need it:

fix: honor text collation in min and max

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[gouhongshen]

Codex automated review

No existing review, issue comment, or inline thread raised these defects. The implementation still produces incorrect collation results and loses or misrepresents binary text collation metadata.

P1 - Use utf8mb4_general_ci weights instead of UCA Loose (pkg/sql/colexec/aggexec/minmax2.go:483)

language.Und with collate.Loose implements UCA/CLDR equivalence, not MySQL's legacy utf8mb4_general_ci. For example, x/text treats ß and ss as equal, while MySQL documents that general_ci maps ß to one s and does not support expansions. Consequently, MAX over values inserted as ('ß'), ('ss') retains ß, whereas general_ci must return ss. The new tests cover case/accent/width only and miss this class of incorrect weights.

P1 - Do not advertise utf8mb4_bin VARCHAR columns as binary data (pkg/frontend/util.go:1603)

A VARCHAR COLLATE utf8mb4_bin now has typ.Charset == CharsetBinary, causing its result metadata to use collation ID 63. This field is a protocol collation ID; the repository's own table maps 46 to utf8mb4_bin and 63 to the distinct binary character set. MySQL also explicitly distinguishes nonbinary _bin text from binary strings. Clients can therefore expose ordinary UTF-8 VARCHAR results as raw binary or skip character conversion. Preserve the UTF-8 collation identity; reserve ID 63 for actual binary types.

P2 - Resolve table charset and collation independently of option order (pkg/sql/plan/build_ddl.go:1271)

The loop lets whichever table option appears last overwrite tableCharset. Thus CREATE TABLE t(v VARCHAR(10)) COLLATE utf8mb4_bin CHARACTER SET utf8mb4 ends with CharsetUTF8, even though both clauses specify the compatible utf8mb4_bin collation. MIN(v) then uses the case-insensitive comparator rather than binary ordering. Table options are parsed as an unordered list, and the contract when both are present is to use the explicit charset and explicit collation, not discard one according to textual order.

P1 - Persist the table collation for columns added later (pkg/sql/plan/build_ddl.go:1267)

tableCharset exists only while planning the initial CREATE and is copied only into columns present then; no table-level default is persisted. ALTER TABLE t ADD COLUMN v VARCHAR(10) consequently creates a general-ci column even when t was created with COLLATE utf8mb4_bin. The changed aggregate will silently use different ordering for initial and subsequently added columns in the same table. Column collation rules require a column without explicit clauses to inherit the table default.

[gouhongshen]

Codex automated review

My previous JSONL metadata and protocol-collation blockers are fixed by 85d6c9a. The earlier comparator, DDL/SHOW/LIKE, CAST/CONVERT, derived/conditional, and remote-version blockers are also addressed. Two new composition paths still change MIN/MAX results.

P1 - Top-level NULL in UNION downgrades text collation to legacy byte order (pkg/sql/plan/make.go:659)

MakePlan2NullTextConstExprWithType emits T_text with Charset omitted, so CharsetLegacy is restored. buildUnionWithResultLen excludes only T_any when resolving the common type, causing this pure NULL branch to participate in coalesce collation merging; mergedTextCharset then downgrades a normal CharsetUTF8 branch to CharsetLegacy. The pure-null bookkeeping is used only for ENUM/SET provenance, not this coercion. For SELECT MIN(x) FROM (SELECT c AS x FROM t UNION ALL SELECT NULL AS x) s, with c values 'a' and 'B', the result becomes bytewise 'B' instead of general-ci 'a'. Make pure NULL neutral during set-operation type merging and add a regression. This is distinct from the earlier CASE/COALESCE/IF findings, whose nested NULLs remain T_any.

P1 - GROUP_CONCAT drops text-shaped binary and _bin collation metadata (pkg/sql/colexec/aggexec/concat2.go:112)

GroupConcatReturnType detects binary inputs only by OID. Text-shaped values carrying CharsetBinary, CharsetUTF8MB4Bin, or CharsetLegacy—including CONVERT(c USING binary), whose direct return metadata is now correct—fall through to T_text.ToType(), which is CharsetUTF8. The executor creates its output vector with this retType, so an outer MIN/MAX uses the general-ci comparator. For example, grouping binary-converted values 'a' and 'B' and then selecting MIN over the GROUP_CONCAT results returns 'a' instead of bytewise 'B'. Derive the aggregate result charset from text arguments, including text-shaped binary values, and cover the nested aggregate path.

[XuPeng-SH]

Reviewed exact head 99ee47a. The direct top-level NULL cases are fixed, but pure-NULL provenance is still lost through a derived table/CTE. setBranchPureNull inspects only the current branch project and isPureNullLiteralExpr accepts only a literal; after projection, SELECT NULL AS x is represented by a ColRef and participates in UNION collation merging as legacy T_text.

A reachable counterexample is:

SELECT MIN(x)
FROM (
SELECT c AS x FROM t
UNION ALL
SELECT x FROM (SELECT NULL AS x) n
) u;

If c uses a case-insensitive collation and contains a/B, the derived NULL branch can downgrade the merged comparison to bytewise legacy semantics and change MIN from a to B. MySQL assigns coercibility 6 to NULL and expressions derived from NULL, so this provenance must survive derived outputs/CTEs rather than relying on syntactic literal detection: https://dev.mysql.com/doc/refman/9.2/en/charset-collation-coercibility.html

Please carry pure-NULL provenance through derived output resolution and add nested-derived/CTE UNION counterexamples.

[XuPeng-SH]

Request changes on exact head f1ce261.

The latest commit closes the previous nested-derived/CTE pure-NULL blocker by carrying that identity through OutputColumnProvenance. However, the same end-to-end collation invariant is still broken for other planner-generated text schemas.

pkg/sql/plan/unnest.go and pkg/sql/plan/current_account.go construct ordinary VARCHAR outputs with plan.Type{Id: ...} and no Charset. Every new query therefore marks these runtime-produced values as CharsetLegacy (0), and MIN/MAX dispatches them to bytes.Compare instead of the new default utf8mb4_general_ci comparator. For example:

SELECT MIN(`key`) FROM unnest('{"a":1,"B":2}') u;

The generated key column is ordinary text, but the plan uses legacy byte order and selects B rather than the general-ci result a.

I reproduced this on the exact head with planner counterexamples for unnest.key, unnest.path, and current_account.account_name: each aggregate argument expected CharsetUTF8 (3) but got 0. This is the same shape as the parse_jsonl gap already fixed in this PR via makeSimplePlan2Type, and a static inventory shows more manually authored VARCHAR/TEXT plan.Type schemas (for example stage_list).

Please centralize explicit charset initialization for planner-generated string columns and audit these manual schemas. Ordinary text should receive an explicit supported text collation; intentionally opaque/binary outputs should be marked explicitly as such. Preserve zero only for genuinely old catalog/plan metadata, and add table-function-to-MIN/MAX regressions so this boundary stays covered.

[aunjgr]

Reviewed exact head f1ce261. The collation invariant is still broken for planner-generated string schemas.

pkg/sql/plan/unnest.go creates key/path VARCHAR outputs with no Charset, and pkg/sql/plan/current_account.go does the same for account_name, user_name, and role_name. They therefore become CharsetLegacy and MIN/MAX uses bytewise comparison instead of the new utf8mb4_general_ci default. For example, MIN(key) over unnest JSON keys a and B can return B instead of a.

This is the same boundary already fixed for parse_jsonl. Centralize explicit charset initialization for planner-generated VARCHAR/TEXT columns, audit the remaining manual plan.Type schemas (for example stage_list), and add table-function-to-MIN/MAX regressions. Reserve zero only for genuinely legacy catalog/plan metadata.

[gouhongshen]

Codex automated review

The new planner-schema pass fixes the reviewer-reported zero-charset outputs for UNNEST, current_account, and stage_list. My prior pure-NULL UNION/derived-boundary and GROUP_CONCAT blockers remain fixed. The mixed-OID LEAST/GREATEST collation-loss blocker remains unresolved.

P1 - Preserve collation for mixed TEXT/VARCHAR LEAST/GREATEST (pkg/sql/plan/function/func_builtin_leastgreatest.go:546)

This resolver selects a fresh T_text.ToType() solely because one operand is TEXT, then leastGreatestCastTypes casts every operand to that target. The PR now gives that fresh type explicit CharsetUTF8 (general-ci), but this path never calls MergeStringCharset. Thus, with v VARCHAR ... COLLATE utf8mb4_bin and t TEXT ... COLLATE utf8mb4_bin, MIN(LEAST(v,t)) over rows ('a','a') and ('B','B') is planned general-ci and returns a; binary collation requires B. Derive the mixed string target charset from all string operands (using the existing merge helper) before applying casts and add a mixed-OID regression test.

[gouhongshen]

Codex automated review

The previous mixed TEXT/VARCHAR LEAST/GREATEST blocker is fixed by f308 via charset merging and regression coverage. Other previously reported blockers are addressed or withdrawn in later updates and are not repeated. A distinct JSON mixed-type path still drops text collation; focused Go tests were blocked by missing CGo headers, while diff/gofmt checks were clean.

P1 - Preserve collation in JSON mixed LEAST/GREATEST targets (pkg/sql/plan/function/func_builtin_leastgreatest.go:518)

When any JSON argument is present, resolveLeastGreatestType exits through leastGreatestJSONMixedType (lines 168-177), bypassing the ordinary mixed-string branch fixed by f308. For VARCHAR ... COLLATE utf8mb4_bin plus JSON, this line returns a fresh T_varchar.ToType(), now CharsetUTF8; leastGreatestCastTypes copies that target to both arguments. An outer MIN/MAX therefore selects compareUTF8mb4GeneralCI instead of binary ordering. For example, with v='a'/'B' and JSON values '"a"'/'"B"', SELECT MIN(LEAST(v,j)) returns 'a' under the generated metadata, while the binary collation requires 'B'. This is distinct from the fixed mixed TEXT/VARCHAR path because JSON dispatch exits earlier. Merge charset from the string inputs in this path (and the analogous JSON/date target at line 453) and add an outer MIN/MAX regression.

[gouhongshen]

Codex automated review

All previously blocking topics are resolved or withdrawn. The prior JSON mixed LEAST/GREATEST collation blocker is fixed for both JSON-mixed and JSON/date paths with regression coverage; comparator semantics, metadata/protocol compatibility, persistence/round-trips, expression/TVF/planner propagation, and mixed-version handling remain addressed. UTF-8 alias normalization is an intentional documented contract, and unsupported collations are rejected. No current findings.

[XuPeng-SH]

Approved after rechecking the exact head and the full review closure.

The previous blockers are addressed across the complete metadata/data path: collation identity is preserved for planner-generated and derived string schemas, mixed string OIDs, conditional/UNION/GROUP_CONCAT paths, JSON LEAST/GREATEST, persisted schema, SHOW/LIKE/ALTER behavior, and remote aggregate protocol compatibility. Internal opaque metadata/index-plugin string columns remain binary, so user collation semantics do not leak into identifiers or checksums.

Focused tests passed for planner propagation, JSON/mixed LEAST/GREATEST, NULL materialization, temporal scale restoration, general-CI MIN/MAX ordering/merge, malformed UTF-8 transitivity, and generated table-function schemas. I found no remaining correctness, hot-path allocation, compatibility, or unhappy-path blocker at this head.

[aunjgr]

Reviewed exact head 846e03f460639cc867232b69b655b48bbd515430.

The force-push is patch-equivalent to the previously approved series except for the clean rebase integration with #26469/#26753. The index-plugin boundary checks are clean, all focused changed-package tests pass, and live CI is green. One uncovered collation contract still changes MIN/MAX results: the supported utf8/utf8mb4 _bin collations are PAD SPACE, but this head compares them as raw NO PAD bytes.

P1 — Preserve PAD SPACE semantics for utf8/utf8mb4 _bin MIN/MAX

pkg/sql/colexec/aggexec/minmax2.go:377 routes CharsetUTF8MB4Bin to raw bytes.Compare, but the supported utf8_bin/utf8mb4_bin collations (protocol IDs 83/46) are PAD SPACE collations; only the distinct binary character set is NO PAD.

This reverses ordering, rather than merely choosing a different representative among equal values. After PAD SPACE normalization, "a " compares as "a" and is less than the valid string "a\0"; raw byte comparison reports the opposite because space (0x20) is greater than NUL. I reproduced this at the selected aggregate comparator on the exact head: it returned +1, while the required comparison is < 0. Consequently, MIN(v) over those two VARCHAR values can return a\0 instead of a .

Add a separate _bin comparator that removes trailing U+0020 before byte comparison, while keeping CharsetBinary and legacy metadata on the raw comparator, and cover this counterexample. MySQL documents the distinction here: https://dev.mysql.com/doc/refman/8.0/en/charset-binary-collations.html

P2 — Advertise the actual PAD SPACE contract for _bin

pkg/frontend/collation.go:33-35 says utf8_bin and utf8mb4_bin are NO PAD, but those legacy _bin collations are PAD SPACE. utf8mb4_0900_bin is the separate NO PAD collation, and this PR correctly rejects that unsupported identity.

SHOW COLLATION therefore misreports the semantics promised by DDL and protocol IDs 83/46. Change both rows to PAD SPACE and make TestAdvertisedCollationsAreExecutable assert each advertised pad attribute so the metadata cannot drift from the aggregate comparator again.