Skip to content

fix(function,plan): GREATEST/LEAST numeric promotion (#25145) and ambiguous-column typo (#25146) - #25168

Merged
fengttt merged 4 commits into
matrixorigin:mainfrom
fengttt:feature/greatest-least-numeric-promotion
Jun 29, 2026
Merged

fix(function,plan): GREATEST/LEAST numeric promotion (#25145) and ambiguous-column typo (#25146)#25168
fengttt merged 4 commits into
matrixorigin:mainfrom
fengttt:feature/greatest-least-numeric-promotion

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 #25145
issue #25146

What this PR does / why we need it:

Two independent bugs from the same investigation surface.

#25145 — GREATEST / LEAST reject implicit numeric promotion

GREATEST/LEAST rejected mixed numeric arguments (e.g. GREATEST(1, 2.0),
GREATEST(BIGINT, DOUBLE), GREATEST(COUNT(*), AVG(x))) with
invalid argument ... bad value, because the type checker required every
non-NULL argument to share the exact same type. MySQL promotes such arguments
to a common numeric type and compares on it.

leastGreatestCheck now keeps the existing fast path when all non-NULL
arguments already share the same type, and otherwise derives a common numeric
type and requests an implicit cast of every argument to it (via
newCheckResultWithCast):

  • any floating-point operand → DOUBLE
  • any DECIMAL operand → DECIMAL widened to hold every operand
    (DECIMAL128/256, falling back to DOUBLE only beyond DECIMAL256)
  • integer operands → narrowest integer that holds them all; signed+unsigned
    that cannot fit INT64 widen to DECIMAL128 to stay lossless

Non-numeric arguments that do not already match are still rejected. The
executor switches gain a DECIMAL256 branch so a promoted DECIMAL256
compares instead of hitting the unreachable-code panic.

#25146 — typo "ambiguouse" → "ambiguous"

The ambiguous-column-reference error (code 20301) raised after a JOIN
misspelled "ambiguous". Fixed the message in bind_context.go and the four
BVT result files that assert it.

Tests

  • New unit test TestLeastGreatestCheck covering the promotion matrix; existing
    TestLeast/TestGreatest still pass.
  • New BVT case function/greatest_least_numeric (27 statements) covering the
    issue reproductions, aggregate-vs-aggregate comparisons, NULL handling, and
    per-row mixed columns.
  • Regression: function/builtin (247), and the four [Bug]: Typo "ambiguouse" → "ambiguous" in column-reference error message #25146-affected BVT cases
    (dtype/varchar, view/alter_view, dml/select/order_by_clause,
    window/window) all pass.

🤖 Generated with Claude Code

fengttt and others added 2 commits June 25, 2026 22:56
…origin#25145)

GREATEST/LEAST rejected mixed numeric arguments (e.g. BIGINT + DOUBLE or
BIGINT + DECIMAL) with "invalid argument ... bad value", forcing users to
wrap every argument in an explicit CAST. MySQL promotes such arguments to a
common numeric type and compares on it.

leastGreatestCheck now keeps the existing fast path when all non-NULL
arguments share the same type, and otherwise derives a common numeric type
and requests an implicit cast of every argument to it:

  - any floating-point operand   -> DOUBLE
  - any DECIMAL operand          -> DECIMAL widened to hold every operand
                                    (DECIMAL128/256, falling back to DOUBLE
                                    only if it would exceed DECIMAL256)
  - integer operands             -> narrowest integer that holds them all;
                                    signed+unsigned that cannot fit INT64
                                    widen to DECIMAL128 to stay lossless

Non-numeric arguments that do not already match are still rejected. The
executor switches gain a DECIMAL256 branch so a promoted DECIMAL256 compares
instead of hitting the unreachable-code panic.

Adds a unit test for the promotion matrix and a BVT case
(function/greatest_least_numeric) covering the issue's reproductions,
aggregate-vs-aggregate comparisons, NULL handling, and per-row mixed columns.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…atrixorigin#25146)

The ambiguous-column-reference error (code 20301) raised after a JOIN
misspelled "ambiguous" as "ambiguouse". The typo looks unprofessional in
user-facing tools and breaks naive error-pattern matching that searches for
"ambiguous".

Fix the message in bind_context.go and update the four BVT result files that
assert it (dtype/varchar, view/alter_view, dml/select/order_by_clause,
window/window).

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 →

…ixorigin#25145)

The Coverage CI gate flagged uncovered lines in the GREATEST/LEAST numeric
promotion code. Add unit and BVT coverage for the branches the existing tests
did not reach:

- TestLeastGreatestCheck: all-unsigned width promotion, BIT operands,
  DECIMAL256 seed, mixed-Oid decimals, and the leading-non-numeric reject path.
- TestLeastGreatestWidthHelpers: every branch of signedTypeForWidth /
  unsignedTypeForWidth.
- TestLeastGreatestCommonNumericType: drives the helper directly to hit the
  all-signed/all-unsigned paths, BIT, the DECIMAL256 seed, and the
  DECIMAL-precision-overflow fallback to DOUBLE.
- TestLeastGreatestDecimal256: exercises the new DECIMAL256 branch of the
  leastFn/greatestFn executors.

These bring leastGreatestCheck, leastGreatestCommonNumericType,
signedTypeForWidth and unsignedTypeForWidth to 100% and cover the new
DECIMAL256 executor cases.

BVT function/greatest_least_numeric gains unsigned-only width promotion,
signed+unsigned overflow to DECIMAL128, BIT+integer, mixed-scale DECIMAL128,
and DECIMAL256 result cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@matrix-meow matrix-meow added size/L Denotes a PR that changes [500,999] lines and removed size/M Denotes a PR that changes [100,499] lines labels Jun 26, 2026
@mergify

mergify Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fengttt
fengttt merged commit 24d9d1e into matrixorigin:main Jun 29, 2026
1 of 4 checks passed
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