From 27f394507e26c13f3d1bd3a8868f7b649038e2fd Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Thu, 4 Jun 2026 02:36:51 +0300 Subject: [PATCH] orm: support OR in dynamic where blocks (#27336) --- vlib/orm/README.md | 27 +++ vlib/orm/orm_dynamic_test.v | 188 +++++++++++++++++ vlib/v/checker/orm.v | 199 +++++++++++------- vlib/v/gen/c/orm.v | 101 ++++++--- vlib/v/parser/orm.v | 77 ++++++- vlib/v/tests/builtin_maps/map_key_expr_test.v | 9 + 6 files changed, 500 insertions(+), 101 deletions(-) diff --git a/vlib/orm/README.md b/vlib/orm/README.md index 05e02b77d..aa6b6dd86 100644 --- a/vlib/orm/README.md +++ b/vlib/orm/README.md @@ -245,6 +245,33 @@ result := sql db { }! ``` +Dynamic ORM blocks can build `WHERE` and `SET` data conditionally. Commas between +emitted dynamic `where` items are joined with `AND`; use `&&` and `||` inside an +item for explicit boolean conditions. + +```v ignore +where_filter := { + if name := req.name { + name == name + }, + id == user_id || tenant_id == tenant_id +} + +rows := sql db { + dynamic select from Foo where where_filter +}! + +update_data := { + name == new_name +} + +sql db { + dynamic update Foo set update_data where { + id == user_id || tenant_id == tenant_id + } +}! +``` + ORM select expressions also support built-in aggregate functions. `count` keeps its legacy syntax, while the other aggregates use SQL-like function calls. diff --git a/vlib/orm/orm_dynamic_test.v b/vlib/orm/orm_dynamic_test.v index cb30c3db5..51c9944b9 100644 --- a/vlib/orm/orm_dynamic_test.v +++ b/vlib/orm/orm_dynamic_test.v @@ -23,6 +23,15 @@ mut: is_required u8 } +@[table: 'dynamic_or_members'] +struct DynamicOrMember { +mut: + id int @[primary] + tenant_id int + name string + status string +} + fn test_dynamic_select_with_inline_where_block() { mut db := sqlite.connect(':memory:')! defer { @@ -315,3 +324,182 @@ fn test_dynamic_select_with_explicit_order_by_asc() { assert rows[1].name == 'Alice' assert rows[1].age == 31 } + +fn test_dynamic_select_where_block_with_or_expression() { + mut db := sqlite.connect(':memory:')! + defer { + db.close() or { panic(err) } + } + + sql db { + create table DynamicOrMember + }! + + members := [ + DynamicOrMember{ + id: 1 + tenant_id: 1 + name: 'Alice' + status: 'active' + }, + DynamicOrMember{ + id: 2 + tenant_id: 2 + name: 'Bob' + status: 'active' + }, + DynamicOrMember{ + id: 3 + tenant_id: 2 + name: 'Charlie' + status: 'pending' + }, + DynamicOrMember{ + id: 4 + tenant_id: 3 + name: 'Diana' + status: 'active' + }, + ] + + for member in members { + sql db { + insert member into DynamicOrMember + }! + } + + active := 'active' + tenant_id := 2 + rows := sql db { + dynamic select from DynamicOrMember where { + status == active && (name == 'Alice' || tenant_id == tenant_id) + } order by id + }! + + assert rows.map(it.name) == ['Alice', 'Bob'] + + grouped_rows := sql db { + dynamic select from DynamicOrMember where { + (name == 'Alice' || status == active) && tenant_id == tenant_id + } order by id + }! + + assert grouped_rows.map(it.name) == ['Bob'] +} + +fn test_dynamic_select_where_block_with_or_expression_and_comma_filter() { + mut db := sqlite.connect(':memory:')! + defer { + db.close() or { panic(err) } + } + + sql db { + create table DynamicOrMember + }! + + members := [ + DynamicOrMember{ + id: 1 + tenant_id: 1 + name: 'Alice' + status: 'active' + }, + DynamicOrMember{ + id: 2 + tenant_id: 2 + name: 'Bob' + status: 'active' + }, + DynamicOrMember{ + id: 3 + tenant_id: 2 + name: 'Charlie' + status: 'pending' + }, + DynamicOrMember{ + id: 4 + tenant_id: 3 + name: 'Diana' + status: 'active' + }, + ] + + for member in members { + sql db { + insert member into DynamicOrMember + }! + } + + tenant_id := 2 + rows := sql db { + dynamic select from DynamicOrMember where { + tenant_id == tenant_id, + name == 'Alice' || status == 'active' + } order by id + }! + + assert rows.map(it.name) == ['Bob'] +} + +fn test_dynamic_update_where_block_with_or_expression() { + mut db := sqlite.connect(':memory:')! + defer { + db.close() or { panic(err) } + } + + sql db { + create table DynamicOrMember + }! + + members := [ + DynamicOrMember{ + id: 1 + tenant_id: 1 + name: 'Alice' + status: 'active' + }, + DynamicOrMember{ + id: 2 + tenant_id: 2 + name: 'Bob' + status: 'active' + }, + DynamicOrMember{ + id: 3 + tenant_id: 2 + name: 'Charlie' + status: 'pending' + }, + DynamicOrMember{ + id: 4 + tenant_id: 3 + name: 'Diana' + status: 'active' + }, + ] + + for member in members { + sql db { + insert member into DynamicOrMember + }! + } + + next_status := 'archived' + update_expr := { + status == next_status + } + user_id := 1 + tenant_id := 2 + + sql db { + dynamic update DynamicOrMember set update_expr where { + id == user_id || tenant_id == tenant_id + } + }! + + rows := sql db { + select from DynamicOrMember order by id + }! + + assert rows.map(it.status) == ['archived', 'archived', 'archived', 'active'] +} diff --git a/vlib/v/checker/orm.v b/vlib/v/checker/orm.v index bb4fbbf7d..f653660fd 100644 --- a/vlib/v/checker/orm.v +++ b/vlib/v/checker/orm.v @@ -46,23 +46,36 @@ fn (mut c Checker) check_sql_query_data_item(mut item ast.SqlQueryDataItem) { } fn (mut c Checker) check_sql_query_data_leaf(node ast.SqlQueryDataLeaf) { - mut expr := node.expr - expr_ := expr.remove_par() - if expr_ is ast.InfixExpr { - if !is_sql_query_data_op(expr_.op) { - c.orm_error('dynamic ORM items must use comparison operators', expr_.pos) - return + c.check_sql_query_data_expr(node.expr, node.pos) +} + +fn (mut c Checker) check_sql_query_data_expr(expr ast.Expr, pos token.Pos) { + match expr { + ast.ParExpr { + c.check_sql_query_data_expr(expr.expr, pos) } - if !is_sql_query_data_field_candidate(expr_.left) { - c.orm_error('left side of a dynamic ORM item must be a field name', expr_.left.pos()) + ast.InfixExpr { + if expr.op in [.and, .logical_or] { + c.check_sql_query_data_expr(expr.left, pos) + c.check_sql_query_data_expr(expr.right, pos) + return + } + if !is_sql_query_data_op(expr.op) { + c.orm_error('dynamic ORM items must use comparison operators', expr.pos) + return + } + if !is_sql_query_data_field_candidate(expr.left) { + c.orm_error('left side of a dynamic ORM item must be a field name', expr.left.pos()) + } + is_nil_comparison := expr.right is ast.Nil && expr.op in [.eq, .ne] + if expr.op !in [.key_is, .not_is] && !is_nil_comparison { + mut rhs_expr := expr.right + c.expr(mut rhs_expr) + } } - is_nil_comparison := expr_.right is ast.Nil && expr_.op in [.eq, .ne] - if expr_.op !in [.key_is, .not_is] && !is_nil_comparison { - mut rhs_expr := expr_.right - c.expr(mut rhs_expr) + else { + c.orm_error('dynamic ORM items must be comparison expressions', pos) } - } else { - c.orm_error('dynamic ORM items must be comparison expressions', node.pos) } } @@ -147,70 +160,36 @@ fn (mut c Checker) check_dynamic_sql_query_data_items(mut items []ast.SqlQueryDa for mut item in items { match item { ast.SqlQueryDataLeaf { - mut expr := item.expr - expr_ := expr.remove_par() - if expr_ is ast.InfixExpr { - field_name := sql_query_data_field_name(expr_.left) - if field_name == '' { - c.orm_error('left side of a dynamic ORM item must be a field name', - expr_.left.pos()) - ok = false - continue - } - matched_fields := fields.filter(it.name == field_name) - if matched_fields.len == 0 { - c.orm_error(util.new_suggestion(field_name, field_names).say('`${table_sym.name}` structure has no field with name `${field_name}`'), - expr_.left.pos()) - ok = false - continue + if !c.check_dynamic_sql_query_data_expr(item.expr, table_sym, fields, field_names, + context) { + ok = false + continue + } + match context { + .where_ { + mut where_expr := item.expr + c.expr(mut where_expr) + c.check_expr_has_no_fn_calls_with_non_orm_return_type(&where_expr) + c.check_where_expr_has_no_pointless_exprs(table_sym, field_names, + &where_expr) + item.expr = where_expr } - field := matched_fields[0] - match context { - .where_ { - if expr_.op in [.and, .logical_or] { - c.orm_error('dynamic ORM `where` items must use a single comparison expression', - expr_.pos) - ok = false - continue + .set_ { + expr_ := item.expr.remove_par() + if expr_ is ast.InfixExpr { + field_name := sql_query_data_field_name(expr_.left) + matched_fields := fields.filter(it.name == field_name) + if matched_fields.len > 0 { + field := matched_fields[0] + mut set_expr := item.expr + old_expected_type := c.expected_type + c.expected_type = field.typ + c.expr(mut set_expr) + c.expected_type = old_expected_type + item.expr = set_expr } - if !is_sql_query_data_op(expr_.op) { - c.orm_error('dynamic ORM `where` items must use comparison operators', - expr_.pos) - ok = false - continue - } - mut where_expr := item.expr - c.expr(mut where_expr) - c.check_expr_has_no_fn_calls_with_non_orm_return_type(&where_expr) - c.check_where_expr_has_no_pointless_exprs(table_sym, field_names, - &where_expr) - item.expr = where_expr - } - .set_ { - if expr_.op != .eq { - c.orm_error('dynamic ORM `set` items must use `==`', expr_.pos) - ok = false - continue - } - for attr in field.attrs { - if attr.name == 'fkey' { - c.orm_error("`${field_name}` is a foreign column of `${table_sym.name}`, it can't update here", - expr_.pos) - ok = false - break - } - } - mut set_expr := item.expr - old_expected_type := c.expected_type - c.expected_type = field.typ - c.expr(mut set_expr) - c.expected_type = old_expected_type - item.expr = set_expr } } - } else { - ok = false - continue } } ast.SqlQueryDataIf { @@ -231,6 +210,72 @@ fn (mut c Checker) check_dynamic_sql_query_data_items(mut items []ast.SqlQueryDa return ok } +fn (mut c Checker) check_dynamic_sql_query_data_expr(expr ast.Expr, table_sym &ast.TypeSymbol, + fields []ast.StructField, field_names []string, context SqlQueryDataContext) bool { + return match expr { + ast.ParExpr { + c.check_dynamic_sql_query_data_expr(expr.expr, table_sym, fields, field_names, context) + } + ast.InfixExpr { + if expr.op in [.and, .logical_or] { + match context { + .where_ { + left_ok := c.check_dynamic_sql_query_data_expr(expr.left, table_sym, + fields, field_names, context) + right_ok := c.check_dynamic_sql_query_data_expr(expr.right, table_sym, + fields, field_names, context) + return left_ok && right_ok + } + .set_ { + c.orm_error('dynamic ORM `set` items must use `==`', expr.pos) + return false + } + } + } + field_name := sql_query_data_field_name(expr.left) + if field_name == '' { + c.orm_error('left side of a dynamic ORM item must be a field name', expr.left.pos()) + return false + } + matched_fields := fields.filter(it.name == field_name) + if matched_fields.len == 0 { + c.orm_error(util.new_suggestion(field_name, field_names).say('`${table_sym.name}` structure has no field with name `${field_name}`'), + expr.left.pos()) + return false + } + match context { + .where_ { + if !is_sql_query_data_op(expr.op) { + c.orm_error('dynamic ORM `where` items must use comparison operators', + expr.pos) + return false + } + } + .set_ { + if expr.op != .eq { + c.orm_error('dynamic ORM `set` items must use `==`', expr.pos) + return false + } + field := matched_fields[0] + for attr in field.attrs { + if attr.name == 'fkey' { + c.orm_error("`${field_name}` is a foreign column of `${table_sym.name}`, it can't update here", + expr.pos) + return false + } + } + } + } + + true + } + else { + c.orm_error('dynamic ORM items must be comparison expressions', expr.pos()) + false + } + } +} + fn (mut c Checker) sql_expr(mut node ast.SqlExpr) ast.Type { c.inside_sql = true defer { @@ -722,6 +767,12 @@ fn (mut c Checker) sql_stmt_line(mut node ast.SqlStmtLine) ast.Type { if node.where_expr !is ast.EmptyExpr && !node.is_array_update { c.expr(mut node.where_expr) + if dynamic_where_expr := c.resolve_sql_query_data_expr(node.where_expr) { + _ = dynamic_where_expr + if !c.check_dynamic_sql_query_data(node.where_expr, table_sym, node.fields, .where_) { + return ast.void_type + } + } } return ast.void_type diff --git a/vlib/v/gen/c/orm.v b/vlib/v/gen/c/orm.v index 5886f768d..92474118d 100644 --- a/vlib/v/gen/c/orm.v +++ b/vlib/v/gen/c/orm.v @@ -465,7 +465,14 @@ fn (mut g Gen) emit_sql_query_data_items(query_var string, items []ast.SqlQueryD for item in items { match item { ast.SqlQueryDataLeaf { - g.emit_sql_query_data_leaf(query_var, item, resolve_columns) + if is_sql_query_data_top_level_or(item.expr) { + start_var := g.new_tmp_var() + g.writeln('${ast.int_type_name} ${start_var} = ${query_var}.fields.len;') + g.emit_sql_query_data_expr(query_var, item.expr, resolve_columns, true) + g.emit_sql_query_data_parentheses(query_var, start_var) + continue + } + g.emit_sql_query_data_expr(query_var, item.expr, resolve_columns, true) } ast.SqlQueryDataIf { mut prev_guard := SqlQueryDataGuardState{} @@ -526,33 +533,67 @@ fn (mut g Gen) emit_sql_query_data_items(query_var string, items []ast.SqlQueryD } } -fn (mut g Gen) emit_sql_query_data_leaf(query_var string, node ast.SqlQueryDataLeaf, resolve_columns bool) { - mut expr := node.expr - expr_ := expr.remove_par() - if expr_ is ast.InfixExpr { - mut field_name := sql_query_data_field_name(expr_.left) - if resolve_columns { - field := g.get_orm_current_table_field(field_name) or { - verror('field "${field_name}" does not exist on "${g.sql_table_name}"') +fn is_sql_query_data_top_level_or(expr ast.Expr) bool { + return match expr { + ast.InfixExpr { expr.op == .logical_or } + else { false } + } +} + +fn (mut g Gen) emit_sql_query_data_parentheses(query_var string, start_var string) { + g.writeln('if (${query_var}.fields.len > ${start_var}) {') + g.indent++ + g.write('builtin__array_push(&${query_var}.parentheses, _MOV((Array_${ast.int_type_name}[1]){') + g.write('builtin__new_array_from_c_array(2, 2, sizeof(${ast.int_type_name}),') + g.write(' _MOV((${ast.int_type_name}[2]){${start_var}, ${query_var}.fields.len - 1}))') + g.writeln('}));') + g.indent-- + g.writeln('}') +} + +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 { + start_var := g.new_tmp_var() + g.writeln('${ast.int_type_name} ${start_var} = ${query_var}.fields.len;') + g.emit_sql_query_data_expr(query_var, expr.expr, resolve_columns, is_and) + g.emit_sql_query_data_parentheses(query_var, start_var) + } + ast.InfixExpr { + if expr.op in [.and, .logical_or] { + g.emit_sql_query_data_expr(query_var, expr.left, resolve_columns, is_and) + g.emit_sql_query_data_expr(query_var, expr.right, resolve_columns, expr.op == .and) + } else { + g.emit_sql_query_data_leaf(query_var, expr, resolve_columns, is_and) } - field_name = g.get_orm_column_name_from_struct_field(field) } - is_nil_comparison := expr_.right is ast.Nil && expr_.op in [.eq, .ne] - ignore_rhs := expr_.op in [.key_is, .not_is] || is_nil_comparison - g.writeln('if (${query_var}.fields.len > 0) {') - g.indent++ - g.writeln('builtin__array_push(&${query_var}.is_and, _MOV((bool[1]){ true }));') - g.indent-- - g.writeln('}') - g.writeln('builtin__array_push(&${query_var}.fields, _MOV((string[1]){ _S("${field_name}") }));') - g.writeln('builtin__array_push(&${query_var}.kinds, _MOV((orm__OperationKind[1]){ ${sql_query_data_op_kind(expr_)} }));') - if !ignore_rhs { - g.write('builtin__array_push(&${query_var}.data, _MOV((orm__Primitive[1]){') - g.write_orm_expr_to_primitive(expr_.right) - g.writeln('}));') + else { + verror('ORM: dynamic query-data leaf must be an infix expression') } - } else { - verror('ORM: dynamic query-data leaf must be an infix expression') + } +} + +fn (mut g Gen) emit_sql_query_data_leaf(query_var string, expr ast.InfixExpr, resolve_columns bool, is_and bool) { + mut field_name := sql_query_data_field_name(expr.left) + if resolve_columns { + field := g.get_orm_current_table_field(field_name) or { + verror('field "${field_name}" does not exist on "${g.sql_table_name}"') + } + field_name = g.get_orm_column_name_from_struct_field(field) + } + is_nil_comparison := expr.right is ast.Nil && expr.op in [.eq, .ne] + ignore_rhs := expr.op in [.key_is, .not_is] || is_nil_comparison + g.writeln('if (${query_var}.fields.len > 0) {') + g.indent++ + g.writeln('builtin__array_push(&${query_var}.is_and, _MOV((bool[1]){ ${is_and} }));') + g.indent-- + g.writeln('}') + g.writeln('builtin__array_push(&${query_var}.fields, _MOV((string[1]){ _S("${field_name}") }));') + g.writeln('builtin__array_push(&${query_var}.kinds, _MOV((orm__OperationKind[1]){ ${sql_query_data_op_kind(expr)} }));') + if !ignore_rhs { + g.write('builtin__array_push(&${query_var}.data, _MOV((orm__Primitive[1]){') + g.write_orm_expr_to_primitive(expr.right) + g.writeln('}));') } } @@ -1141,6 +1182,10 @@ fn (mut g Gen) write_orm_update(node &ast.SqlStmtLine, table_name string, connec if node.is_dynamic { dynamic_update_data_var = g.emit_dynamic_sql_query_data(node.update_data_expr) } + mut dynamic_where_var := '' + if dynamic_where_expr := g.resolve_sql_query_data_expr(node.where_expr) { + dynamic_where_var = g.emit_sql_query_data(dynamic_where_expr, true) + } if node.is_array_update { g.write_orm_bulk_update(node, table_name, connection_var_name, result_var_name) return @@ -1204,7 +1249,11 @@ fn (mut g Gen) write_orm_update(node &ast.SqlStmtLine, table_name string, connec g.indent-- g.writeln('},') } - g.write_orm_where(node.where_expr) + if dynamic_where_var != '' { + g.writeln(dynamic_where_var) + } else { + g.write_orm_where(node.where_expr) + } g.indent-- g.writeln(');') } diff --git a/vlib/v/parser/orm.v b/vlib/v/parser/orm.v index c53860fc9..2f363dfa7 100644 --- a/vlib/v/parser/orm.v +++ b/vlib/v/parser/orm.v @@ -564,10 +564,23 @@ fn (p &Parser) is_sql_query_data_expr() bool { for p.peek_token(idx).kind == .comment { idx++ } - first := p.peek_token(idx) + mut first := p.peek_token(idx) if first.kind == .key_if { return true } + has_leading_paren := first.kind == .lpar + if has_leading_paren && p.sql_query_data_item_has_top_level_colon(idx) { + return false + } + mut leading_parens := 0 + for first.kind == .lpar { + leading_parens++ + idx++ + for p.peek_token(idx).kind == .comment { + idx++ + } + first = p.peek_token(idx) + } if first.kind != .name { return false } @@ -575,9 +588,71 @@ fn (p &Parser) is_sql_query_data_expr() bool { for p.peek_token(idx).kind == .dot && p.peek_token(idx + 1).kind == .name { idx += 2 } + for leading_parens > 0 && p.peek_token(idx).kind == .rpar { + leading_parens-- + idx++ + for p.peek_token(idx).kind == .comment { + idx++ + } + } return is_sql_query_data_operator(p.peek_token(idx).kind) } +fn (p &Parser) sql_query_data_item_has_top_level_colon(start_idx int) bool { + mut idx := start_idx + mut par_depth := 0 + mut bracket_depth := 0 + mut brace_depth := 0 + for { + kind := p.peek_token(idx).kind + match kind { + .eof { + return false + } + .comment {} + .lpar { + par_depth++ + } + .rpar { + if par_depth > 0 { + par_depth-- + } + } + .lsbr { + bracket_depth++ + } + .rsbr { + if bracket_depth > 0 { + bracket_depth-- + } + } + .lcbr { + brace_depth++ + } + .rcbr { + if brace_depth == 0 { + return false + } + brace_depth-- + } + .comma { + if par_depth == 0 && bracket_depth == 0 && brace_depth == 0 { + return false + } + } + .colon { + if par_depth == 0 && bracket_depth == 0 && brace_depth == 0 { + return true + } + } + else {} + } + + idx++ + } + return false +} + fn is_sql_query_data_operator(kind token.Kind) bool { return kind in [.eq, .ne, .gt, .lt, .ge, .le, .key_like, .key_ilike, .key_in, .not_in, .key_is, .not_is] diff --git a/vlib/v/tests/builtin_maps/map_key_expr_test.v b/vlib/v/tests/builtin_maps/map_key_expr_test.v index 4b1a2a8f9..5b8296a87 100644 --- a/vlib/v/tests/builtin_maps/map_key_expr_test.v +++ b/vlib/v/tests/builtin_maps/map_key_expr_test.v @@ -25,3 +25,12 @@ fn test_method_call() { assert m2[Enum.a.str()] == 'first' assert m2[Enum.b.str()] == 'second' } + +fn test_parenthesized_comparison_key() { + a := 1 + b := 1 + m3 := { + (a == b).str(): 'same' + } + assert m3['true'] == 'same' +} -- 2.39.5