Skip to content

Conversation

@fengttt
Copy link
Contributor

@fengttt fengttt commented Nov 7, 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 #22721

What this PR does / why we need it:

Make plan testable.


PR Type

Enhancement


Description

  • Refactored EXPLAIN statement parsing to support flexible option combinations

  • Added constants for EXPLAIN options (phyplan, analyze, verbose, force, check, format)

  • Replaced multiple hardcoded grammar rules with unified explain_option_list rule

  • Introduced MakeExplainStmt helper function for centralized statement creation logic

  • Generalized OptionContains function to check any option type

  • Fixed typo: epxlainexplain in variable names


Diagram Walkthrough

flowchart LR
  A["Multiple hardcoded<br/>EXPLAIN rules"] -->|Consolidate| B["Unified<br/>explain_option_list"]
  B -->|Process| C["MakeExplainStmt<br/>helper function"]
  C -->|Create| D["ExplainStmt<br/>with options"]
  E["Option constants<br/>defined"] -->|Used by| C
Loading

File Walkthrough

Relevant files
Enhancement
explain.go
Add option constants and centralize statement creation     

pkg/sql/parsers/tree/explain.go

  • Added six option constants: PhyPlanOption, AnalyzeOption,
    VerboseOption, ForceOption, CheckOption, FormatOption
  • Replaced IsContainAnalyze and IsContainPhyPlan with generic
    OptionContains function
  • Removed MakeOptions helper function
  • Added MakeExplainStmt function to centralize EXPLAIN statement
    creation logic based on options
+27/-17 
mysql_sql.y
Consolidate EXPLAIN grammar rules and fix typos                   

pkg/sql/parsers/dialect/mysql/mysql_sql.y

  • Fixed typo: renamed epxlainOptions and epxlainOption to explainOptions
    and explainOption
  • Replaced 119 lines of repetitive EXPLAIN grammar rules with unified
    explain_option_list and explain_option_elem rules
  • Added support for FORCE and CHECK options in explain_option_key
  • Fixed typo: explain_foramt_valueexplain_format_value
  • Simplified utility_option_list to use direct slice initialization
    instead of MakeOptions
  • Added STRING as valid utility_option_arg type
+35/-105
Miscellaneous
mysql_sql.go
Generated parser code from grammar updates                             

pkg/sql/parsers/dialect/mysql/mysql_sql.go

  • Auto-generated file from grammar changes in mysql_sql.y
+9480/-9530
Dependencies
go.mod
Update module dependencies                                                             

go.mod

  • Dependency version updates
+0/-1     
go.sum
Update dependency checksums                                                           

go.sum

  • Checksum updates for dependency versions
+0/-2     

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 7, 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

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: Comprehensive Audit Trails

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

Status:
No auditing: The new parsing/option-handling code adds EXPLAIN options and statement construction
without any logging of critical actions, but as parser code it may rely on higher layers
for auditing.

Referred Code
func MakeOptionElem(name string, value string) OptionElem {
	return OptionElem{
		Name:  name,
		Value: value,
	}
}

// OptionContains checks if the option is in the options list and
// returns the value of the option
func OptionContains(options []OptionElem, option string) bool {
	if len(options) > 0 {
		for _, o := range options {
			if strings.EqualFold(o.Name, option) {
				return true
			}
		}
	}
	return false
}

func MakeExplainStmt(stmt Statement, options []OptionElem) Statement {


 ... (clipped 13 lines)
Generic: Robust Error Handling and Edge Case Management

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

Status:
Missing validation: New functions like OptionContains and MakeExplainStmt assume option parsing succeeds and
do not handle malformed or conflicting options, which may be validated elsewhere in the
parser.

Referred Code
// OptionContains checks if the option is in the options list and
// returns the value of the option
func OptionContains(options []OptionElem, option string) bool {
	if len(options) > 0 {
		for _, o := range options {
			if strings.EqualFold(o.Name, option) {
				return true
			}
		}
	}
	return false
}

func MakeExplainStmt(stmt Statement, options []OptionElem) Statement {
	if OptionContains(options, PhyPlanOption) {
		explainStmt := NewExplainPhyPlan(stmt, "text")
		explainStmt.Options = options
		return explainStmt
	} else if OptionContains(options, AnalyzeOption) {
		explainStmt := NewExplainAnalyze(stmt, "text")
		explainStmt.Options = options


 ... (clipped 6 lines)
Generic: Security-First Input Validation and Data Handling

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

Status:
Option sanitation: The grammar accepts STRING for CHECK and FORMAT values and passes them directly into
OptionElem without explicit sanitization here, which may be safe if later layers validate
or constrain usage.

Referred Code
explain_option_elem:
    ANALYZE { $$ = tree.MakeOptionElem(tree.AnalyzeOption, "NULL") }
    | FORCE { $$ = tree.MakeOptionElem(tree.ForceOption, "NULL") }
    | VERBOSE { $$ = tree.MakeOptionElem(tree.VerboseOption, "NULL") }
    | PHYPLAN { $$ = tree.MakeOptionElem(tree.PhyPlanOption, "NULL") }
    | FORMAT explain_format_value { $$ = tree.MakeOptionElem(tree.FormatOption, $2) }
    | CHECK STRING { $$ = tree.MakeOptionElem(tree.CheckOption, $2) }
  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@fengttt fengttt merged commit edc1e62 into matrixorigin:main Nov 14, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Review effort 3/5 size/XXL Denotes a PR that changes 2000+ lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants