Skip to content

Conversation

@flypiggyyoyoyo
Copy link
Collaborator

@flypiggyyoyoyo flypiggyyoyoyo commented Dec 12, 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 #23265

What this PR does / why we need it:

This PR adds two enhancements:

  1. WEEK() function mode parameter support: Adds optional second parameter mode (0-7) for WEEK(date, mode) and WEEK(datetime, mode) to control week numbering behavior, making it compatible with MySQL's WEEK()
    function specification.
  2. Configurable CTE max recursion depth: Allows users to configure CTE recursion limit via SET cte_max_recursion_depth = N instead of using a hardcoded value. This enables deeper recursion for complex
    hierarchical queries when needed.

PR Type

Enhancement, Bug fix


Description

  • Add optional mode parameter (0-7) to WEEK() function for date/datetime types

    • Supports MySQL-compatible week numbering modes
    • Handles mode validation and clamping to valid range
  • Make CTE max recursion depth configurable via SET cte_max_recursion_depth variable

    • Replaces hardcoded recursion limit with dynamic resolution
    • Enables deeper recursion for complex hierarchical queries
  • Add comprehensive test coverage for WEEK() mode parameter and CTE recursion depth


Should we remain consistent with MySQL?

  1. Prerequisites (Create Database/Table)

MySQL:

CREATE DATABASE IF NOT EXISTS test;
USE test;

MO:

CREATE DATABASE IF NOT EXISTS test;
USE test;

  1. WEEK Function Tests

Normal Cases (MySQL and MO results are identical ✅)

SQL MySQL MO
SELECT week('2023-01-01', 0); 1 1
SELECT week('2023-01-01', 1); 0 0
SELECT week('2023-01-02', 0); 1 1
SELECT week('2023-01-02', 1); 1 1
SELECT week('2023-12-31', 0); 53 53
SELECT week('2023-12-31', 1); 52 52
SELECT week('2023-01-01', -1); 52 52
SELECT week('2023-01-01', 8); 1 1
SELECT week('2023-01-01', 100); 1 1
SELECT week(null, 0); NULL NULL
SELECT week('2023-01-01', null); 1 1

Invalid Cases (MySQL and MO behavior differs ⚠️)

SQL MySQL MO Note
SELECT week('0000-00-00', 0); NULL Error: invalid argument parsedate MO is stricter, rejects invalid date
SELECT week('2023-02-30', 0); NULL Error: invalid argument parsedate MO is stricter, rejects non-existent date
SELECT week('abc', 0); NULL Error: invalid argument parsedate MO is stricter, rejects invalid format
SELECT week('', 0); NULL NULL Identical

  1. CTE Recursion Depth Tests

Prerequisites

  CREATE DATABASE IF NOT EXISTS test;
  USE test;
  DROP TABLE IF EXISTS t_cte;
  CREATE TABLE t_cte(a int);
  INSERT INTO t_cte VALUES (1);

Normal Cases (MySQL and MO results are identical ✅)

SQL MySQL MO
SET cte_max_recursion_depth = 200;WITH RECURSIVE c AS (SELECT a FROM t_cte UNION ALL SELECT a+1 FROM c WHERE a < 150) SELECT count(*) FROM c; 150 150
SET cte_max_recursion_depth = 50;WITH RECURSIVE c AS (SELECT a FROM t_cte UNION ALL SELECT a+1 FROM c WHERE a < 100) SELECT count(*) FROM c; Error: Recursive query aborted after 51 iterations Error: recursive level out of range

Invalid Cases (MySQL and MO behavior differs ⚠️)

SQL MySQL MO Note
SET cte_max_recursion_depth = -1; Success, silently converts to 0 Error: convert to the system variable int type failed MO is stricter, rejects negative values
SET cte_max_recursion_depth = -100; Success, silently converts to 0 Error: convert to the system variable int type failed MO is stricter, rejects negative values
SET cte_max_recursion_depth = 0; Success, value is 0 Success, value is 0 Identical ✅
SET cte_max_recursion_depth = 4294967295; Success, value is 4294967295 Success Identical ✅
SET cte_max_recursion_depth = 4294967296; Success, silently truncates to 4294967295 Error: convert to the system variable int type failed MO is stricter, rejects overflow
SET cte_max_recursion_depth = 'abc'; Error: Incorrect argument type Error: convert to the system variable int type failed Both error ✅

Diagram Walkthrough

flowchart LR
  A["WEEK Function"] -->|"Add mode parameter"| B["DateToWeek/DatetimeToWeek"]
  B -->|"Support modes 0-7"| C["Week calculation"]
  D["CTE Recursion"] -->|"Read variable"| E["cte_max_recursion_depth"]
  E -->|"Dynamic limit"| F["MergeCTE execution"]
  G["Test Cases"] -->|"Validate"| B
  G -->|"Validate"| F
Loading

File Walkthrough

Relevant files
Enhancement
func_unary.go
Add mode parameter support to WEEK functions                         

pkg/sql/plan/function/func_unary.go

  • Refactored DateToWeek() to support optional mode parameter with
    validation
  • Refactored DatetimeToWeek() to support optional mode parameter with
    validation
  • Replaced simple WeekOfYear2() calls with Week(mode) method calls
  • Added manual loop-based implementation to handle mode parameter
    extraction and null handling
+72/-6   
list_builtIn.go
Register WEEK function overloads with mode parameter         

pkg/sql/plan/function/list_builtIn.go

  • Added two new function overloads for WEEK() with mode parameter
  • Overload 2: WEEK(date, int64) returning uint8
  • Overload 3: WEEK(datetime, int64) returning uint8
  • Maintains backward compatibility with existing single-parameter
    overloads
+20/-0   
mergecte.go
Make CTE recursion limit configurable via variable             

pkg/sql/colexec/mergecte/mergecte.go

  • Replaced hardcoded moDefaultRecursionMax with dynamic variable
    resolution
  • Added proc.GetResolveVariableFunc() call to fetch
    cte_max_recursion_depth variable
  • Implemented fallback to default value if variable resolution fails
  • Added type checking and validation for resolved variable value
+9/-1     
Tests
func_date.sql
Add WEEK function mode parameter test cases                           

test/distributed/cases/function/func_date.sql

  • Added test cases for WEEK() function with mode parameter
  • Tests cover modes 0, 1, 2, 3 with both date and datetime types
  • Tests include edge cases like year boundaries (2023-01-01, 2023-12-31)
  • Validates behavior with NULL values
+11/-0   
func_date.result
Update WEEK function test results with mode parameter       

test/distributed/cases/function/func_date.result

  • Updated expected results for WEEK() function tests with various modes
  • Fixed whitespace formatting in existing test results (tabs to spaces)
  • Added comprehensive output for WEEK() mode parameter test cases
  • Validates correct week numbers for different modes and dates
+109/-20
recursive_cte.sql
Add CTE recursion depth configuration test cases                 

test/distributed/cases/recursive_cte/recursive_cte.sql

  • Added test cases for configurable cte_max_recursion_depth variable
  • Tests SET command with different recursion depth values (200, 50, 100)
  • Validates that queries succeed/fail based on configured depth limits
  • Tests cleanup with table drop statements
+11/-0   
recursive_cte.result
Update recursive CTE test results with configurable depth

test/distributed/cases/recursive_cte/recursive_cte.result

  • Updated expected results for recursive CTE tests with configurable
    depth
  • Changed previous error result to successful query output for depth=200
    case
  • Added expected output for CTE recursion depth variable tests
  • Validates that depth limit of 50 correctly rejects queries exceeding
    it
  • Fixed column name casing in Person table test results
+609/-3 

@CLAassistant
Copy link

CLAassistant commented Dec 12, 2025

CLA assistant check
All committers have signed the CLA.

@qodo-code-review
Copy link

qodo-code-review bot commented Dec 12, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

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: 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:
Missing Audit: The new logic that reads and enforces cte_max_recursion_depth performs a critical control
decision without any explicit auditing/logging of the configured value or enforcement
outcome in the added lines.

Referred Code
maxRecursion := moDefaultRecursionMax
if resolveFunc := proc.GetResolveVariableFunc(); resolveFunc != nil {
	if val, err := resolveFunc("cte_max_recursion_depth", true, false); err == nil {
		if v, ok := val.(int64); ok {
			maxRecursion = int(v)
		}
	}
}
if mergeCTE.ctr.recursiveLevel > maxRecursion {
	result.Status = vm.ExecStop
	return result, moerr.NewCheckRecursiveLevel(proc.Ctx)
}

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:
Silent Fallback: When resolving cte_max_recursion_depth, errors or non-int64 values are silently ignored
and default is used without recording context, which may hinder debugging and hides
misconfiguration.

Referred Code
maxRecursion := moDefaultRecursionMax
if resolveFunc := proc.GetResolveVariableFunc(); resolveFunc != nil {
	if val, err := resolveFunc("cte_max_recursion_depth", true, false); err == nil {
		if v, ok := val.(int64); ok {
			maxRecursion = int(v)
		}
	}
}
if mergeCTE.ctr.recursiveLevel > maxRecursion {
	result.Status = vm.ExecStop

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 size/XL Denotes a PR that changes [1000, 1999] lines and removed size/L Denotes a PR that changes [500,999] lines labels Dec 17, 2025
@mergify mergify bot added the queued label Dec 17, 2025
@mergify mergify bot merged commit 35d1c6d into matrixorigin:main Dec 17, 2025
19 checks passed
@mergify
Copy link
Contributor

mergify bot commented Dec 17, 2025

Merge Queue Status

✅ The pull request has been merged at 57050c5

This pull request spent 7 seconds in the queue, with no time running CI.
The checks were run in-place.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
    • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / SCA Test on Ubuntu/x86
    • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
    • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone CI / UT Test on Ubuntu/x86
    • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
    • check-skipped = Matrixone CI / UT Test on Ubuntu/x86
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
    • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
    • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • any of [🛡 GitHub branch protection]:
    • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
    • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = Matrixone Utils CI / Coverage
    • check-neutral = Matrixone Utils CI / Coverage
    • check-success = Matrixone Utils CI / Coverage

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 Review effort 2/5 size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants