Skip to content

Conversation

@aunjgr
Copy link
Contributor

@aunjgr aunjgr commented Nov 10, 2025

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 #22795

What this PR does / why we need it:

zonemap for array type columns are not initialized


PR Type

Bug fix


Description

  • Disable zonemap filtering for array type columns

  • Add explicit check for T_array_float32 and T_array_float64 types

  • Prevent uninitialized zonemap access for array columns


Diagram Walkthrough

flowchart LR
  A["ExprIsZonemappable function"] --> B["Check expression type"]
  B --> C["Column expression detected"]
  C --> D["Array type check"]
  D --> E["Return false for arrays"]
  D --> F["Return true for non-arrays"]
Loading

File Walkthrough

Relevant files
Bug fix
utils.go
Add array type zonemap filtering exclusion                             

pkg/sql/plan/utils.go

  • Added new case handler for plan.Expr_Col expression type
  • Implemented check to detect array types (T_array_float32,
    T_array_float64)
  • Returns false for array types to disable zonemap filtering
  • Returns true for non-array column types to allow zonemap filtering
+8/-0     

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 10, 2025

PR Compliance Guide 🔍

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

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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

Learn more about managing compliance generic rules or creating your own custom rules

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

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No logging: The new logic adds early-return conditions without any audit logging of the decision or
context, which may hinder traceability of critical planning decisions.

Referred Code
case *plan.Expr_Col:
	// Array types don't support zonemapping
	if expr.Typ.Id == int32(types.T_array_float32) || expr.Typ.Id == int32(types.T_array_float64) {
		return false
	}
	return true

Learn more about managing compliance generic rules or creating your own custom rules

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

@matrix-meow matrix-meow added the size/XS Denotes a PR that changes [1, 9] lines label Nov 10, 2025
@mergify mergify bot added the kind/bug Something isn't working label Nov 10, 2025
@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 10, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use a generic prefix check

Generalize the array type check to be more robust and future-proof. Instead of
checking for specific array types, use a prefix check on the type's string
representation to disable zonemapping for all array types.

pkg/sql/plan/utils.go [1101-1106]

 	case *plan.Expr_Col:
 		// Array types don't support zonemapping
-		if expr.Typ.Id == int32(types.T_array_float32) || expr.Typ.Id == int32(types.T_array_float64) {
+		if strings.HasPrefix(types.T(expr.Typ.Id).String(), "T_array") {
 			return false
 		}
 		return true
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the hardcoded check for specific array types is brittle and proposes a more robust prefix-based check, making the code more maintainable and future-proof.

Low
  • Update

@matrix-meow matrix-meow added size/S Denotes a PR that changes [10,99] lines and removed size/XS Denotes a PR that changes [1, 9] lines labels Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/wip kind/bug Something isn't working Review effort 1/5 size/S Denotes a PR that changes [10,99] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants