(pick to 3.0-dev): IF function with GROUPING in ROLLUP

[XuPeng-SH]

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:

issue #19998

What this PR does / why we need it:

Root Cause

The issue occurs in the type checking logic of the IF() function (iffCheck in operatorSet.go):

When IF() receives mixed types (string and numeric), it tries to unify them to a single return type
The function iterates through retOperatorIffSupports array to find the best type match
Previously, numeric types (int8, int16, int32, etc.) were listed before string types (varchar, char, etc.)
This caused the system to attempt converting 'All years' (string) to int type
While the type system allows varchar→int implicit cast (for numeric strings like '123'), the actual cast of 'All years' to int fails at runtime

Solution

Added special handling logic in iffCheck function in /pkg/sql/plan/function/operatorSet.go to detect mixed string/numeric types and
prioritize string type as the return type.

---

PR Type

Bug fix

---

Description

• Fix IF function type checking with mixed string/numeric types

• Prioritize string return type when mixing string and numeric operands

• Prevent runtime cast errors for non-numeric strings like 'All years'

• Add comprehensive test coverage for mixed-type IF expressions

---

Diagram Walkthrough

flowchart LR
  A["IF with mixed<br/>string/numeric types"] --> B["Detect mixed types<br/>in iffCheck"]
  B --> C["Return varchar type<br/>instead of numeric"]
  C --> D["Avoid runtime<br/>cast errors"]

Loading

File Walkthrough

	Relevant files
Bug fix	operatorSet.go Add mixed type handling in IF type checking                            pkg/sql/plan/function/operatorSet.go Added special handling logic in iffCheck function to detect mixed string/numeric types When one operand is string and the other is numeric, prioritize varchar as return type Use setMaxWidthFromSource to preserve width information from source types Moved source variable declaration to accommodate new logic +13/-1
Tests	operatorSet_test.go Add unit tests for mixed-type IF expressions                          pkg/sql/plan/function/operatorSet_test.go Added new test function Test_IffCheck_MixedTypes with three test cases Test case 1: IF with string and int32 returns varchar type Test case 2: IF with int32 and string (reversed order) returns varchar type Test case 3: IF with same numeric types returns numeric type +44/-0    rollup.sql Add comprehensive ROLLUP test cases for mixed types            test/distributed/cases/window/rollup.sql Added comprehensive test suite for issue [Bug]: mo report 'invalid argument cast to int, bad value All years' in rollup with grouping. #19998 with 6 test scenarios Test 1: IF with string literal and numeric column in ROLLUP context Test 2: Mixed types in different order with GROUPING condition Test 3: Multiple mixed-type IF expressions with GROUPING Test 4: Nested IF with GROUPING and mixed types Test 5: CASE expression alternative to IF with mixed types Test 6: Verify varchar return type works with string concatenation +93/-0    rollup.result Add expected results for mixed-type ROLLUP tests                  test/distributed/cases/window/rollup.result Added expected output results for all 6 new test scenarios Demonstrates correct handling of IF with GROUPING returning string labels Shows proper aggregation results with mixed string/numeric types Includes nested IF and CASE expression results +123/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit cfa6e4a)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No auditing: The added logic for type resolution in iffCheck performs no auditing or logging of critical actions, but this code path may not be a security-auditable action area and the diff alone cannot determine audit requirements. Referred Code // Special handling for mixed string/numeric types (issue #19998) // If one is string and the other is numeric, prefer string type // This avoids runtime errors when casting 'All years' to int source := []types.Type{inputs[1], inputs[2]} isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() if (source[0].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && source[1].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() setMaxWidthFromSource(&retType, source) return newCheckResultWithCast(0, []types.Type{types.T_bool.ToType(), retType, retType}) }
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge cases unclear: The new mixed-type branch assumes MySQL string or numeric detection but does not explicitly handle nulls, empty strings, or width overflow beyond setMaxWidthFromSource, which may be acceptable depending on surrounding framework not visible in the diff. Referred Code // Special handling for mixed string/numeric types (issue #19998) // If one is string and the other is numeric, prefer string type // This avoids runtime errors when casting 'All years' to int source := []types.Type{inputs[1], inputs[2]} isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() if (source[0].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && source[1].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() setMaxWidthFromSource(&retType, source) return newCheckResultWithCast(0, []types.Type{types.T_bool.ToType(), retType, retType}) }

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit 33a1e9d

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No auditing: New logic changes type selection for IF but does not add any audit logging for critical actions, although it may not be applicable to this low-level type-checking function. Referred Code // Special handling for mixed string/numeric types (issue #19998) // If one is string and the other is numeric, prefer string type // This avoids runtime errors when casting 'All years' to int source := []types.Type{inputs[1], inputs[2]} isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() if (source[0].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && source[1].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() setMaxWidthFromSource(&retType, source) return newCheckResultWithCast(0, []types.Type{types.T_bool.ToType(), retType, retType}) }
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge cases unhandled: The special-case branch prefers varchar for mixed string/numeric but does not explicitly handle non-MySQL string types or mixed decimal/char width/precision edge cases beyond max width setting. Referred Code // Special handling for mixed string/numeric types (issue #19998) // If one is string and the other is numeric, prefer string type // This avoids runtime errors when casting 'All years' to int source := []types.Type{inputs[1], inputs[2]} isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() if (source[0].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && source[1].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() setMaxWidthFromSource(&retType, source) return newCheckResultWithCast(0, []types.Type{types.T_bool.ToType(), retType, retType}) }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Address type precedence systemically Instead of adding special-case logic for the IF function, improve the general type precedence and coercion cost model. This will systemically handle mixed string/numeric types and ensure consistent behavior across all relevant functions like CASE or COALESCE. Examples: pkg/sql/plan/function/operatorSet.go [341-352] // Special handling for mixed string/numeric types (issue #19998) // If one is string and the other is numeric, prefer string type // This avoids runtime errors when casting 'All years' to int source := []types.Type{inputs[1], inputs[2]} isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() if (source[0].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && source[1].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() setMaxWidthFromSource(&retType, source) ... (clipped 2 lines) Solution Walkthrough: Before: // in pkg/sql/plan/function/operatorSet.go func iffCheck(_ []overload, inputs []types.Type) checkResult { // ... // Special handling for mixed string/numeric types isNumeric0 := inputs[1].Oid.IsInteger() || ... isNumeric1 := inputs[2].Oid.IsInteger() || ... if (inputs[1].Oid.IsMySQLString() && isNumeric1) || (isNumeric0 && inputs[2].Oid.IsMySQLString()) { retType := types.T_varchar.ToType() return newCheckResultWithCast(0, []types.Type{..., retType, retType}) } // General cost-based type resolution logic minCost := math.MaxInt32 for _, rett := range retOperatorIffSupports { // ... calculate cost and find best type } // ... } After: // in pkg/sql/plan/function/operatorSet.go func iffCheck(_ []overload, inputs []types.Type) checkResult { // ... // NO special handling for mixed types. // The general logic is now robust enough. // General cost-based type resolution logic // This logic would be updated to correctly prioritize string // over numeric in mixed-type scenarios based on a refined // cost model, making it applicable to other functions as well. minCost := math.MaxInt32 source := []types.Type{inputs[1], inputs[2]} for _, rett := range retOperatorIffSupports { // ... calculate cost based on improved coercion cost model } // ... } Suggestion importance[1-10]: 9 __ Why: This is a strong architectural suggestion that correctly identifies the local nature of the fix and proposes a more robust, systemic solution that would prevent similar bugs in other functions.	High
General	Add assertion for return type Add assertions to the IF(int, int) test case to verify that the return type is correctly an integer, ensuring the new logic for mixed types does not cause regressions. pkg/sql/plan/function/operatorSet_test.go [423-432] // Test case 3: IF(condition, int, int) should return int { inputs := []types.Type{ types.T_bool.ToType(), types.T_int32.ToType(), types.T_int32.ToType(), } result := iffCheck(nil, inputs) require.True(t, result.status != failedFunctionParametersWrong, "iffCheck should accept same int types") + require.True(t, len(result.finalType) > 0, "should have final types") + require.True(t, result.finalType[1].Oid.IsInteger(), "return type should be integer for int,int inputs") } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that a test case is incomplete and proposes adding assertions to verify the return type, which makes the test more robust and aligned with its description.	Low
	Refactor duplicated logic into a helper Refactor the duplicated numeric type check into a local helper function to improve code clarity and maintainability. pkg/sql/plan/function/operatorSet.go [345-346] - isNumeric0 := source[0].Oid.IsInteger() || source[0].Oid.IsFloat() || source[0].Oid.IsDecimal() - isNumeric1 := source[1].Oid.IsInteger() || source[1].Oid.IsFloat() || source[1].Oid.IsDecimal() + isNumeric := func(t types.Type) bool { + return t.Oid.IsInteger() || t.Oid.IsFloat() || t.Oid.IsDecimal() + } + isNumeric0 := isNumeric(source[0]) + isNumeric1 := isNumeric(source[1]) Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies duplicated logic and proposes a valid refactoring that improves readability and maintainability by creating a local helper function.	Low
Update