Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the handling of PostgreSQL array operators in view definitions by ensuring that only equality operations (= ANY) are converted to IN syntax, while other operators (>, <, <>) preserve their ANY/ALL syntax.
- Adds proper detection of equality vs. non-equality array operators
- Implements separate formatting paths for
= ANY(converted toIN) vs. other operators (preserved asANY/ALL) - Adds comprehensive test coverage for various array operators in view definitions
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ir/formatter.go | Implements the core fix with enhanced ScalarArrayOpExpr formatting and operator extraction logic |
| testdata/diff/create_view/add_view_array_operators/. | Adds test data covering various array operators to validate the fix works correctly |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ir/formatter.go
Outdated
| // Get the operator name by deparsing | ||
| deparsed, err := f.deparseNode(&pg_query.Node{Node: &pg_query.Node_ScalarArrayOpExpr{ScalarArrayOpExpr: arrayOp}}) | ||
| if err != nil { | ||
| // If deparse fails, just return empty (shouldn't happen in practice) | ||
| return | ||
| } | ||
|
|
||
| // Check if operator is = (equality) | ||
| // Deparse will give us something like "col = ANY (...)" or "col > ANY (...)" | ||
| // We need to extract the operator | ||
| isEqualityOp := strings.Contains(deparsed, " = ANY") || strings.Contains(deparsed, "= ANY") |
There was a problem hiding this comment.
The code performs deparsing twice for the same expression - once here to check if it's an equality operator, and again in extractOperator(). Consider extracting the operator first and checking for equality, or cache the deparsed result to avoid redundant work.
479958f to
3553498
Compare
#87 followup