fix: make odata search case insensitive#225
Conversation
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wavefront/server/plugins/datasource/datasource/odata_parser.py (1)
352-353: Consider pre-lowercasing the bound parameter value in Python instead of wrapping withLOWER()in SQL.The current approach is correct —
LOWER(@param)on a constant bound value is evaluated once per query, not per row. However, pre-computing the lowercase in Python simplifies the generated SQL fragment and makes the stored parameter value immediately inspectable/loggable in its final form.♻️ Proposed simplification
if operator == 'contains': parsed_value = self.parse_value(value) - self.params[param_key] = f'%{parsed_value}%' - return f'LOWER({field}) {sql_op} LOWER({self.dynamic_var_char}{param_key})' + self.params[param_key] = f'%{str(parsed_value).lower()}%' + return f'LOWER({field}) {sql_op} {self.dynamic_var_char}{param_key}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wavefront/server/plugins/datasource/datasource/odata_parser.py` around lines 352 - 353, The code currently sets self.params[param_key] = f'%{parsed_value}%' and generates SQL using LOWER({field}) {sql_op} LOWER({self.dynamic_var_char}{param_key}); change this so the parameter value is lowercased in Python (e.g., set self.params[param_key] = f'%{parsed_value.lower()}%') and remove the LOWER() wrapper around the bound parameter in the returned fragment (leave LOWER({field}) {sql_op} {self.dynamic_var_char}{param_key}); update the logic in the method that assigns self.params and returns the SQL fragment (refer to self.params, param_key, parsed_value, dynamic_var_char, field, sql_op) so stored parameters are pre-lowercased and the generated SQL is simplified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wavefront/server/plugins/datasource/datasource/odata_parser.py`:
- Around line 352-353: The code currently sets self.params[param_key] =
f'%{parsed_value}%' and generates SQL using LOWER({field}) {sql_op}
LOWER({self.dynamic_var_char}{param_key}); change this so the parameter value is
lowercased in Python (e.g., set self.params[param_key] =
f'%{parsed_value.lower()}%') and remove the LOWER() wrapper around the bound
parameter in the returned fragment (leave LOWER({field}) {sql_op}
{self.dynamic_var_char}{param_key}); update the logic in the method that assigns
self.params and returns the SQL fragment (refer to self.params, param_key,
parsed_value, dynamic_var_char, field, sql_op) so stored parameters are
pre-lowercased and the generated SQL is simplified.
Summary by CodeRabbit