orm: support OR in dynamic where blocks#27336
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6562f66ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| match item { | ||
| ast.SqlQueryDataLeaf { | ||
| g.emit_sql_query_data_leaf(query_var, item, resolve_columns) | ||
| g.emit_sql_query_data_expr(query_var, item.expr, resolve_columns, true) |
There was a problem hiding this comment.
When a dynamic where item contains || and is adjacent to another emitted item, this flattens the item's terms into the surrounding QueryData without adding parentheses. For example where { tenant_id == t, name == 'Alice' || status == 'active' } is emitted as tenant_id = ? AND name = ? OR status = ?, so SQL precedence makes rows with status = 'active' match regardless of tenant; the comma should join the whole item with AND, i.e. tenant_id = ? AND (name = ? OR status = ?). This also affects conditional branches that emit an OR item followed by another item.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05361d709c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| fn (mut g Gen) emit_sql_query_data_expr(query_var string, expr ast.Expr, resolve_columns bool, is_and bool) { | ||
| match expr { | ||
| ast.ParExpr { |
There was a problem hiding this comment.
This adds ParExpr handling for dynamic ORM conditions, but query-data blocks whose first item starts with ( still are not parsed as SqlQueryDataExpr because is_sql_query_data_expr() only recognizes { name <op> ... } or { if ... }. As a result, a natural grouping such as dynamic select from Foo where { (name == x || status == y) && tenant_id == t } is rejected before this branch can run, so OR support is incomplete for expressions that need leading parentheses to express precedence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc417879e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for first.kind == .lpar { | ||
| leading_parens++ | ||
| idx++ | ||
| for p.peek_token(idx).kind == .comment { | ||
| idx++ | ||
| } | ||
| first = p.peek_token(idx) | ||
| } |
There was a problem hiding this comment.
Because this predicate is called for every { expression before falling back to map_init(), accepting leading parentheses makes non-ORM map literals such as m := { (a == b): 1 } get parsed as SqlQueryDataExpr; the query-data parser then reaches the : and reports expected ',' or '}'. Before this change the leading ( caused the expression to fall through to normal map parsing, so please avoid classifying parenthesized map keys as dynamic ORM blocks.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #27002.
Dynamic ORM query-data blocks now support explicit boolean expressions inside a single emitted item, including
||and parenthesized groups. This lets dynamicwhereblocks generateORconditions instead of forcing every emitted item to be joined withAND.The checker now recursively validates dynamic query-data expressions, while cgen recursively emits the corresponding
orm.QueryDatafields, operation kinds,is_andflags, and parentheses. Dynamicupdate ... where { ... }blocks are also resolved as dynamic query data, matching dynamic select behavior.Validation
./v -g -keepc -o ./vnew cmd/v./vnew fmt -w vlib/orm/orm_dynamic_test.v vlib/v/checker/orm.v vlib/v/gen/c/orm.v./vnew check-md vlib/orm/README.md./vnew test vlib/orm/orm_dynamic_test.v./vnew -silent vlib/v/compiler_errors_test.v./vnew -silent vlib/v/gen/c/coutput_test.v./vnew -silent test vlib/v/reported one unrelatedvlib/v/builder/builder_test.vfailure because atccfallback warning was included in stdout; rerun withVFLAGS='-cc clang' ./vnew -cc clang -silent vlib/v/builder/builder_test.vpassed.