- 
                Notifications
    You must be signed in to change notification settings 
- Fork 0
          perf(search): simplify LIKE search (drop LOWER(...) in favour of COLLATE NOCASE)
          #695
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… index) maybe we need to add `COLLATE NOCASE` to the where clause, idk... see: Hochfrequenz/xml-fundamend-python#193
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes database query performance by removing LOWER(...) function calls from SQL LIKE search operations, allowing the database to utilize indexes more effectively. The search values are still converted to lowercase on the application side before being passed to the query.
Key changes:
- Removed LOWER()SQL function wrapping from column references inLIKEclauses
- Maintained .toLowerCase()on search parameter values to preserve case-insensitive matching
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
LIKE search (drop LOWER(...) and hence use index)LIKE search (drop LOWER(...) in favour of COLLATE NOCASE)
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| queryBuilder.andWhere(`al.${columnName} COLLATE NOCASE LIKE :${paramBase}_contains`, { | ||
| [`${paramBase}_contains`]: `%${value.contains.toLowerCase()}%`, | 
    
      
    
      Copilot
AI
    
    
    
      Oct 21, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The columnName variable is interpolated directly into the SQL string without sanitization, creating a potential SQL injection vulnerability. Consider using a whitelist of allowed column names or TypeORM's column metadata to validate the column name before interpolation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was like this before. out of scope for this PR
maybe we need to add
COLLATE NOCASEto the where clause, idk...see: Hochfrequenz/xml-fundamend-python#193
@hf-krechan : man müsste mal aus dem fundamend-pr eine DB bauen und schauen ob das so funktioniert. ich glaube aber, dass aktuell der index auf der spalte nicht genutzt wird und ohne collate nocase nur für ascii-zeichen genutzt würde (siehe auch die description vom fundanemnd-pr)