Skip to content

Conversation

@iamlinjunhong
Copy link
Contributor

@iamlinjunhong iamlinjunhong commented Oct 24, 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 #22662

What this PR does / why we need it:

test


PR Type

Tests


Description

  • Add comprehensive hint rewrite test cases with 539 test scenarios

  • Test basic single and multi-table rewriting functionality

  • Test complex queries including aggregations, window functions, subqueries

  • Test error handling for invalid JSON, syntax errors, and edge cases

  • Add database isolation to join apply tests


Diagram Walkthrough

flowchart LR
  A["Test Suite"] --> B["Hint Rewrite Tests"]
  A --> C["Join Apply Tests"]
  B --> D["hint.sql<br/>545 test cases"]
  B --> E["hint.result<br/>539 expected results"]
  D --> F["Basic Rewrites"]
  D --> G["Complex Queries"]
  D --> H["Error Cases"]
  C --> I["apply.test<br/>with DB isolation"]
  C --> J["apply.result<br/>with DB isolation"]
Loading

File Walkthrough

Relevant files
Tests
hint.result
Add hint rewrite test result file                                               

test/distributed/cases/hint/hint.result

  • New file containing 539 lines of expected test results for hint
    rewrite functionality
  • Covers basic table rewrites, multi-table joins, aggregations, window
    functions
  • Includes error cases with expected error messages for invalid syntax
    and non-existent tables
  • Tests edge cases like empty rules, special characters, case
    sensitivity
+539/-0 
hint.sql
Add hint rewrite SQL test cases                                                   

test/distributed/cases/hint/hint.sql

  • New file with 545 lines of SQL test cases for hint rewrite
    functionality
  • Tests basic rewriting of single and multiple tables with WHERE clause
    filters
  • Tests complex scenarios: aggregations, GROUP BY, window functions,
    subqueries, UNION
  • Tests error conditions: malformed JSON, missing tables, recursive
    references, syntax errors
  • Tests edge cases: empty rules, special characters in table names, case
    sensitivity
+545/-0 
apply.result
Add database isolation to apply tests                                       

test/distributed/cases/join/apply.result

  • Add database creation and setup at the beginning of test file
  • Add database cleanup at the end of test file
  • Ensures test isolation by creating dedicated join_test database
+4/-0     
apply.test
Add database isolation to apply tests                                       

test/distributed/cases/join/apply.test

  • Add database creation and setup at the beginning of test file
  • Add database cleanup at the end of test file
  • Ensures test isolation by creating dedicated join_test database
+4/-0     

@CLAassistant
Copy link

CLAassistant commented Oct 24, 2025

CLA assistant check
All committers have signed the CLA.

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 24, 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 <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

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

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Oct 24, 2025

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

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix broken subquery aggregation test

Add the missing product_name and category columns to the sales table definition
and corresponding INSERT statements to fix the broken subquery aggregation test.

test/distributed/cases/hint/hint.sql [241-263]

 -- subquery aggregation rewrite
 drop table if exists sales;
 create table sales (
     sale_id int primary key,
     product_id int,
+    product_name varchar(50),
+    category varchar(30),
     quantity int,
     sale_date date
 );
 insert into sales values
-(1, 1, 10, '2025-01-01'),
-(2, 1, 15, '2025-01-02'),
-(3, 2, 20, '2025-01-01'),
-(4, 2, 25, '2025-01-03'),
-(5, 3, 30, '2025-01-02');
+(1, 1, 'Laptop', 'Electronics', 10, '2025-01-01'),
+(2, 1, 'Laptop', 'Electronics', 15, '2025-01-02'),
+(3, 2, 'Mouse', 'Electronics', 20, '2025-01-01'),
+(4, 2, 'Mouse', 'Electronics', 25, '2025-01-03'),
+(5, 3, 'Desk', 'Furniture', 30, '2025-01-02');
 /*+ {
         "rewrites": {
             "hint_test.top_products": "SELECT product_id, product_name, category, (SELECT AVG(quantity) FROM sales s2 WHERE s2.category = s1.category) as category_avg_qty, quantity FROM sales s1 WHERE quantity > (SELECT AVG(quantity) FROM sales s3 WHERE s3.category = s1.category)"
         }
 } */
 select product_name, category, quantity, category_avg_qty
 from top_products
 order by category, quantity desc;
 drop table sales;
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a bug where the sales table schema is missing columns product_name and category, which are used in the rewrite hint, making the test case invalid.

High
Consolidate multiple hints into one

Consolidate the two separate hint blocks into a single block to ensure both
rewrite rules for users and orders are applied to the query.

test/distributed/cases/hint/hint.sql [480-515]

 -- multiple hint comment tests
 drop table if exists users;
 create table users (
     id int primary key,
     name varchar(50),
     age int,
     city varchar(50)
 );
 insert into users values
 (1, 'Alice', 25, 'Beijing'),
 (2, 'Bob', 30, 'Shanghai'),
 (3, 'Charlie', 35, 'Guangzhou'),
 (4, 'David', 28, 'Shenzhen');
 drop table if exists orders;
 create table orders (
     order_id int primary key,
     user_id int,
     amount decimal(10,2),
     status varchar(20)
 );
 insert into orders values
 (1, 1, 100.00, 'completed'),
 (2, 2, 200.00, 'pending'),
 (3, 1, 150.00, 'completed'),
 (4, 3, 300.00, 'cancelled');
 /*+ {
     "rewrites": {
-        "hint_test.users": "SELECT * FROM users WHERE age > 25"
-    }
-} */
-/*+ {
-    "rewrites": {
+        "hint_test.users": "SELECT * FROM users WHERE age > 25",
         "hint_test.orders": "SELECT * FROM orders WHERE status = 'completed'"
     }
 } */
 select * from users, orders where users.id = orders.user_id;
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that only the first of two consecutive hint blocks is applied, causing the test to not function as intended, and proposes a valid fix.

Medium
High-level
Consolidate duplicated test setup code

The hint.sql test file contains duplicated table creation and data insertion
logic. This should be consolidated into a single setup block at the beginning of
the file to improve readability and maintainability.

Examples:

test/distributed/cases/hint/hint.sql [8-86]
drop table if exists users;
create table users (
    id int primary key,
    name varchar(50),
    age int,
    city varchar(50)
);
insert into users values
(1, 'Alice', 25, 'Beijing'),
(2, 'Bob', 30, 'Shanghai'),

 ... (clipped 69 lines)

Solution Walkthrough:

Before:

-- Test case 1
drop table if exists users;
create table users (...);
insert into users values (...);
/*+ hint */
select * from hint_test.users;
drop table users;

-- Test case 2
drop table if exists users;
create table users (...); -- Duplicated setup
insert into users values (...); -- Duplicated setup
/*+ hint */
select * from users;

After:

-- Shared setup at the start of the file
drop database if exists hint_test;
create database hint_test;
use hint_test;
create table users (...);
insert into users values (...);
create table orders (...);
insert into orders values (...);
-- ... other common tables

-- Test case 1
/*+ hint */
select * from hint_test.users;

-- Test case 2
/*+ hint */
select * from users;
-- ... more tests without re-creating tables
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies extensive and unnecessary code duplication in the new hint.sql test file, which significantly harms the readability and future maintainability of the test suite.

Medium
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/wip Review effort 2/5 size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants