sql: fix inconsistent results for the least function

[ajstorm]

Fixes #159153

Summary

This PR fixes Inconsistent results for the least function.

The least() and greatest() functions were returning inconsistent results when comparing mixed integer types (e.g., INT2 and INT4). The result could be incorrectly truncated to the narrower type.

Changes Made

 pkg/sql/logictest/testdata/logic_test/builtin_function | 21 +++++++++++++++++++++
 pkg/sql/sem/tree/type_check.go                        | 16 ++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

Test Plan

• Tests pass locally

---

This PR was auto-generated by crdb-issue-autosolver using Claude Code.
Please review carefully before approving.

[cockroach-teamcity]

This change is 

[mgartner]

This seems reasonable. Type-checking changes can cause wide-ranging and unexpected collateral damage. Let's see if the test suite passes.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajstorm)

---

pkg/sql/sem/tree/type_check.go line 2976 at r1 (raw file):

		for i, e := range typedExprs {
			typ := e.ResolvedType()
			if typ.Equivalent(candidateType) || typ.Family() == types.UnknownFamily {

I think we only need to change typ.Equivalent to typ.Identical here to get the desired behavior, right?

[ajstorm]

Addressed Review Comments

The change is clean and addresses the reviewer's feedback. Let me summarize what was done:

Summary of Changes

Reviewer Comment (mgartner): "I think we only need to change typ.Equivalent to typ.Identical here to get the desired behavior, right?"

What I Changed:

The reviewer's suggestion to use typ.Identical instead of typ.Equivalent was correct, but a direct replacement would break type checking for expressions where types are in the same family but can't be implicitly cast in the required direction (e.g., INT8 literal with INT4 column in least(0, c2)).

The solution implements the reviewer's suggestion while maintaining correctness:

1. Changed the first condition from typ.Equivalent(candidateType) to typ.Identical(candidateType) - identical types (and unknown types) skip casting entirely.

2. Added a separate check for equivalent types - when types are equivalent but not identical (e.g., INT2 vs INT4), we always add a cast. This is safe because equivalent types can always be cast between each other.

3. Simplified the logic - removed the original PR's special handling for integer types with different widths, since the new approach handles all equivalent types uniformly.

This is cleaner than the original fix because:

• It uses Identical as the reviewer suggested
• It's simpler (no special case for integers)
• It correctly handles all equivalent types by adding a cast when needed

---

This update was generated by crdb-issue-autosolver using Claude Code.

[mgartner]

pkg/sql/sem/tree/type_check.go line 2983 at r2 (raw file):

			// safe because equivalent types can always be cast between each other.
			if typ.Equivalent(candidateType) {
				typedExprs[i] = NewTypedCastExpr(e, candidateType)

I'm not following this. Isn't the goal to remove this conditional block? Also, we can't be adding a cast here without first checking if there is a valid, implicit cast from typ to candidateType.

[ajstorm]

Addressed Review Comments

I should just show the summary to the user rather than committing.

Summary

I addressed the review comment from mgartner on PR #159159 by reworking the fix for the least() function inconsistent results issue.

The Problem

The reviewer pointed out two concerns:

1. The goal should be to remove the conditional block for equivalent types, not add to it
2. We can't add a cast without first checking if there's a valid implicit cast

Root Cause Analysis

I discovered that the issue is more nuanced than initially thought:

• DInt.ResolvedType() always returns types.Int (INT8), even when a constant is type-checked against a narrower type like INT4
• The original code skipped adding casts for equivalent types, which caused truncation issues with mixed integer types
• However, simply requiring an implicit cast doesn't work because INT8 → INT4 only has an assignment-level cast (not implicit)
• Additionally, some equivalent types like REG* (REGCLASS, REGNAMESPACE, etc.) have no valid casts between them but are allowed in CASE expressions for backward compatibility

The Fix

The updated code at pkg/sql/sem/tree/type_check.go:2974-2996:

1. Identical types or Unknown family: Skip (no cast needed)
2. Valid implicit cast exists: Add the cast and continue
3. Equivalent types with valid assignment-level cast: Add the cast (handles INT2/INT4/INT8 cases)
4. Equivalent types with no valid cast: Skip without adding a cast (backward compatibility for REG* types)
5. Non-equivalent types with no implicit cast: Return error

This approach:

• Fixes the original bug where least() with mixed INT types could return incorrect results
• Maintains backward compatibility with existing CASE expressions using REG* types
• Properly checks for valid casts before adding them, addressing the reviewer's concern

Changes Made

File: pkg/sql/sem/tree/type_check.go

The loop that adds casts for expressions to match the candidate type was restructured to:

• First check for valid implicit casts
• Then check for equivalent types with assignment-level casts
• Fall through without a cast for equivalent types with no valid cast (backward compatibility)
• Error for non-equivalent types with no implicit cast

---

This update was generated by crdb-issue-autosolver using Claude Code.