Enable using functions on ooquery wheres and Add Unaccent function - #37
Enable using functions on ooquery wheres and Add Unaccent function#37guilleJB wants to merge 12 commits into
Conversation
guilleJB
changed the title
|
Pyhton 2 an 3 runs 
|
There was a problem hiding this comment.
Pull Request Overview
This PR enables using functions in ooquery where clauses and adds a new Unaccent function for database queries. The changes allow SQL functions to be used in query conditions with proper field resolution and parameter handling.
Key changes:
- Added support for Function objects in query expressions with recursive parameter resolution
- Introduced new Unaccent function for PostgreSQL text normalization
- Enhanced expression parsing to handle functions on both sides of comparisons
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| spec/ooquery_spec.py | Comprehensive test coverage for Unaccent and Upper function usage in queries |
| ooquery/parser.py | Core logic for function resolution and expression parsing with new resolve_value method |
| ooquery/functions.py | New Unaccent function implementation extending sql.functions.Function |
| ooquery/init.py | Module exports updated to include functions module |
Comments suppressed due to low confidence (1)
ooquery/parser.py:1
- [nitpick] Comments are written in Catalan. For consistency and maintainability, consider translating these to English as they document important logic about field resolution and function parameter handling.
# coding=utf-8
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| sel.where = And((join2.right.code == 'XXX',)) | ||
| expect(tuple(sql)).to(equal(tuple(sel))) | ||
|
|
||
| with it('unnacent funciton runs good'): |
There was a problem hiding this comment.
Spelling errors in test description: 'unnacent' should be 'unaccent' and 'funciton' should be 'function'.
| with it('unnacent funciton runs good'): | |
| with it('unaccent function runs good'): |
| sel.where = And((Upper(t.field3) > Upper('XXXX'),)) | ||
| expect(tuple(sql)).to(equal(tuple(sel))) | ||
|
|
||
| with it('must support Upper queries with fields without especific and values'): |
There was a problem hiding this comment.
Spelling error: 'especific' should be 'specific'.
| with it('must support Upper queries with fields without especific and values'): | |
| with it('must support Upper queries with fields without specific and values'): |
|
|
||
| with it('must support unaccent queries with fields and values'): | ||
|
|
||
| from sql.functions import Upper |
There was a problem hiding this comment.
The import from sql.functions import Upper is repeated in multiple test cases. Consider moving this import to the top of the file to reduce duplication.
| expect(tuple(sql)).to(equal(tuple(sel))) | ||
|
|
||
| with it('must support Upper queries'): | ||
| from sql.functions import Upper |
There was a problem hiding this comment.
The import from sql.functions import Upper is repeated in multiple test cases. Consider moving this import to the top of the file to reduce duplication.
| expect(tuple(sql)).to(equal(tuple(sel))) | ||
|
|
||
| with it('must support Upper queries with fields and values'): | ||
| from sql.functions import Upper |
There was a problem hiding this comment.
The import from sql.functions import Upper is repeated in multiple test cases. Consider moving this import to the top of the file to reduce duplication.
| if isinstance(value, string_types): | ||
| if in_function: | ||
| if side == 'right' and '.' not in value: | ||
| # dins de funció al RHS, cadena plana ⇒ literal (parametritzada) |
There was a problem hiding this comment.
[nitpick] Comments are written in Catalan. For consistency and maintainability, consider translating these to English as they document important logic about field resolution and function parameter handling.
| if side == 'right' and '.' not in value: | ||
| # dins de funció al RHS, cadena plana ⇒ literal (parametritzada) | ||
| return value | ||
| # dins de funció al LHS o RHS amb camí 'a.b' ⇒ camp |
There was a problem hiding this comment.
[nitpick] Comments are written in Catalan. For consistency and maintainability, consider translating these to English as they document important logic about field resolution and function parameter handling.
| return value | ||
| # dins de funció al LHS o RHS amb camí 'a.b' ⇒ camp | ||
| return self.get_table_field(self.table, value) | ||
| # Top-level: LHS ⇒ camp, RHS ⇒ literal |
There was a problem hiding this comment.
[nitpick] Comments are written in Catalan. For consistency and maintainability, consider translating these to English as they document important logic about field resolution and function parameter handling.
| for p in value.params: | ||
| new_params.append( | ||
| self.resolve_value(p, side=side, in_function=True)) | ||
| # IMPORTANT: no mutar tuples! Re-creem la funció amb els params resolts |
There was a problem hiding this comment.
[nitpick] Comment is in Catalan. Consider translating to English: '# IMPORTANT: don't mutate tuples! Recreate the function with resolved params'.
| # IMPORTANT: no mutar tuples! Re-creem la funció amb els params resolts | |
| # IMPORTANT: don't mutate tuples! Recreate the function with resolved params |
| def get_expressions(self, expression): | ||
| fields = [expression[0]] | ||
| fields = [] | ||
| if isinstance(expression[0], Function): | ||
| resolved_field = self.resolve_value(expression[0], side='left') | ||
| fields.append(resolved_field) | ||
| else: | ||
| fields.append(expression[0]) |
There was a problem hiding this comment.
The logic for handling Function vs non-Function expressions is duplicated between get_expressions and get_expressions2. Consider consolidating this into a single method or refactoring to avoid duplication.
No description provided.