Skip to content

Conversation

@Ariznawlll
Copy link
Contributor

@Ariznawlll Ariznawlll commented Feb 3, 2026

PR Type

Enhancement


Description

  • Add wait_expect feature for intelligent polling instead of fixed sleep

  • Implement configurable interval and timeout parameters for wait_expect

  • Add parsing logic for @wait_expect flag with validation

  • Implement executeWithWaitExpect method for repeated execution until match

  • Add comprehensive getting started documentation with examples


Diagram Walkthrough

flowchart LR
  A["SqlCommand"] -->|"add waitExpect fields"| B["waitExpect config"]
  C["ScriptParser"] -->|"parse @wait_expect flag"| D["Extract interval/timeout"]
  D -->|"validate parameters"| E["Set command properties"]
  F["Executor.run"] -->|"check isWaitExpect"| G["executeWithWaitExpect"]
  G -->|"loop with interval"| H["Execute SQL"]
  H -->|"check result match"| I["Success or Timeout"]
  J["COMMON"] -->|"add WAIT_EXPECT_FLAG"| K["Flag constant"]
Loading

File Walkthrough

Relevant files
Enhancement
SqlCommand.java
Add wait_expect configuration fields                                         

src/main/java/io/mo/cases/SqlCommand.java

  • Add waitExpect boolean field to enable wait_expect mode
  • Add waitExpectInterval field for check interval in seconds
  • Add waitExpectTimeout field for timeout duration in seconds
  • All fields include getter/setter annotations
+12/-0   
Executor.java
Implement wait_expect execution logic with polling             

src/main/java/io/mo/db/Executor.java

  • Modify run method to call executeWithWaitExpect when wait_expect is
    enabled
  • Add wait_expect handling in genRS method with timeout-based sleep
  • Implement new executeWithWaitExpect method with polling logic
  • Method repeatedly executes SQL and checks result match until success
    or timeout
  • Handles both successful results and SQL errors with proper logging
+125/-1 
ScriptParser.java
Add wait_expect flag parsing and validation                           

src/main/java/io/mo/util/ScriptParser.java

  • Add condition to detect @wait_expect flag in comment lines
  • Implement parseWaitExpectFlag method to parse flag format
  • Validate format: -- @wait_expect(interval, timeout)
  • Validate parameters: both must be positive integers, interval <=
    timeout
  • Set parsed values on SqlCommand object with debug logging
+54/-0   
Configuration changes
COMMON.java
Add wait_expect flag constant                                                       

src/main/java/io/mo/constant/COMMON.java

  • Add WAIT_EXPECT_FLAG constant with value "-- @wait_expect"
  • Follows existing flag naming convention
+1/-0     
Documentation
WAIT_EXPECT_GETTING_STARTED.md
Add comprehensive wait_expect documentation                           

WAIT_EXPECT_GETTING_STARTED.md

  • Comprehensive getting started guide with basic syntax and examples
  • Includes 4 common scenarios: async tasks, data sync, cache updates,
    index building
  • Parameter selection guidelines with interval and timeout
    recommendations
  • Real-world case studies showing performance improvements
  • Debugging tips, common errors, and best practices
  • Performance comparison showing 87.5% time savings vs fixed sleep
+343/-0 

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unbounded wait duration

Description: Potential denial-of-service/hang risk: @wait_expect user-controlled timeout/interval
values from scripts are only validated as positive and interval <= timeout, with no upper
bounds, enabling very long sleeps (e.g., hours) in genRS (sleep-before-execute) and long
polling loops in executeWithWaitExpect, which can stall CI/test runners or tie up database
resources.
Executor.java [396-859]

Referred Code
// For wait_expect in genRS mode: sleep timeout seconds then execute once
if (command.isWaitExpect()) {
    int timeout = command.getWaitExpectTimeout();
    logger.info(String.format("[%s][row:%d] genRS mode: sleeping %ds before execution (wait_expect timeout)",
            command.getScriptFile(), command.getPosition(), timeout));
    Thread.sleep(timeout * 1000L);
}

statement.execute(sqlCmd);
if (command.isNeedWait()) {
    Thread.sleep(COMMON.WAIT_TIMEOUT / 10);
    if (waitThread != null && waitThread.isAlive()) {
        try {
            logger.error(String.format(
                    "Command[%s][row:%d] has been executed before connection[id=%d] commit.\nBut still need to wait for connection[id=%d] being committed",
                    command.getCommand(), command.getPosition(), command.getWaitConnId(),
                    command.getWaitConnId()));
            waitThread.join();
            script.addFailedCmd(command);
            command.getTestResult().setResult(RESULT.RESULT_TYPE_FAILED);
            logger.error("[" + script.getFileName() + "][row:" + command.getPosition() + "]["


 ... (clipped 443 lines)
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: Comprehensive Audit Trails

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

Status: Passed

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

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:
ResultSet resource leak: The new executeWithWaitExpect polling loop repeatedly calls statement.getResultSet()
without explicitly closing the ResultSet, risking resource leaks and degraded stability
over multiple attempts.

Referred Code
while (true) {
    attemptCount++;
    long elapsedMillis = System.currentTimeMillis() - startTime;

    try {
        // Execute the SQL
        statement.execute(sqlCmd);

        // Get the result
        ResultSet resultSet = statement.getResultSet();
        StmtResult actResult;

        if (resultSet != null) {
            RSSet rsSet = new RSSet(resultSet, command);
            actResult = new StmtResult(rsSet);
        } else {
            actResult = new StmtResult();
            actResult.setType(RESULT.STMT_RESULT_TYPE_NONE);
        }

        command.setActResult(actResult);


 ... (clipped 24 lines)

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:
Unstructured log entries: The new wait-expect logging uses free-form formatted strings rather than structured logs
(e.g., JSON), reducing parseability and audit/monitoring effectiveness.

Referred Code
logger.info(String.format("[%s][row:%d] Executing with wait_expect: interval=%ds, timeout=%ds",
        command.getScriptFile(), command.getPosition(), interval, timeout));

while (true) {
    attemptCount++;
    long elapsedMillis = System.currentTimeMillis() - startTime;

    try {
        // Execute the SQL
        statement.execute(sqlCmd);

        // Get the result
        ResultSet resultSet = statement.getResultSet();
        StmtResult actResult;

        if (resultSet != null) {
            RSSet rsSet = new RSSet(resultSet, command);
            actResult = new StmtResult(rsSet);
        } else {
            actResult = new StmtResult();
            actResult.setType(RESULT.STMT_RESULT_TYPE_NONE);


 ... (clipped 16 lines)

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:
DB error exposed: On SQLException, the code stores e.getMessage() into actResult/TestResult, which may be
user-visible test output and could leak internal database details depending on how results
are surfaced.

Referred Code
} catch (SQLException e) {
    // Handle SQL errors
    StmtResult actResult = new StmtResult();
    actResult.setType(RESULT.STMT_RESULT_TYPE_ERROR);
    actResult.setErrorMessage(e.getMessage());
    command.setActResult(actResult);
    command.getTestResult().setActResult(actResult.toString());

    // Set expected result to error type for comparison
    if (command.getExpResult() != null) {
        command.getExpResult().setType(RESULT.STMT_RESULT_TYPE_ERROR);
        command.getExpResult().setErrorMessage(command.getExpResult().getExpectRSText());
    }

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:
Missing defensive bounds: executeWithWaitExpect trusts waitExpectInterval/waitExpectTimeout values without defensive
bounds checks, so invalid/huge values set outside ScriptParser could cause excessive
sleeping, long-running loops, or time calculation issues.

Referred Code
int interval = command.getWaitExpectInterval();
int timeout = command.getWaitExpectTimeout();
long startTime = System.currentTimeMillis();
long timeoutMillis = timeout * 1000L;
int attemptCount = 0;

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

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

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix timeout logic and code duplication

Refactor the executeWithWaitExpect method to fix a critical timeout bug and
remove duplicated code by restructuring the main loop and consolidating logic.

src/main/java/io/mo/db/Executor.java [767-861]

-private void executeWithWaitExpect(Statement statement, String sqlCmd, SqlCommand command, TestScript script) 
+private void executeWithWaitExpect(Statement statement, String sqlCmd, SqlCommand command, TestScript script)
         throws SQLException, InterruptedException {
-    
+
     int interval = command.getWaitExpectInterval();
     int timeout = command.getWaitExpectTimeout();
     long startTime = System.currentTimeMillis();
     long timeoutMillis = timeout * 1000L;
     int attemptCount = 0;
-    
+
     logger.info(String.format("[%s][row:%d] Executing with wait_expect: interval=%ds, timeout=%ds",
             command.getScriptFile(), command.getPosition(), interval, timeout));
-    
-    while (true) {
+
+    while (System.currentTimeMillis() - startTime < timeoutMillis) {
         attemptCount++;
-        long elapsedMillis = System.currentTimeMillis() - startTime;
-        
+        long loopStartTime = System.currentTimeMillis();
+
         try {
-            // Execute the SQL
             statement.execute(sqlCmd);
-            
-            // Get the result
+
             ResultSet resultSet = statement.getResultSet();
             StmtResult actResult;
-            
+
             if (resultSet != null) {
                 RSSet rsSet = new RSSet(resultSet, command);
                 actResult = new StmtResult(rsSet);
             } else {
                 actResult = new StmtResult();
                 actResult.setType(RESULT.STMT_RESULT_TYPE_NONE);
             }
-            
+
             command.setActResult(actResult);
             command.getTestResult().setActResult(actResult.toString());
-            
-            // Check if result matches expected
-            if (command.checkResult()) {
-                logger.info(String.format("[%s][row:%d] wait_expect succeeded after %d attempts (%.2fs)",
-                        command.getScriptFile(), command.getPosition(), attemptCount, elapsedMillis / 1000.0));
-                return; // Success!
-            }
-            
-            // Check if timeout reached
-            if (elapsedMillis >= timeoutMillis) {
-                logger.warn(String.format("[%s][row:%d] wait_expect timeout after %d attempts (%.2fs). Last result did not match expected.",
-                        command.getScriptFile(), command.getPosition(), attemptCount, elapsedMillis / 1000.0));
-                return; // Timeout - return with last result
-            }
-            
-            // Wait before next attempt
-            long remainingMillis = timeoutMillis - elapsedMillis;
-            long sleepMillis = Math.min(interval * 1000L, remainingMillis);
-            
-            if (sleepMillis > 0) {
-                Thread.sleep(sleepMillis);
-            }
-            
+
         } catch (SQLException e) {
-            // Handle SQL errors
             StmtResult actResult = new StmtResult();
             actResult.setType(RESULT.STMT_RESULT_TYPE_ERROR);
             actResult.setErrorMessage(e.getMessage());
             command.setActResult(actResult);
             command.getTestResult().setActResult(actResult.toString());
-            
-            // Set expected result to error type for comparison
+
             if (command.getExpResult() != null) {
                 command.getExpResult().setType(RESULT.STMT_RESULT_TYPE_ERROR);
                 command.getExpResult().setErrorMessage(command.getExpResult().getExpectRSText());
             }
-            
-            // Check if error matches expected
-            if (command.checkResult()) {
-                logger.info(String.format("[%s][row:%d] wait_expect succeeded with expected error after %d attempts (%.2fs)",
-                        command.getScriptFile(), command.getPosition(), attemptCount, elapsedMillis / 1000.0));
-                return; // Expected error matched
-            }
-            
-            // Check if timeout reached
-            if (elapsedMillis >= timeoutMillis) {
-                logger.warn(String.format("[%s][row:%d] wait_expect timeout after %d attempts (%.2fs). Last error did not match expected.",
-                        command.getScriptFile(), command.getPosition(), attemptCount, elapsedMillis / 1000.0));
-                return; // Timeout - return with last error
-            }
-            
-            // Wait before next attempt
-            long remainingMillis = timeoutMillis - elapsedMillis;
-            long sleepMillis = Math.min(interval * 1000L, remainingMillis);
-            
-            if (sleepMillis > 0) {
-                Thread.sleep(sleepMillis);
-            }
+        }
+
+        long elapsedMillis = System.currentTimeMillis() - startTime;
+        if (command.checkResult()) {
+            String logMessage = (command.getActResult().getType() == RESULT.STMT_RESULT_TYPE_ERROR)
+                    ? "[%s][row:%d] wait_expect succeeded with expected error after %d attempts (%.2fs)"
+                    : "[%s][row:%d] wait_expect succeeded after %d attempts (%.2fs)";
+            logger.info(String.format(logMessage,
+                    command.getScriptFile(), command.getPosition(), attemptCount, elapsedMillis / 1000.0));
+            return;
+        }
+
+        long remainingMillis = timeoutMillis - (System.currentTimeMillis() - startTime);
+        if (remainingMillis <= 0) {
+            break;
+        }
+
+        long sleepMillis = Math.min(interval * 1000L, remainingMillis);
+        if (sleepMillis > 0) {
+            Thread.sleep(sleepMillis);
         }
     }
+
+    long totalElapsedMillis = System.currentTimeMillis() - startTime;
+    String message = (command.getActResult() != null && command.getActResult().getType() == RESULT.STMT_RESULT_TYPE_ERROR)
+            ? "Last error did not match expected."
+            : "Last result did not match expected.";
+    logger.warn(String.format("[%s][row:%d] wait_expect timeout after %d attempts (%.2fs). %s",
+            command.getScriptFile(), command.getPosition(), attemptCount, totalElapsedMillis / 1000.0, message));
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical bug in the timeout logic and significant code duplication, proposing a robust refactoring that improves both correctness and maintainability.

High
General
Close ResultSet in loop

Prevent a resource leak by ensuring the ResultSet is closed within the polling
loop, preferably using a try-with-resources block.

src/main/java/io/mo/db/Executor.java [791-797]

-// Get the result
 ResultSet resultSet = statement.getResultSet();
 StmtResult actResult;
 
 if (resultSet != null) {
-    RSSet rsSet = new RSSet(resultSet, command);
-    actResult = new StmtResult(rsSet);
+    try (ResultSet rs = resultSet) {
+        RSSet rsSet = new RSSet(rs, command);
+        actResult = new StmtResult(rsSet);
+    }
 } else {
     actResult = new StmtResult();
     actResult.setType(RESULT.STMT_RESULT_TYPE_NONE);
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential resource leak by not closing the ResultSet within a loop and proposes the standard best practice try-with-resources to fix it.

Medium
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants