Skip to content

Conversation

@aptend
Copy link
Collaborator

@aptend aptend commented Jan 5, 2026

PR Type

Enhancement, Bug fix


Description

  • Uncommented and activated SQL type mapping with type codes

  • Changed regex pattern handling to skip result file patterns

  • Enhanced config loading to prioritize project root directory

  • Updated tests to verify regex patterns only from SQL files


Diagram Walkthrough

flowchart LR
  A["RESULT.java<br/>Type Mapping"] -->|Uncommented| B["TYPE_NAME_MAP<br/>with type codes"]
  C["ResultParser.java<br/>Regex Handling"] -->|Changed| D["skipRegexPatternsFromResult<br/>Skip result file patterns"]
  E["BaseConfigUtil.java<br/>Config Loading"] -->|Enhanced| F["Load from root dir<br/>then classpath"]
  G["RegexParsingTest.java<br/>Tests"] -->|Updated| H["Verify SQL file<br/>patterns only"]
Loading

File Walkthrough

Relevant files
Enhancement
RESULT.java
Activate SQL type name mapping with type codes                     

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

  • Uncommented the java.sql.Types import statement
  • Activated the TYPE_NAME_MAP static initialization block
  • Added SQL type code comments for each type mapping entry
  • Made the type map public and final for use throughout the application
+43/-42 
BaseConfigUtil.java
Enhance config loading with root directory priority           

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

  • Added priority loading from project root directory using
    System.getProperty("user.dir")
  • Implemented fallback to classpath loading for backward compatibility
  • Improved error handling with separate try-catch blocks for each
    loading strategy
  • Simplified null checks and return logic
+23/-9   
Bug fix
ResultParser.java
Skip regex patterns from result files                                       

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

  • Removed imports for RegexPattern and java.util.regex.Pattern
  • Renamed method from parseRegexPatternsFromResult to
    skipRegexPatternsFromResult
  • Removed 90+ lines of regex pattern parsing logic from result files
  • Changed behavior to skip regex patterns in result files instead of
    parsing them
  • Updated method documentation to clarify that only SQL file patterns
    are used
+10/-102
Tests
RegexParsingTest.java
Update tests for regex pattern precedence                               

src/test/java/io/mo/util/RegexParsingTest.java

  • Updated testParseRegexFromResultFile test to verify regex patterns are
    skipped
  • Changed assertion from expecting 2 patterns to expecting 0 patterns
  • Added new test testSqlFileRegexPatternsTakePrecedence to verify SQL
    file patterns work
  • Added clarifying comments in Chinese explaining the behavior change
  • Verified that result file regex patterns are ignored while SQL file
    patterns are preserved
+41/-2   

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unsafe YAML deserialization

Description: new Yaml().load(...) is used to deserialize YAML from a filesystem path derived from
System.getProperty("user.dir"), which can allow unsafe SnakeYAML deserialization (e.g.,
YAML tags instantiating arbitrary classes) if an attacker can influence the config file
contents or location; prefer SafeConstructor/Yaml(new SafeConstructor(...)) or a typed
schema-based loader.
BaseConfigUtil.java [27-47]

Referred Code
// 优先从项目根目录加载配置文件
java.nio.file.Path rootPath = Paths.get(System.getProperty("user.dir"), configFileName);
if (Files.exists(rootPath)) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(rootPath));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }
}

// 如果根目录不存在,尝试从 classpath 加载(向后兼容)
URL url = BaseConfigUtil.class.getClassLoader().getResource(configFileName);
if (url != null) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(
            Paths.get(URLDecoder.decode(url.getFile(), "utf-8"))));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }
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:
Resource leak risk: The new config-loading code opens streams via Files.newInputStream(...) without
try-with-resources and swallows IOException by only printing a stack trace then returning
null, reducing debuggability and risking leaked file handles.

Referred Code
java.nio.file.Path rootPath = Paths.get(System.getProperty("user.dir"), configFileName);
if (Files.exists(rootPath)) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(rootPath));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }
}

// 如果根目录不存在,尝试从 classpath 加载(向后兼容)
URL url = BaseConfigUtil.class.getClassLoader().getResource(configFileName);
if (url != null) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(
            Paths.get(URLDecoder.decode(url.getFile(), "utf-8"))));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }

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:
Stacktrace exposure: The new exception handling uses e.printStackTrace() which can expose internal file paths
and implementation details to user-visible stderr/stdout instead of controlled internal
logging.

Referred Code
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(rootPath));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }
}

// 如果根目录不存在,尝试从 classpath 加载(向后兼容)
URL url = BaseConfigUtil.class.getClassLoader().getResource(configFileName);
if (url != null) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(
            Paths.get(URLDecoder.decode(url.getFile(), "utf-8"))));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }

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 stacktraces: The new code logs errors via e.printStackTrace() (unstructured and potentially sensitive),
rather than using a structured logger with redaction/controls.

Referred Code
    } catch (IOException e) {
        e.printStackTrace();
    }
}

// 如果根目录不存在,尝试从 classpath 加载(向后兼容)
URL url = BaseConfigUtil.class.getClassLoader().getResource(configFileName);
if (url != null) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(
            Paths.get(URLDecoder.decode(url.getFile(), "utf-8"))));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {
        e.printStackTrace();
    }

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:
Path traversal question: The new root-directory load path is built from System.getProperty("user.dir")
and configFileName, which may allow unintended file access if configFileName is externally
influenced, but this cannot be confirmed from the diff alone.

Referred Code
java.nio.file.Path rootPath = Paths.get(System.getProperty("user.dir"), configFileName);
if (Files.exists(rootPath)) {
    try {
        Map<?, ?> raw = yaml.load(Files.newInputStream(rootPath));
        return raw != null ? (Map<String, Object>) raw : null;
    } catch (IOException e) {

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
General
Use url.openStream for classpath

Replace Files.newInputStream with url.openStream() within a try-with-resources
block for loading classpath resources. This change prevents failures when the
application is run from a JAR file.

src/main/java/io/mo/util/BaseConfigUtil.java [42-44]

-Map<?, ?> raw = yaml.load(Files.newInputStream(
-    Paths.get(URLDecoder.decode(url.getFile(), "utf-8"))));
-return raw != null ? (Map<String, Object>) raw : null;
+try (java.io.InputStream in = url.openStream()) {
+    Map<?, ?> raw = yaml.load(in);
+    return raw != null ? (Map<String, Object>) raw : null;
+} catch (IOException e) {
+    e.printStackTrace();
+}
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that loading a classpath resource via a decoded file path is fragile and will fail in packaged JARs. Using url.openStream() is a much more robust approach that significantly improves application reliability.

Medium
Close YAML InputStream automatically

Wrap the Files.newInputStream(rootPath) call in a try-with-resources block to
prevent potential resource leaks by ensuring the InputStream is always closed.

src/main/java/io/mo/util/BaseConfigUtil.java [30-35]

-try {
-    Map<?, ?> raw = yaml.load(Files.newInputStream(rootPath));
+try (java.io.InputStream in = Files.newInputStream(rootPath)) {
+    Map<?, ?> raw = yaml.load(in);
     return raw != null ? (Map<String, Object>) raw : null;
 } catch (IOException e) {
     e.printStackTrace();
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This suggestion correctly points out a potential resource leak and proposes using a try-with-resources statement, which is a standard Java best practice for ensuring resources are closed properly.

Low
Possible issue
Correctly preserve leading empty lines

Modify the skipRegexPatternsFromResult method to correctly preserve leading
empty lines in the result text. The current logic collapses initial empty lines,
which can alter the expected output.

src/main/java/io/mo/util/ResultParser.java [320-346]

 private static String skipRegexPatternsFromResult(String result, SqlCommand cmd) {
     if (result == null || result.isEmpty()) {
         return result;
     }
 
     String[] lines = result.split("\n", -1);
     StringBuilder remainingResult = new StringBuilder();
-    boolean foundNonRegexLine = false;
 
     for (String line : lines) {
         String trimmedLine = line.trim();
         if (trimmedLine.startsWith(COMMON.REGEX_FLAG)) {
             // Skip regex patterns from result file - only use patterns from SQL test file
             // This line is metadata and should be removed from the result content
             continue;
         } else {
             // This is a regular result line
-            if (foundNonRegexLine || remainingResult.length() > 0) {
+            if (remainingResult.length() > 0) {
                 remainingResult.append("\n");
             }
             remainingResult.append(line);
-            foundNonRegexLine = true;
         }
     }
 
     return remainingResult.toString();
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a subtle bug where leading newlines in the result text are dropped, which could cause incorrect test validations. Fixing this improves the correctness and robustness of the result parser.

Medium
  • More

@heni02 heni02 merged commit d6873cd into matrixorigin:main Jan 5, 2026
2 checks passed
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