Skip to content

fix(parser): stop column name leaking into CAST(... AS UNSIGNED/SIGNED) type (#25131) - #25169

Merged
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
fengttt:fix/groupby-cast-25131
Jun 26, 2026
Merged

fix(parser): stop column name leaking into CAST(... AS UNSIGNED/SIGNED) type (#25131)#25169
mergify[bot] merged 2 commits into
matrixorigin:mainfrom
fengttt:fix/groupby-cast-25131

Conversation

@fengttt

@fengttt fengttt commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • BUG

Which issue(s) this PR fixes:

issue #25131

What this PR does / why we need it:

GROUP BY CAST(col AS UNSIGNED) with the same CAST in SELECT and
ORDER BY failed with:

SQL syntax error: column "t.col" must appear in the GROUP BY clause or be used in an aggregate function

even though the SELECT, GROUP BY, and ORDER BY expressions are the
identical CAST(col AS UNSIGNED).

Root cause

In the mysql grammar, integer_opt (the optional INTEGER/INT keyword
after SIGNED/UNSIGNED in a cast type) had an empty action {} that never
assigned $$. goyacc leaves stale parser-stack memory in an unassigned $$,
and for CAST(col AS UNSIGNED) that stale <str> value was the preceding
column name
. The UNSIGNED integer_opt rule uses it as the type's
FamilyString, so the cast rendered as e.g.:

cast(zsd04.vgpos as vgpos unsigned)

ORDER BY qualifies its column references (qualifyColumnNames) and keys the
grouped-expression lookup on tree.String of the cast. The leaked column name
made that key differ from the GROUP BY key, so the match failed and the bare
column was reported as not grouped. (SELECT binds the cast directly, so the
corruption only surfaced once ORDER BY was present.)

Fix

Assign $$ = "" in the empty integer_opt action so the cast target type is
rendered deterministically (as unsigned / as signed). This also fixes the
same latent corruption for CAST(... AS SIGNED). Regenerated mysql_sql.go
(0 shift/reduce conflicts; regeneration is deterministic).

Tests

  • Parser round-trip regression cases in mysql_sql_test.go (TestValid):
    cast(col as unsigned) / cast(col as signed) in GROUP BY/ORDER BY render
    stably.
  • New BVT case dml/select/group_by_cast covering the issue's two
    reproductions plus SIGNED, UNSIGNED INTEGER, DESC ordering, and
    integer-column variants.
  • Regression: dml/select (1047) and function/builtin (247) pass.

🤖 Generated with Claude Code

…D) type (matrixorigin#25131)

GROUP BY CAST(col AS UNSIGNED) with the same CAST in SELECT and ORDER BY
failed with "column must appear in the GROUP BY clause or be used in an
aggregate function".

Root cause: in the mysql grammar, `integer_opt` (the optional INTEGER/INT
keyword after SIGNED/UNSIGNED in a cast type) had an empty action `{}` that
never assigned `$$`. goyacc therefore left stale stack memory in it — for
`CAST(col AS UNSIGNED)` that stale value was the preceding column name. The
`UNSIGNED integer_opt` rule uses it as the type's FamilyString, so the cast
rendered as e.g. `cast(zsd04.vgpos as vgpos unsigned)`. ORDER BY qualifies
its column refs (via qualifyColumnNames) and keys the grouped-expression
lookup on tree.String of the cast; the leaked column name made that key
differ from the GROUP BY key, so the match failed and the bare column was
reported as not grouped. (SELECT happened to bind the cast directly, so it
only surfaced once ORDER BY was present.)

Fix: make the empty `integer_opt` action assign `$$ = ""` so the cast target
type is rendered deterministically (`as unsigned` / `as signed`). This also
fixes the same latent corruption for CAST(... AS SIGNED). Regenerated
mysql_sql.go (0 conflicts).

Adds parser round-trip regression cases and a BVT case
(dml/select/group_by_cast) covering the issue's reproductions plus SIGNED,
UNSIGNED INTEGER, DESC ordering, and integer-column variants.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

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 →

@mergify

mergify Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

@mergify

mergify Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-06-26 19:33 UTC · Rule: main · triggered by @fengttt with the merge queue checkbox
  • Checks skipped · PR is already up-to-date
  • Merged2026-06-26 19:33 UTC · at 6fe5a26a2f8c011e41d9d29b8775bc9e86d774b6 · squash

This pull request spent 21 seconds in the queue, including 2 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • github-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-skipped = Matrixone Utils CI / Coverage
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Linux/arm64
    • check-neutral = Matrixone CI / SCA Test on Linux/arm64
    • check-skipped = Matrixone CI / SCA Test on Linux/arm64

@mergify mergify Bot added the queued label Jun 26, 2026
@mergify
mergify Bot merged commit 6058cfc into matrixorigin:main Jun 26, 2026
28 checks passed
@mergify mergify Bot removed the queued label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants